| 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: <4a5c4bf1a299f487c4f97e0054293cbeff139fa7.camel@kernel.org>
Date: Wed, 12 Nov 2025 10:01:49 -0500
From: Jeff Layton <jlayton@...nel.org>
To: NeilBrown <neil@...wn.name>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner
<brauner@...nel.org>, Amir Goldstein <amir73il@...il.com>
Cc: Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, Chris Mason
<clm@...com>, David Sterba <dsterba@...e.com>, David Howells
<dhowells@...hat.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>, Danilo Krummrich
<dakr@...nel.org>, Tyler Hicks <code@...icks.com>, Miklos Szeredi
<miklos@...redi.hu>, Chuck Lever <chuck.lever@...cle.com>, Olga
Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>, Namjae
Jeon <linkinjeon@...nel.org>, Steve French <smfrench@...il.com>, Sergey
Senozhatsky <senozhatsky@...omium.org>, Carlos Maiolino <cem@...nel.org>,
John Johansen <john.johansen@...onical.com>, Paul Moore
<paul@...l-moore.com>, James Morris <jmorris@...ei.org>, "Serge E. Hallyn"
<serge@...lyn.com>, Stephen Smalley <stephen.smalley.work@...il.com>,
Ondrej Mosnacek <omosnace@...hat.com>, Mateusz Guzik <mjguzik@...il.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Stefan Berger
<stefanb@...ux.ibm.com>, "Darrick J. Wong" <djwong@...nel.org>,
linux-kernel@...r.kernel.org, netfs@...ts.linux.dev,
ecryptfs@...r.kernel.org, linux-nfs@...r.kernel.org,
linux-unionfs@...r.kernel.org, linux-cifs@...r.kernel.org,
linux-xfs@...r.kernel.org, linux-security-module@...r.kernel.org,
selinux@...r.kernel.org
Subject: Re: [PATCH v5 08/14] VFS: add start_creating_killable() and
start_removing_killable()
On Thu, 2025-11-06 at 11:50 +1100, NeilBrown wrote:
> From: NeilBrown <neil@...wn.name>
>
> These are similar to start_creating() and start_removing(), but allow a
> fatal signal to abort waiting for the lock.
>
> They are used in btrfs for subvol creation and removal.
>
> btrfs_may_create() no longer needs IS_DEADDIR() and
> start_creating_killable() includes that check.
>
> Reviewed-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: NeilBrown <neil@...wn.name>
> ---
> fs/btrfs/ioctl.c | 41 +++++++---------------
> fs/namei.c | 80 +++++++++++++++++++++++++++++++++++++++++--
> include/linux/namei.h | 6 ++++
> 3 files changed, 95 insertions(+), 32 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 8cb7d5a462ef..d0c3bb0423bb 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -904,14 +904,9 @@ static noinline int btrfs_mksubvol(struct dentry *parent,
> struct fscrypt_str name_str = FSTR_INIT((char *)qname->name, qname->len);
> int ret;
>
> - ret = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);
> - if (ret == -EINTR)
> - return ret;
> -
> - dentry = lookup_one(idmap, qname, parent);
> - ret = PTR_ERR(dentry);
> + dentry = start_creating_killable(idmap, parent, qname);
> if (IS_ERR(dentry))
> - goto out_unlock;
> + return PTR_ERR(dentry);
>
> ret = btrfs_may_create(idmap, dir, dentry);
> if (ret)
> @@ -940,9 +935,7 @@ static noinline int btrfs_mksubvol(struct dentry *parent,
> out_up_read:
> up_read(&fs_info->subvol_sem);
> out_dput:
> - dput(dentry);
> -out_unlock:
> - btrfs_inode_unlock(BTRFS_I(dir), 0);
> + end_creating(dentry, parent);
> return ret;
> }
>
> @@ -2417,18 +2410,10 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
> goto free_subvol_name;
> }
>
> - ret = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);
> - if (ret == -EINTR)
> - goto free_subvol_name;
> - dentry = lookup_one(idmap, &QSTR(subvol_name), parent);
> + dentry = start_removing_killable(idmap, parent, &QSTR(subvol_name));
> if (IS_ERR(dentry)) {
> ret = PTR_ERR(dentry);
> - goto out_unlock_dir;
> - }
> -
> - if (d_really_is_negative(dentry)) {
> - ret = -ENOENT;
> - goto out_dput;
> + goto out_end_removing;
> }
>
> inode = d_inode(dentry);
> @@ -2449,7 +2434,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
> */
> ret = -EPERM;
> if (!btrfs_test_opt(fs_info, USER_SUBVOL_RM_ALLOWED))
> - goto out_dput;
> + goto out_end_removing;
>
> /*
> * Do not allow deletion if the parent dir is the same
> @@ -2460,21 +2445,21 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
> */
> ret = -EINVAL;
> if (root == dest)
> - goto out_dput;
> + goto out_end_removing;
>
> ret = inode_permission(idmap, inode, MAY_WRITE | MAY_EXEC);
> if (ret)
> - goto out_dput;
> + goto out_end_removing;
> }
>
> /* check if subvolume may be deleted by a user */
> ret = btrfs_may_delete(idmap, dir, dentry, 1);
> if (ret)
> - goto out_dput;
> + goto out_end_removing;
>
> if (btrfs_ino(BTRFS_I(inode)) != BTRFS_FIRST_FREE_OBJECTID) {
> ret = -EINVAL;
> - goto out_dput;
> + goto out_end_removing;
> }
>
> btrfs_inode_lock(BTRFS_I(inode), 0);
> @@ -2483,10 +2468,8 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
> if (!ret)
> d_delete_notify(dir, dentry);
>
> -out_dput:
> - dput(dentry);
> -out_unlock_dir:
> - btrfs_inode_unlock(BTRFS_I(dir), 0);
> +out_end_removing:
> + end_removing(dentry);
> free_subvol_name:
> kfree(subvol_name_ptr);
> free_parent:
> diff --git a/fs/namei.c b/fs/namei.c
> index 729b42fb143b..e70d056b9543 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2778,19 +2778,33 @@ static int filename_parentat(int dfd, struct filename *name,
> * Returns: a locked dentry, or an error.
> *
> */
> -struct dentry *start_dirop(struct dentry *parent, struct qstr *name,
> - unsigned int lookup_flags)
> +static struct dentry *__start_dirop(struct dentry *parent, struct qstr *name,
> + unsigned int lookup_flags,
> + unsigned int state)
> {
> struct dentry *dentry;
> struct inode *dir = d_inode(parent);
>
> - inode_lock_nested(dir, I_MUTEX_PARENT);
> + if (state == TASK_KILLABLE) {
> + int ret = down_write_killable_nested(&dir->i_rwsem,
> + I_MUTEX_PARENT);
> + if (ret)
> + return ERR_PTR(ret);
> + } else {
> + inode_lock_nested(dir, I_MUTEX_PARENT);
> + }
> dentry = lookup_one_qstr_excl(name, parent, lookup_flags);
> if (IS_ERR(dentry))
> inode_unlock(dir);
> return dentry;
> }
>
> +struct dentry *start_dirop(struct dentry *parent, struct qstr *name,
> + unsigned int lookup_flags)
> +{
> + return __start_dirop(parent, name, lookup_flags, TASK_NORMAL);
> +}
> +
> /**
> * end_dirop - signal completion of a dirop
> * @de: the dentry which was returned by start_dirop or similar.
> @@ -3275,6 +3289,66 @@ struct dentry *start_removing(struct mnt_idmap *idmap, struct dentry *parent,
> }
> EXPORT_SYMBOL(start_removing);
>
> +/**
> + * start_creating_killable - prepare to create a given name with permission checking
> + * @idmap: idmap of the mount
> + * @parent: directory in which to prepare to create the name
> + * @name: the name to be created
> + *
> + * Locks are taken and a lookup in performed prior to creating
> + * an object in a directory. Permission checking (MAY_EXEC) is performed
> + * against @idmap.
> + *
> + * If the name already exists, a positive dentry is returned.
> + *
> + * If a signal is received or was already pending, the function aborts
> + * with -EINTR;
> + *
> + * Returns: a negative or positive dentry, or an error.
> + */
> +struct dentry *start_creating_killable(struct mnt_idmap *idmap,
> + struct dentry *parent,
> + struct qstr *name)
> +{
> + int err = lookup_one_common(idmap, name, parent);
> +
> + if (err)
> + return ERR_PTR(err);
> + return __start_dirop(parent, name, LOOKUP_CREATE, TASK_KILLABLE);
> +}
> +EXPORT_SYMBOL(start_creating_killable);
> +
> +/**
> + * start_removing_killable - prepare to remove a given name with permission checking
> + * @idmap: idmap of the mount
> + * @parent: directory in which to find the name
> + * @name: the name to be removed
> + *
> + * Locks are taken and a lookup in performed prior to removing
> + * an object from a directory. Permission checking (MAY_EXEC) is performed
> + * against @idmap.
> + *
> + * If the name doesn't exist, an error is returned.
> + *
> + * end_removing() should be called when removal is complete, or aborted.
> + *
> + * If a signal is received or was already pending, the function aborts
> + * with -EINTR;
> + *
> + * Returns: a positive dentry, or an error.
> + */
> +struct dentry *start_removing_killable(struct mnt_idmap *idmap,
> + struct dentry *parent,
> + struct qstr *name)
> +{
> + int err = lookup_one_common(idmap, name, parent);
> +
> + if (err)
> + return ERR_PTR(err);
> + return __start_dirop(parent, name, 0, TASK_KILLABLE);
> +}
> +EXPORT_SYMBOL(start_removing_killable);
> +
> /**
> * start_creating_noperm - prepare to create a given name without permission checking
> * @parent: directory in which to prepare to create the name
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index d089e4e8fdd0..196c66156a8a 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -93,6 +93,12 @@ struct dentry *start_creating(struct mnt_idmap *idmap, struct dentry *parent,
> struct qstr *name);
> struct dentry *start_removing(struct mnt_idmap *idmap, struct dentry *parent,
> struct qstr *name);
> +struct dentry *start_creating_killable(struct mnt_idmap *idmap,
> + struct dentry *parent,
> + struct qstr *name);
> +struct dentry *start_removing_killable(struct mnt_idmap *idmap,
> + struct dentry *parent,
> + struct qstr *name);
> struct dentry *start_creating_noperm(struct dentry *parent, struct qstr *name);
> struct dentry *start_removing_noperm(struct dentry *parent, struct qstr *name);
> struct dentry *start_removing_dentry(struct dentry *parent,
Nice. Maybe we can start using the killable versions in more places
this way!
Reviewed-by: Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists