Unverified Commit c9ba789d authored by NeilBrown's avatar NeilBrown Committed by Christian Brauner
Browse files

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: default avatarAmir Goldstein <amir73il@gmail.com>
Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarNeilBrown <neil@brown.name>

--
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: default avatar <syzbot@syzkaller.appspotmail.com>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent bd6ede8a

fs/fuse/dir.c

+8 −11
Original line number Diff line number Diff line
@@ -1397,27 +1397,25 @@ int fuse_reverse_inval_entry(struct fuse_conn *fc, u64 parent_nodeid, 
	if (!parent)

		return -ENOENT;



	inode_lock_nested(parent, I_MUTEX_PARENT);

	if (!S_ISDIR(parent->i_mode))

		goto unlock;

		goto put_parent;



	err = -ENOENT;

	dir = d_find_alias(parent);

	if (!dir)

		goto unlock;

		goto put_parent;



	name->hash = full_name_hash(dir, name->name, name->len);

	entry = d_lookup(dir, name);

	entry = start_removing_noperm(dir, name);

	dput(dir);

	if (!entry)

		goto unlock;

	if (IS_ERR(entry))

		goto put_parent;



	fuse_dir_changed(parent);

	if (!(flags & FUSE_EXPIRE_ONLY))

		d_invalidate(entry);

	fuse_invalidate_entry_cache(entry);



	if (child_nodeid != 0 && d_really_is_positive(entry)) {

	if (child_nodeid != 0) {

		inode_lock(d_inode(entry));

		if (get_node_id(d_inode(entry)) != child_nodeid) {

			err = -ENOENT;
@@ -1445,10 +1443,9 @@ int fuse_reverse_inval_entry(struct fuse_conn *fc, u64 parent_nodeid, 
	} else {

		err = 0;

	}

	dput(entry);



 unlock:

	inode_unlock(parent);

	end_removing(entry);

 put_parent:

	iput(parent);

	return err;

}

fs/namei.c

+48 −0
Original line number Diff line number Diff line
@@ -3275,6 +3275,54 @@ struct dentry *start_removing(struct mnt_idmap *idmap, struct dentry *parent, 
}

EXPORT_SYMBOL(start_removing);



/**

 * start_creating_noperm - prepare to create a given name without permission checking

 * @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.

 *

 * If the name already exists, a positive dentry is returned.

 *

 * Returns: a negative or positive dentry, or an error.

 */

struct dentry *start_creating_noperm(struct dentry *parent,

				     struct qstr *name)

{

	int err = lookup_noperm_common(name, parent);



	if (err)

		return ERR_PTR(err);

	return start_dirop(parent, name, LOOKUP_CREATE);

}

EXPORT_SYMBOL(start_creating_noperm);



/**

 * start_removing_noperm - prepare to remove a given name without permission checking

 * @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.

 *

 * If the name doesn't exist, an error is returned.

 *

 * end_removing() should be called when removal is complete, or aborted.

 *

 * Returns: a positive dentry, or an error.

 */

struct dentry *start_removing_noperm(struct dentry *parent,

				     struct qstr *name)

{

	int err = lookup_noperm_common(name, parent);



	if (err)

		return ERR_PTR(err);

	return start_dirop(parent, name, 0);

}

EXPORT_SYMBOL(start_removing_noperm);



#ifdef CONFIG_UNIX98_PTYS

int path_pts(struct path *path)

{

fs/xfs/scrub/orphanage.c

+4 −7
Original line number Diff line number Diff line
@@ -152,11 +152,10 @@ xrep_orphanage_create( 
	}



	/* Try to find the orphanage directory. */

	inode_lock_nested(root_inode, I_MUTEX_PARENT);

	orphanage_dentry = lookup_noperm(&QSTR(ORPHANAGE), root_dentry);

	orphanage_dentry = start_creating_noperm(root_dentry, &QSTR(ORPHANAGE));

	if (IS_ERR(orphanage_dentry)) {

		error = PTR_ERR(orphanage_dentry);

		goto out_unlock_root;

		goto out_dput_root;

	}



	/*
@@ -170,7 +169,7 @@ xrep_orphanage_create( 
					     orphanage_dentry, 0750);

		error = PTR_ERR(orphanage_dentry);

		if (IS_ERR(orphanage_dentry))

			goto out_unlock_root;

			goto out_dput_orphanage;

	}



	/* Not a directory? Bail out. */
@@ -200,9 +199,7 @@ xrep_orphanage_create( 
	sc->orphanage_ilock_flags = 0;



out_dput_orphanage:

	dput(orphanage_dentry);

out_unlock_root:

	inode_unlock(VFS_I(sc->mp->m_rootip));

	end_creating(orphanage_dentry, root_dentry);

out_dput_root:

	dput(root_dentry);

out:

include/linux/namei.h

+2 −0
Original line number Diff line number Diff line
@@ -92,6 +92,8 @@ 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_noperm(struct dentry *parent, struct qstr *name);

struct dentry *start_removing_noperm(struct dentry *parent, struct qstr *name);



/**

 * end_creating - finish action started with start_creating

ipc/mqueue.c

+12 −20
Original line number Diff line number Diff line
@@ -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),

	err = vfs_unlink(&nop_mnt_idmap, d_inode(mnt->mnt_root),

			 dentry, NULL);

	}

	dput(dentry);



out_unlock:

	inode_unlock(d_inode(mnt->mnt_root));

	end_removing(dentry);

	iput(inode);



out_drop_write:

	mnt_drop_write(mnt);

out_name:

	putname(name);