Re: [PATCH 04/27] fs: port fs{g,u}id helpers to const mnt_idmap

[Jan Kara]

On Tue 01-09-26 14:14:29, Christian Brauner wrote:
> Convert to const struct mnt_idmap.
> 
> A mount's idmapping is immutable. The only thing that is allowed to be
> modified afterwards is the reference count and that is hidden behind
> mnt_idmap_get() and mnt_idmap_put(). Everything else only ever reads
> from the idmapping. This is the same model that struct cred uses and the
> idmapping is also rather sensitive.
> 
> So make the idmap argument const wherever we can. The conversion is done
> from the bottom up so callers can continue to pass a non-const pointer
> to a const parameter until the conversion is finished.
> 
> No functional changes.
> 
> Signed-off-by: Christian Brauner (Amutable) <brauner@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/ceph/mds_client.h          |  2 +-
>  fs/fuse/dir.c                 | 13 +++++++------
>  fs/fuse/fuse_i.h              |  4 ++--
>  fs/fuse/req.c                 |  8 +++++---
>  include/linux/fs.h            |  6 +++---
>  include/linux/mnt_idmapping.h |  4 ++--
>  6 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index 3c62e3c3530b..1ac8a7e08e5c 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -375,7 +375,7 @@ struct ceph_mds_request {
>  	int r_fmode;        /* file mode, if expecting cap */
>  	int r_request_release_offset;
>  	const struct cred *r_cred;
> -	struct mnt_idmap *r_mnt_idmap;
> +	const struct mnt_idmap *r_mnt_idmap;
>  	struct timespec64 r_stamp;
>  
>  	/* for choosing which mds to send this request to */
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 0d297f74c663..7a204f55ca84 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -820,7 +820,7 @@ static void free_ext_value(struct fuse_args *args)
>   * If the filesystem doesn't support this, then fall back to separate
>   * 'mknod' + 'open' requests.
>   */
> -static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
> +static int fuse_create_open(const struct mnt_idmap *idmap, struct inode *dir,
>  			    struct dentry *entry, struct file *file,
>  			    unsigned int flags, umode_t mode, u32 opcode)
>  {
> @@ -980,7 +980,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
>  /*
>   * Code shared between mknod, mkdir, symlink and link
>   */
> -static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_mount *fm,
> +static struct dentry *create_new_entry(const struct mnt_idmap *idmap, struct fuse_mount *fm,
>  				       struct fuse_args *args, struct inode *dir,
>  				       struct dentry *entry, umode_t mode)
>  {
> @@ -1053,7 +1053,7 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
>  	return ERR_PTR(err);
>  }
>  
> -static int create_new_nondir(struct mnt_idmap *idmap, struct fuse_mount *fm,
> +static int create_new_nondir(const struct mnt_idmap *idmap, struct fuse_mount *fm,
>  			     struct fuse_args *args, struct inode *dir,
>  			     struct dentry *entry, umode_t mode)
>  {
> @@ -1256,9 +1256,10 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
>  	return err;
>  }
>  
> -static int fuse_rename_common(struct mnt_idmap *idmap, struct inode *olddir, struct dentry *oldent,
> -			      struct inode *newdir, struct dentry *newent,
> -			      unsigned int flags, int opcode, size_t argsize)
> +static int fuse_rename_common(const struct mnt_idmap *idmap, struct inode *olddir,
> +			      struct dentry *oldent, struct inode *newdir,
> +			      struct dentry *newent, unsigned int flags,
> +			      int opcode, size_t argsize)
>  {
>  	int err;
>  	struct fuse_rename2_in inarg;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index c8d4c5f3af7e..a07c75377a30 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1003,7 +1003,7 @@ void __exit fuse_ctl_cleanup(void);
>  /*
>   * Simple request sending that does request allocation and freeing
>   */
> -ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
> +ssize_t __fuse_simple_request(const struct mnt_idmap *idmap,
>  			      struct fuse_mount *fm,
>  			      struct fuse_args *args);
>  
> @@ -1012,7 +1012,7 @@ static inline ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_arg
>  	return __fuse_simple_request(&invalid_mnt_idmap, fm, args);
>  }
>  
> -static inline ssize_t fuse_simple_idmap_request(struct mnt_idmap *idmap,
> +static inline ssize_t fuse_simple_idmap_request(const struct mnt_idmap *idmap,
>  						struct fuse_mount *fm,
>  						struct fuse_args *args)
>  {
> diff --git a/fs/fuse/req.c b/fs/fuse/req.c
> index a01ee743d31e..a133b012d041 100644
> --- a/fs/fuse/req.c
> +++ b/fs/fuse/req.c
> @@ -3,7 +3,8 @@
>  #include "dev.h"
>  #include "fuse_i.h"
>  
> -static int fuse_fill_creds(struct fuse_mount *fm, struct fuse_args *args, struct mnt_idmap *idmap)
> +static int fuse_fill_creds(struct fuse_mount *fm, struct fuse_args *args,
> +			   const struct mnt_idmap *idmap)
>  {
>  	struct fuse_conn *fc = fm->fc;
>  	bool no_idmap = !fm->sb || (fm->sb->s_iflags & SB_I_NOIDMAP);
> @@ -49,7 +50,8 @@ static int fuse_fill_creds(struct fuse_mount *fm, struct fuse_args *args, struct
>  	return 0;
>  }
>  
> -static int fuse_req_prep(struct fuse_mount *fm, struct fuse_args *args, struct mnt_idmap *idmap)
> +static int fuse_req_prep(struct fuse_mount *fm, struct fuse_args *args,
> +			 const struct mnt_idmap *idmap)
>  {
>  	if (!args->force && fm->fc->conn_error)
>  		return -ECONNREFUSED;
> @@ -57,7 +59,7 @@ static int fuse_req_prep(struct fuse_mount *fm, struct fuse_args *args, struct m
>  	return fuse_fill_creds(fm, args, idmap);
>  }
>  
> -ssize_t __fuse_simple_request(struct mnt_idmap *idmap, struct fuse_mount *fm,
> +ssize_t __fuse_simple_request(const struct mnt_idmap *idmap, struct fuse_mount *fm,
>  			      struct fuse_args *args)
>  {
>  	struct fuse_conn *fc = fm->fc;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f9d1e05e8ae6..959c747ef75a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1544,7 +1544,7 @@ static inline void i_gid_update(struct mnt_idmap *idmap,
>   * an idmapped mount map the caller's fsuid according to @idmap.
>   */
>  static inline void inode_fsuid_set(struct inode *inode,
> -				   struct mnt_idmap *idmap)
> +				   const struct mnt_idmap *idmap)
>  {
>  	inode->i_uid = mapped_fsuid(idmap, i_user_ns(inode));
>  }
> @@ -1558,7 +1558,7 @@ static inline void inode_fsuid_set(struct inode *inode,
>   * an idmapped mount map the caller's fsgid according to @idmap.
>   */
>  static inline void inode_fsgid_set(struct inode *inode,
> -				   struct mnt_idmap *idmap)
> +				   const struct mnt_idmap *idmap)
>  {
>  	inode->i_gid = mapped_fsgid(idmap, i_user_ns(inode));
>  }
> @@ -1575,7 +1575,7 @@ static inline void inode_fsgid_set(struct inode *inode,
>   * Return: true if fsuid and fsgid is mapped, false if not.
>   */
>  static inline bool fsuidgid_has_mapping(struct super_block *sb,
> -					struct mnt_idmap *idmap)
> +					const struct mnt_idmap *idmap)
>  {
>  	struct user_namespace *fs_userns = sb->s_user_ns;
>  	kuid_t kuid;
> diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
> index b16f52d27ba0..6656aea23be2 100644
> --- a/include/linux/mnt_idmapping.h
> +++ b/include/linux/mnt_idmapping.h
> @@ -225,7 +225,7 @@ static inline kgid_t vfsgid_into_kgid(vfsgid_t vfsgid)
>   *
>   * Return: the caller's current fsuid mapped up according to @idmap.
>   */
> -static inline kuid_t mapped_fsuid(struct mnt_idmap *idmap,
> +static inline kuid_t mapped_fsuid(const struct mnt_idmap *idmap,
>  				  struct user_namespace *fs_userns)
>  {
>  	return from_vfsuid(idmap, fs_userns, VFSUIDT_INIT(current_fsuid()));
> @@ -244,7 +244,7 @@ static inline kuid_t mapped_fsuid(struct mnt_idmap *idmap,
>   *
>   * Return: the caller's current fsgid mapped up according to @idmap.
>   */
> -static inline kgid_t mapped_fsgid(struct mnt_idmap *idmap,
> +static inline kgid_t mapped_fsgid(const struct mnt_idmap *idmap,
>  				  struct user_namespace *fs_userns)
>  {
>  	return from_vfsgid(idmap, fs_userns, VFSGIDT_INIT(current_fsgid()));
> 
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR