From c9ba789dad15ba65662bba17595c0aeaa0cfcf1c Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Thu, 13 Nov 2025 11:18:29 +1100 Subject: VFS: introduce start_creating_noperm() and start_removing_noperm() xfs, fuse, ipc/mqueue need variants of start_creating or start_removing which do not check permissions. This patch adds _noperm versions of these functions. Note that do_mq_open() was only calling mntget() so it could call path_put() - it didn't really need an extra reference on the mnt. Now it doesn't call mntget() and uses end_creating() which does the dput() half of path_put(). Also mq_unlink() previously passed d_inode(dentry->d_parent) as the dir inode to vfs_unlink(). This is after locking d_inode(mnt->mnt_root) These two inodes are the same, but normally calls use the textual parent. So I've changes the vfs_unlink() call to be given d_inode(mnt->mnt_root). Reviewed-by: Amir Goldstein Reviewed-by: Jeff Layton Signed-off-by: NeilBrown -- changes since v2: - dir arg passed to vfs_unlink() in mq_unlink() changed to match the dir passed to lookup_noperm() - restore assignment to path->mnt even though the mntget() is removed. Link: https://patch.msgid.link/20251113002050.676694-7-neilb@ownmail.net Tested-by: syzbot@syzkaller.appspotmail.com Signed-off-by: Christian Brauner --- ipc/mqueue.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) (limited to 'ipc') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 093551fe66a7..6d7610310003 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -913,13 +913,12 @@ static int do_mq_open(const char __user *u_name, int oflag, umode_t mode, goto out_putname; ro = mnt_want_write(mnt); /* we'll drop it in any case */ - inode_lock(d_inode(root)); - path.dentry = lookup_noperm(&QSTR(name->name), root); + path.dentry = start_creating_noperm(root, &QSTR(name->name)); if (IS_ERR(path.dentry)) { error = PTR_ERR(path.dentry); goto out_putfd; } - path.mnt = mntget(mnt); + path.mnt = mnt; error = prepare_open(path.dentry, oflag, ro, mode, name, attr); if (!error) { struct file *file = dentry_open(&path, oflag, current_cred()); @@ -928,13 +927,12 @@ static int do_mq_open(const char __user *u_name, int oflag, umode_t mode, else error = PTR_ERR(file); } - path_put(&path); out_putfd: if (error) { put_unused_fd(fd); fd = error; } - inode_unlock(d_inode(root)); + end_creating(path.dentry, root); if (!ro) mnt_drop_write(mnt); out_putname: @@ -957,7 +955,7 @@ SYSCALL_DEFINE1(mq_unlink, const char __user *, u_name) int err; struct filename *name; struct dentry *dentry; - struct inode *inode = NULL; + struct inode *inode; struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns; struct vfsmount *mnt = ipc_ns->mq_mnt; @@ -969,26 +967,20 @@ SYSCALL_DEFINE1(mq_unlink, const char __user *, u_name) err = mnt_want_write(mnt); if (err) goto out_name; - inode_lock_nested(d_inode(mnt->mnt_root), I_MUTEX_PARENT); - dentry = lookup_noperm(&QSTR(name->name), mnt->mnt_root); + dentry = start_removing_noperm(mnt->mnt_root, &QSTR(name->name)); if (IS_ERR(dentry)) { err = PTR_ERR(dentry); - goto out_unlock; + goto out_drop_write; } inode = d_inode(dentry); - if (!inode) { - err = -ENOENT; - } else { - ihold(inode); - err = vfs_unlink(&nop_mnt_idmap, d_inode(dentry->d_parent), - dentry, NULL); - } - dput(dentry); - -out_unlock: - inode_unlock(d_inode(mnt->mnt_root)); + ihold(inode); + err = vfs_unlink(&nop_mnt_idmap, d_inode(mnt->mnt_root), + dentry, NULL); + end_removing(dentry); iput(inode); + +out_drop_write: mnt_drop_write(mnt); out_name: putname(name); -- cgit v1.2.3 From fe497f0759e0efb949f9480911d00b6045c21f50 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Thu, 13 Nov 2025 11:18:37 +1100 Subject: VFS: change vfs_mkdir() to unlock on failure. vfs_mkdir() already drops the reference to the dentry on failure but it leaves the parent locked. This complicates end_creating() which needs to unlock the parent even though the dentry is no longer available. If we change vfs_mkdir() to unlock on failure as well as releasing the dentry, we can remove the "parent" arg from end_creating() and simplify the rules for calling it. Note that cachefiles_get_directory() can choose to substitute an error instead of actually calling vfs_mkdir(), for fault injection. In that case it needs to call end_creating(), just as vfs_mkdir() now does on error. ovl_create_real() will now unlock on error. So the conditional end_creating() after the call is removed, and end_creating() is called internally on error. Reviewed-by: Amir Goldstein Reviewed-by: Jeff Layton Tested-by: syzbot@syzkaller.appspotmail.com Signed-off-by: NeilBrown Link: https://patch.msgid.link/20251113002050.676694-15-neilb@ownmail.net Signed-off-by: Christian Brauner --- ipc/mqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'ipc') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 6d7610310003..83d9466710d6 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -932,7 +932,7 @@ out_putfd: put_unused_fd(fd); fd = error; } - end_creating(path.dentry, root); + end_creating(path.dentry); if (!ro) mnt_drop_write(mnt); out_putname: -- cgit v1.2.3