Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 93faccbb authored by Eric W. Biederman's avatar Eric W. Biederman
Browse files

fs: Better permission checking for submounts



To support unprivileged users mounting filesystems two permission
checks have to be performed: a test to see if the user allowed to
create a mount in the mount namespace, and a test to see if
the user is allowed to access the specified filesystem.

The automount case is special in that mounting the original filesystem
grants permission to mount the sub-filesystems, to any user who
happens to stumble across the their mountpoint and satisfies the
ordinary filesystem permission checks.

Attempting to handle the automount case by using override_creds
almost works.  It preserves the idea that permission to mount
the original filesystem is permission to mount the sub-filesystem.
Unfortunately using override_creds messes up the filesystems
ordinary permission checks.

Solve this by being explicit that a mount is a submount by introducing
vfs_submount, and using it where appropriate.

vfs_submount uses a new mount internal mount flags MS_SUBMOUNT, to let
sget and friends know that a mount is a submount so they can take appropriate
action.

sget and sget_userns are modified to not perform any permission checks
on submounts.

follow_automount is modified to stop using override_creds as that
has proven problemantic.

do_mount is modified to always remove the new MS_SUBMOUNT flag so
that we know userspace will never by able to specify it.

autofs4 is modified to stop using current_real_cred that was put in
there to handle the previous version of submount permission checking.

cifs is modified to pass the mountpoint all of the way down to vfs_submount.

debugfs is modified to pass the mountpoint all of the way down to
trace_automount by adding a new parameter.  To make this change easier
a new typedef debugfs_automount_t is introduced to capture the type of
the debugfs automount function.

Cc: stable@vger.kernel.org
Fixes: 069d5ac9 ("autofs:  Fix automounts by using current_real_cred()->uid")
Fixes: aeaa4a79 ("fs: Call d_automount with the filesystems creds")
Reviewed-by: default avatarTrond Myklebust <trond.myklebust@primarydata.com>
Reviewed-by: default avatarSeth Forshee <seth.forshee@canonical.com>
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
parent c6c70f44
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -202,7 +202,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt)

	/* try and do the mount */
	_debug("--- attempting mount %s -o %s ---", devname, options);
	mnt = vfs_kern_mount(&afs_fs_type, 0, devname, options);
	mnt = vfs_submount(mntpt, &afs_fs_type, devname, options);
	_debug("--- mount result %p ---", mnt);

	free_page((unsigned long) devname);
+2 −2
Original line number Diff line number Diff line
@@ -436,8 +436,8 @@ int autofs4_wait(struct autofs_sb_info *sbi,
		memcpy(&wq->name, &qstr, sizeof(struct qstr));
		wq->dev = autofs4_get_dev(sbi);
		wq->ino = autofs4_get_ino(sbi);
		wq->uid = current_real_cred()->uid;
		wq->gid = current_real_cred()->gid;
		wq->uid = current_cred()->uid;
		wq->gid = current_cred()->gid;
		wq->pid = pid;
		wq->tgid = tgid;
		wq->status = -EINTR; /* Status return if interrupted */
+4 −3
Original line number Diff line number Diff line
@@ -245,7 +245,8 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
 * @fullpath:		full path in UNC format
 * @ref:		server's referral
 */
static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb,
static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt,
		struct cifs_sb_info *cifs_sb,
		const char *fullpath, const struct dfs_info3_param *ref)
{
	struct vfsmount *mnt;
@@ -259,7 +260,7 @@ static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb,
	if (IS_ERR(mountdata))
		return (struct vfsmount *)mountdata;

	mnt = vfs_kern_mount(&cifs_fs_type, 0, devname, mountdata);
	mnt = vfs_submount(mntpt, &cifs_fs_type, devname, mountdata);
	kfree(mountdata);
	kfree(devname);
	return mnt;
@@ -334,7 +335,7 @@ static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt)
			mnt = ERR_PTR(-EINVAL);
			break;
		}
		mnt = cifs_dfs_do_refmount(cifs_sb,
		mnt = cifs_dfs_do_refmount(mntpt, cifs_sb,
				full_path, referrals + i);
		cifs_dbg(FYI, "%s: cifs_dfs_do_refmount:%s , mnt:%p\n",
			 __func__, referrals[i].node_name, mnt);
+4 −4
Original line number Diff line number Diff line
@@ -187,9 +187,9 @@ static const struct super_operations debugfs_super_operations = {

static struct vfsmount *debugfs_automount(struct path *path)
{
	struct vfsmount *(*f)(void *);
	f = (struct vfsmount *(*)(void *))path->dentry->d_fsdata;
	return f(d_inode(path->dentry)->i_private);
	debugfs_automount_t f;
	f = (debugfs_automount_t)path->dentry->d_fsdata;
	return f(path->dentry, d_inode(path->dentry)->i_private);
}

static const struct dentry_operations debugfs_dops = {
@@ -504,7 +504,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir);
 */
struct dentry *debugfs_create_automount(const char *name,
					struct dentry *parent,
					struct vfsmount *(*f)(void *),
					debugfs_automount_t f,
					void *data)
{
	struct dentry *dentry = start_creating(name, parent);
+0 −3
Original line number Diff line number Diff line
@@ -1100,7 +1100,6 @@ static int follow_automount(struct path *path, struct nameidata *nd,
			    bool *need_mntput)
{
	struct vfsmount *mnt;
	const struct cred *old_cred;
	int err;

	if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
@@ -1129,9 +1128,7 @@ static int follow_automount(struct path *path, struct nameidata *nd,
	if (nd->total_link_count >= 40)
		return -ELOOP;

	old_cred = override_creds(&init_cred);
	mnt = path->dentry->d_op->d_automount(path);
	revert_creds(old_cred);
	if (IS_ERR(mnt)) {
		/*
		 * The filesystem is allowed to return -EISDIR here to indicate
Loading