| 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: <wqp4ruxfzv47xwz2fca5trvpwg7rxufvd3nlfiu5kfsasqzsih@lutnvxe4ri62>
Date: Thu, 5 Jun 2025 13:19:54 +0200
From: Jan Kara <jack@...e.cz>
To: Jeff Layton <jlayton@...nel.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Chuck Lever <chuck.lever@...cle.com>,
Alexander Aring <alex.aring@...il.com>, Trond Myklebust <trondmy@...nel.org>,
Anna Schumaker <anna@...nel.org>, Steve French <sfrench@...ba.org>,
Paulo Alcantara <pc@...guebit.com>, Ronnie Sahlberg <ronniesahlberg@...il.com>,
Shyam Prasad N <sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>,
Bharath SM <bharathsm@...rosoft.com>, NeilBrown <neil@...wn.name>,
Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>, Jonathan Corbet <corbet@....net>,
Amir Goldstein <amir73il@...il.com>, Miklos Szeredi <miklos@...redi.hu>,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org,
linux-cifs@...r.kernel.org, samba-technical@...ts.samba.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH RFC v2 04/28] vfs: allow mkdir to wait for delegation
break on parent
On Mon 02-06-25 10:01:47, Jeff Layton wrote:
> In order to add directory delegation support, we need to break
> delegations on the parent whenever there is going to be a change in the
> directory.
>
> Rename the existing vfs_mkdir to __vfs_mkdir, make it static and add a
> new delegated_inode parameter. Add a new exported vfs_mkdir wrapper
> around it that passes a NULL pointer for delegated_inode.
>
> Signed-off-by: Jeff Layton <jlayton@...nel.org>
FWIW I went through the changes adding breaking of delegations to VFS
directory functions and they look ok to me. Just I dislike the addition of
__vfs_mkdir() (and similar) helpers because over longer term the helpers
tend to pile up and the maze of functions (already hard to follow in VFS)
gets unwieldy. Either I'd try to give it a proper name or (if exposing the
functionality to the external world is fine - which seems it is) you could
just add the argument to vfs_mkdir() and change all the callers? I've
checked and for each of the modified functions there's less than 10 callers
so the churn shouldn't be that big. What do others think?
Honza
> ---
> fs/namei.c | 67 +++++++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 42 insertions(+), 25 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 0fea12860036162c01a291558e068fde9c986142..7c9e237ed1b1a535934ffe5e523424bb035e7ae0 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4318,29 +4318,9 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
> return do_mknodat(AT_FDCWD, getname(filename), mode, dev);
> }
>
> -/**
> - * vfs_mkdir - create directory returning correct dentry if possible
> - * @idmap: idmap of the mount the inode was found from
> - * @dir: inode of the parent directory
> - * @dentry: dentry of the child directory
> - * @mode: mode of the child directory
> - *
> - * Create a directory.
> - *
> - * If the inode has been found through an idmapped mount the idmap of
> - * the vfsmount must be passed through @idmap. This function will then take
> - * care to map the inode according to @idmap before checking permissions.
> - * On non-idmapped mounts or if permission checking is to be performed on the
> - * raw inode simply pass @nop_mnt_idmap.
> - *
> - * In the event that the filesystem does not use the *@...try but leaves it
> - * negative or unhashes it and possibly splices a different one returning it,
> - * the original dentry is dput() and the alternate is returned.
> - *
> - * In case of an error the dentry is dput() and an ERR_PTR() is returned.
> - */
> -struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> - struct dentry *dentry, umode_t mode)
> +static struct dentry *__vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> + struct dentry *dentry, umode_t mode,
> + struct inode **delegated_inode)
> {
> int error;
> unsigned max_links = dir->i_sb->s_max_links;
> @@ -4363,6 +4343,10 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> if (max_links && dir->i_nlink >= max_links)
> goto err;
>
> + error = try_break_deleg(dir, delegated_inode);
> + if (error)
> + goto err;
> +
> de = dir->i_op->mkdir(idmap, dir, dentry, mode);
> error = PTR_ERR(de);
> if (IS_ERR(de))
> @@ -4378,6 +4362,33 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> dput(dentry);
> return ERR_PTR(error);
> }
> +
> +/**
> + * vfs_mkdir - create directory returning correct dentry if possible
> + * @idmap: idmap of the mount the inode was found from
> + * @dir: inode of the parent directory
> + * @dentry: dentry of the child directory
> + * @mode: mode of the child directory
> + *
> + * Create a directory.
> + *
> + * If the inode has been found through an idmapped mount the idmap of
> + * the vfsmount must be passed through @idmap. This function will then take
> + * care to map the inode according to @idmap before checking permissions.
> + * On non-idmapped mounts or if permission checking is to be performed on the
> + * raw inode simply pass @nop_mnt_idmap.
> + *
> + * In the event that the filesystem does not use the *@...try but leaves it
> + * negative or unhashes it and possibly splices a different one returning it,
> + * the original dentry is dput() and the alternate is returned.
> + *
> + * In case of an error the dentry is dput() and an ERR_PTR() is returned.
> + */
> +struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> + struct dentry *dentry, umode_t mode)
> +{
> + return __vfs_mkdir(idmap, dir, dentry, mode, NULL);
> +}
> EXPORT_SYMBOL(vfs_mkdir);
>
> int do_mkdirat(int dfd, struct filename *name, umode_t mode)
> @@ -4386,6 +4397,7 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode)
> struct path path;
> int error;
> unsigned int lookup_flags = LOOKUP_DIRECTORY;
> + struct inode *delegated_inode = NULL;
>
> retry:
> dentry = filename_create(dfd, name, &path, lookup_flags);
> @@ -4396,12 +4408,17 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode)
> error = security_path_mkdir(&path, dentry,
> mode_strip_umask(path.dentry->d_inode, mode));
> if (!error) {
> - dentry = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
> - dentry, mode);
> + dentry = __vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
> + dentry, mode, &delegated_inode);
> if (IS_ERR(dentry))
> error = PTR_ERR(dentry);
> }
> done_path_create(&path, dentry);
> + if (delegated_inode) {
> + error = break_deleg_wait(&delegated_inode);
> + if (!error)
> + goto retry;
> + }
> if (retry_estale(error, lookup_flags)) {
> lookup_flags |= LOOKUP_REVAL;
> goto retry;
>
> --
> 2.49.0
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists