| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c3cd7afa8a319d7e45266d30741acb582b0085b0.camel@kernel.org>
Date: Mon, 21 Jul 2025 09:06:10 -0400
From: Jeff Layton <jlayton@...nel.org>
To: NeilBrown <neil@...wn.name>, Linus Torvalds
<torvalds@...ux-foundation.org>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] VFS: unify old_mnt_idmap and new_mnt_idmap in
renamedata
On Mon, 2025-07-21 at 17:59 +1000, NeilBrown wrote:
> A rename can only rename with a single mount. Callers of vfs_rename()
> must and do ensure this is the case.
>
> So there is no point in having two mnt_idmaps in renamedata as they are
> always the same. Only of of them is passed to ->rename in any case.
>
> This patch replaces both with a single "mnt_idmap" and changes all
> callers.
>
> Signed-off-by: NeilBrown <neil@...wn.name>
> ---
> fs/cachefiles/namei.c | 3 +--
> fs/ecryptfs/inode.c | 3 +--
> fs/namei.c | 17 ++++++++---------
> fs/nfsd/vfs.c | 3 +--
> fs/overlayfs/overlayfs.h | 3 +--
> fs/smb/server/vfs.c | 3 +--
> include/linux/fs.h | 6 ++----
> 7 files changed, 15 insertions(+), 23 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 91dfd0231877..d1edb2ac3837 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -387,10 +387,9 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> cachefiles_io_error(cache, "Rename security error %d", ret);
> } else {
> struct renamedata rd = {
> - .old_mnt_idmap = &nop_mnt_idmap,
> + .mnt_idmap = &nop_mnt_idmap,
> .old_parent = dir,
> .old_dentry = rep,
> - .new_mnt_idmap = &nop_mnt_idmap,
> .new_parent = cache->graveyard,
> .new_dentry = grave,
> };
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 72fbe1316ab8..abd954c6a14e 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -634,10 +634,9 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
> goto out_lock;
> }
>
> - rd.old_mnt_idmap = &nop_mnt_idmap;
> + rd.mnt_idmap = &nop_mnt_idmap;
> rd.old_parent = lower_old_dir_dentry;
> rd.old_dentry = lower_old_dentry;
> - rd.new_mnt_idmap = &nop_mnt_idmap;
> rd.new_parent = lower_new_dir_dentry;
> rd.new_dentry = lower_new_dentry;
> rc = vfs_rename(&rd);
> diff --git a/fs/namei.c b/fs/namei.c
> index cd43ff89fbaa..1c80445693d4 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -5024,20 +5024,20 @@ int vfs_rename(struct renamedata *rd)
> if (source == target)
> return 0;
>
> - error = may_delete(rd->old_mnt_idmap, old_dir, old_dentry, is_dir);
> + error = may_delete(rd->mnt_idmap, old_dir, old_dentry, is_dir);
> if (error)
> return error;
>
> if (!target) {
> - error = may_create(rd->new_mnt_idmap, new_dir, new_dentry);
> + error = may_create(rd->mnt_idmap, new_dir, new_dentry);
> } else {
> new_is_dir = d_is_dir(new_dentry);
>
> if (!(flags & RENAME_EXCHANGE))
> - error = may_delete(rd->new_mnt_idmap, new_dir,
> + error = may_delete(rd->mnt_idmap, new_dir,
> new_dentry, is_dir);
> else
> - error = may_delete(rd->new_mnt_idmap, new_dir,
> + error = may_delete(rd->mnt_idmap, new_dir,
> new_dentry, new_is_dir);
> }
> if (error)
> @@ -5052,13 +5052,13 @@ int vfs_rename(struct renamedata *rd)
> */
> if (new_dir != old_dir) {
> if (is_dir) {
> - error = inode_permission(rd->old_mnt_idmap, source,
> + error = inode_permission(rd->mnt_idmap, source,
> MAY_WRITE);
> if (error)
> return error;
> }
> if ((flags & RENAME_EXCHANGE) && new_is_dir) {
> - error = inode_permission(rd->new_mnt_idmap, target,
> + error = inode_permission(rd->mnt_idmap, target,
> MAY_WRITE);
> if (error)
> return error;
> @@ -5126,7 +5126,7 @@ int vfs_rename(struct renamedata *rd)
> if (error)
> goto out;
> }
> - error = old_dir->i_op->rename(rd->new_mnt_idmap, old_dir, old_dentry,
> + error = old_dir->i_op->rename(rd->mnt_idmap, old_dir, old_dentry,
> new_dir, new_dentry, flags);
> if (error)
> goto out;
> @@ -5269,10 +5269,9 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
>
> rd.old_parent = old_path.dentry;
> rd.old_dentry = old_dentry;
> - rd.old_mnt_idmap = mnt_idmap(old_path.mnt);
> + rd.mnt_idmap = mnt_idmap(old_path.mnt);
> rd.new_parent = new_path.dentry;
> rd.new_dentry = new_dentry;
> - rd.new_mnt_idmap = mnt_idmap(new_path.mnt);
> rd.delegated_inode = &delegated_inode;
> rd.flags = flags;
> error = vfs_rename(&rd);
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 7d522e426b2d..a21940cadede 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1940,10 +1940,9 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> goto out_dput_old;
> } else {
> struct renamedata rd = {
> - .old_mnt_idmap = &nop_mnt_idmap,
> + .mnt_idmap = &nop_mnt_idmap,
> .old_parent = fdentry,
> .old_dentry = odentry,
> - .new_mnt_idmap = &nop_mnt_idmap,
> .new_parent = tdentry,
> .new_dentry = ndentry,
> };
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index bb0d7ded8e76..4f84abaa0d68 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -361,10 +361,9 @@ static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir,
> {
> int err;
> struct renamedata rd = {
> - .old_mnt_idmap = ovl_upper_mnt_idmap(ofs),
> + .mnt_idmap = ovl_upper_mnt_idmap(ofs),
> .old_parent = olddir,
> .old_dentry = olddentry,
> - .new_mnt_idmap = ovl_upper_mnt_idmap(ofs),
> .new_parent = newdir,
> .new_dentry = newdentry,
> .flags = flags,
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index 49e731dd0529..bfd62a21e75c 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -770,10 +770,9 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> goto out4;
> }
>
> - rd.old_mnt_idmap = mnt_idmap(old_path->mnt),
> + rd.mnt_idmap = mnt_idmap(old_path->mnt),
> rd.old_parent = old_parent,
> rd.old_dentry = old_child,
> - rd.new_mnt_idmap = mnt_idmap(new_path.mnt),
> rd.new_parent = new_path.dentry,
> rd.new_dentry = new_dentry,
> rd.flags = flags,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 1948b2c828d3..d3e27da8a6aa 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2005,20 +2005,18 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
>
> /**
> * struct renamedata - contains all information required for renaming
> - * @old_mnt_idmap: idmap of the old mount the inode was found from
> + * @mnt_idmap: idmap of the mount in which the rename is happening.
> * @old_parent: parent of source
> * @old_dentry: source
> - * @new_mnt_idmap: idmap of the new mount the inode was found from
> * @new_parent: parent of destination
> * @new_dentry: destination
> * @delegated_inode: returns an inode needing a delegation break
> * @flags: rename flags
> */
> struct renamedata {
> - struct mnt_idmap *old_mnt_idmap;
> + struct mnt_idmap *mnt_idmap;
> struct dentry *old_parent;
> struct dentry *old_dentry;
> - struct mnt_idmap *new_mnt_idmap;
> struct dentry *new_parent;
> struct dentry *new_dentry;
> struct inode **delegated_inode;
Nice cleanup.
Reviewed-by: Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists