Skip to content

Commit 93faccb

Browse files
ebiedermebiederm
committed
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: [email protected] Fixes: 069d5ac ("autofs: Fix automounts by using current_real_cred()->uid") Fixes: aeaa4a7 ("fs: Call d_automount with the filesystems creds") Reviewed-by: Trond Myklebust <[email protected]> Reviewed-by: Seth Forshee <[email protected]> Signed-off-by: "Eric W. Biederman" <[email protected]>
1 parent c6c70f4 commit 93faccb

13 files changed

Lines changed: 47 additions & 22 deletions

File tree

‎fs/afs/mntpt.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt)
202202

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

208208
free_page((unsigned long) devname);

‎fs/autofs4/waitq.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,8 +436,8 @@ int autofs4_wait(struct autofs_sb_info *sbi,
436436
memcpy(&wq->name, &qstr, sizeof(struct qstr));
437437
wq->dev = autofs4_get_dev(sbi);
438438
wq->ino = autofs4_get_ino(sbi);
439-
wq->uid = current_real_cred()->uid;
440-
wq->gid = current_real_cred()->gid;
439+
wq->uid = current_cred()->uid;
440+
wq->gid = current_cred()->gid;
441441
wq->pid = pid;
442442
wq->tgid = tgid;
443443
wq->status = -EINTR; /* Status return if interrupted */

‎fs/cifs/cifs_dfs_ref.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,8 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
245245
* @fullpath: full path in UNC format
246246
* @ref: server's referral
247247
*/
248-
static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb,
248+
static struct vfsmount *cifs_dfs_do_refmount(struct dentry *mntpt,
249+
struct cifs_sb_info *cifs_sb,
249250
const char *fullpath, const struct dfs_info3_param *ref)
250251
{
251252
struct vfsmount *mnt;
@@ -259,7 +260,7 @@ static struct vfsmount *cifs_dfs_do_refmount(struct cifs_sb_info *cifs_sb,
259260
if (IS_ERR(mountdata))
260261
return (struct vfsmount *)mountdata;
261262

262-
mnt = vfs_kern_mount(&cifs_fs_type, 0, devname, mountdata);
263+
mnt = vfs_submount(mntpt, &cifs_fs_type, devname, mountdata);
263264
kfree(mountdata);
264265
kfree(devname);
265266
return mnt;
@@ -334,7 +335,7 @@ static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt)
334335
mnt = ERR_PTR(-EINVAL);
335336
break;
336337
}
337-
mnt = cifs_dfs_do_refmount(cifs_sb,
338+
mnt = cifs_dfs_do_refmount(mntpt, cifs_sb,
338339
full_path, referrals + i);
339340
cifs_dbg(FYI, "%s: cifs_dfs_do_refmount:%s , mnt:%p\n",
340341
__func__, referrals[i].node_name, mnt);

‎fs/debugfs/inode.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,9 @@ static const struct super_operations debugfs_super_operations = {
187187

188188
static struct vfsmount *debugfs_automount(struct path *path)
189189
{
190-
struct vfsmount *(*f)(void *);
191-
f = (struct vfsmount *(*)(void *))path->dentry->d_fsdata;
192-
return f(d_inode(path->dentry)->i_private);
190+
debugfs_automount_t f;
191+
f = (debugfs_automount_t)path->dentry->d_fsdata;
192+
return f(path->dentry, d_inode(path->dentry)->i_private);
193193
}
194194

195195
static const struct dentry_operations debugfs_dops = {
@@ -504,7 +504,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir);
504504
*/
505505
struct dentry *debugfs_create_automount(const char *name,
506506
struct dentry *parent,
507-
struct vfsmount *(*f)(void *),
507+
debugfs_automount_t f,
508508
void *data)
509509
{
510510
struct dentry *dentry = start_creating(name, parent);

‎fs/namei.c‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,6 @@ static int follow_automount(struct path *path, struct nameidata *nd,
11001100
bool *need_mntput)
11011101
{
11021102
struct vfsmount *mnt;
1103-
const struct cred *old_cred;
11041103
int err;
11051104

11061105
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,
11291128
if (nd->total_link_count >= 40)
11301129
return -ELOOP;
11311130

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

‎fs/namespace.c‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,21 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
989989
}
990990
EXPORT_SYMBOL_GPL(vfs_kern_mount);
991991

992+
struct vfsmount *
993+
vfs_submount(const struct dentry *mountpoint, struct file_system_type *type,
994+
const char *name, void *data)
995+
{
996+
/* Until it is worked out how to pass the user namespace
997+
* through from the parent mount to the submount don't support
998+
* unprivileged mounts with submounts.
999+
*/
1000+
if (mountpoint->d_sb->s_user_ns != &init_user_ns)
1001+
return ERR_PTR(-EPERM);
1002+
1003+
return vfs_kern_mount(type, MS_SUBMOUNT, name, data);
1004+
}
1005+
EXPORT_SYMBOL_GPL(vfs_submount);
1006+
9921007
static struct mount *clone_mnt(struct mount *old, struct dentry *root,
9931008
int flag)
9941009
{
@@ -2794,7 +2809,7 @@ long do_mount(const char *dev_name, const char __user *dir_name,
27942809

27952810
flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_BORN |
27962811
MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT |
2797-
MS_STRICTATIME | MS_NOREMOTELOCK);
2812+
MS_STRICTATIME | MS_NOREMOTELOCK | MS_SUBMOUNT);
27982813

27992814
if (flags & MS_REMOUNT)
28002815
retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,

‎fs/nfs/namespace.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static struct vfsmount *nfs_do_clone_mount(struct nfs_server *server,
226226
const char *devname,
227227
struct nfs_clone_mount *mountdata)
228228
{
229-
return vfs_kern_mount(&nfs_xdev_fs_type, 0, devname, mountdata);
229+
return vfs_submount(mountdata->dentry, &nfs_xdev_fs_type, devname, mountdata);
230230
}
231231

232232
/**

‎fs/nfs/nfs4namespace.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
279279
mountdata->hostname,
280280
mountdata->mnt_path);
281281

282-
mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
282+
mnt = vfs_submount(mountdata->dentry, &nfs4_referral_fs_type, page, mountdata);
283283
if (!IS_ERR(mnt))
284284
break;
285285
}

‎fs/super.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ struct super_block *sget_userns(struct file_system_type *type,
469469
struct super_block *old;
470470
int err;
471471

472-
if (!(flags & MS_KERNMOUNT) &&
472+
if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT)) &&
473473
!(type->fs_flags & FS_USERNS_MOUNT) &&
474474
!capable(CAP_SYS_ADMIN))
475475
return ERR_PTR(-EPERM);
@@ -499,7 +499,7 @@ struct super_block *sget_userns(struct file_system_type *type,
499499
}
500500
if (!s) {
501501
spin_unlock(&sb_lock);
502-
s = alloc_super(type, flags, user_ns);
502+
s = alloc_super(type, (flags & ~MS_SUBMOUNT), user_ns);
503503
if (!s)
504504
return ERR_PTR(-ENOMEM);
505505
goto retry;
@@ -540,8 +540,15 @@ struct super_block *sget(struct file_system_type *type,
540540
{
541541
struct user_namespace *user_ns = current_user_ns();
542542

543+
/* We don't yet pass the user namespace of the parent
544+
* mount through to here so always use &init_user_ns
545+
* until that changes.
546+
*/
547+
if (flags & MS_SUBMOUNT)
548+
user_ns = &init_user_ns;
549+
543550
/* Ensure the requestor has permissions over the target filesystem */
544-
if (!(flags & MS_KERNMOUNT) && !ns_capable(user_ns, CAP_SYS_ADMIN))
551+
if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT)) && !ns_capable(user_ns, CAP_SYS_ADMIN))
545552
return ERR_PTR(-EPERM);
546553

547554
return sget_userns(type, test, set, flags, user_ns, data);

‎include/linux/debugfs.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,10 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent);
9797
struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
9898
const char *dest);
9999

100+
typedef struct vfsmount *(*debugfs_automount_t)(struct dentry *, void *);
100101
struct dentry *debugfs_create_automount(const char *name,
101102
struct dentry *parent,
102-
struct vfsmount *(*f)(void *),
103+
debugfs_automount_t f,
103104
void *data);
104105

105106
void debugfs_remove(struct dentry *dentry);

0 commit comments

Comments
 (0)