From e262e32d6bde0f77fb0c95d977482fc872c51996 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 1 Nov 2018 23:07:23 +0000 Subject: vfs: Suppress MS_* flag defs within the kernel unless explicitly enabled Only the mount namespace code that implements mount(2) should be using the MS_* flags. Suppress them inside the kernel unless uapi/linux/mount.h is included. Signed-off-by: David Howells Signed-off-by: Al Viro Reviewed-by: David Howells --- fs/namespace.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/namespace.c') diff --git a/fs/namespace.c b/fs/namespace.c index 98d27da43304..6ae784ece25c 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "pnode.h" #include "internal.h" -- cgit From 43f5e655eff7e124d4e484515689cba374ab698e Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 1 Nov 2018 23:07:25 +0000 Subject: vfs: Separate changing mount flags full remount Separate just the changing of mount flags (MS_REMOUNT|MS_BIND) from full remount because the mount data will get parsed with the new fs_context stuff prior to doing a remount - and this causes the syscall to fail under some circumstances. To quote Eric's explanation: [...] mount(..., MS_REMOUNT|MS_BIND, ...) now validates the mount options string, which breaks systemd unit files with ProtectControlGroups=yes (e.g. systemd-networkd.service) when systemd does the following to change a cgroup (v1) mount to read-only: mount(NULL, "/run/systemd/unit-root/sys/fs/cgroup/systemd", NULL, MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_REMOUNT|MS_BIND, NULL) ... when the kernel has CONFIG_CGROUPS=y but no cgroup subsystems enabled, since in that case the error "cgroup1: Need name or subsystem set" is hit when the mount options string is empty. Probably it doesn't make sense to validate the mount options string at all in the MS_REMOUNT|MS_BIND case, though maybe you had something else in mind. This is also worthwhile doing because we will need to add a mount_setattr() syscall to take over the remount-bind function. Reported-by: Eric Biggers Signed-off-by: David Howells Signed-off-by: Al Viro Reviewed-by: David Howells --- fs/namespace.c | 146 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 54 deletions(-) (limited to 'fs/namespace.c') diff --git a/fs/namespace.c b/fs/namespace.c index 6ae784ece25c..08cffdad6665 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -246,13 +246,9 @@ out_free_cache: * mnt_want/drop_write() will _keep_ the filesystem * r/w. */ -int __mnt_is_readonly(struct vfsmount *mnt) +bool __mnt_is_readonly(struct vfsmount *mnt) { - if (mnt->mnt_flags & MNT_READONLY) - return 1; - if (sb_rdonly(mnt->mnt_sb)) - return 1; - return 0; + return (mnt->mnt_flags & MNT_READONLY) || sb_rdonly(mnt->mnt_sb); } EXPORT_SYMBOL_GPL(__mnt_is_readonly); @@ -508,11 +504,12 @@ static int mnt_make_readonly(struct mount *mnt) return ret; } -static void __mnt_unmake_readonly(struct mount *mnt) +static int __mnt_unmake_readonly(struct mount *mnt) { lock_mount_hash(); mnt->mnt.mnt_flags &= ~MNT_READONLY; unlock_mount_hash(); + return 0; } int sb_prepare_remount_readonly(struct super_block *sb) @@ -2204,21 +2201,91 @@ out: return err; } -static int change_mount_flags(struct vfsmount *mnt, int ms_flags) +/* + * Don't allow locked mount flags to be cleared. + * + * No locks need to be held here while testing the various MNT_LOCK + * flags because those flags can never be cleared once they are set. + */ +static bool can_change_locked_flags(struct mount *mnt, unsigned int mnt_flags) +{ + unsigned int fl = mnt->mnt.mnt_flags; + + if ((fl & MNT_LOCK_READONLY) && + !(mnt_flags & MNT_READONLY)) + return false; + + if ((fl & MNT_LOCK_NODEV) && + !(mnt_flags & MNT_NODEV)) + return false; + + if ((fl & MNT_LOCK_NOSUID) && + !(mnt_flags & MNT_NOSUID)) + return false; + + if ((fl & MNT_LOCK_NOEXEC) && + !(mnt_flags & MNT_NOEXEC)) + return false; + + if ((fl & MNT_LOCK_ATIME) && + ((fl & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) + return false; + + return true; +} + +static int change_mount_ro_state(struct mount *mnt, unsigned int mnt_flags) { - int error = 0; - int readonly_request = 0; + bool readonly_request = (mnt_flags & MNT_READONLY); - if (ms_flags & MS_RDONLY) - readonly_request = 1; - if (readonly_request == __mnt_is_readonly(mnt)) + if (readonly_request == __mnt_is_readonly(&mnt->mnt)) return 0; if (readonly_request) - error = mnt_make_readonly(real_mount(mnt)); - else - __mnt_unmake_readonly(real_mount(mnt)); - return error; + return mnt_make_readonly(mnt); + + return __mnt_unmake_readonly(mnt); +} + +/* + * Update the user-settable attributes on a mount. The caller must hold + * sb->s_umount for writing. + */ +static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags) +{ + lock_mount_hash(); + mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK; + mnt->mnt.mnt_flags = mnt_flags; + touch_mnt_namespace(mnt->mnt_ns); + unlock_mount_hash(); +} + +/* + * Handle reconfiguration of the mountpoint only without alteration of the + * superblock it refers to. This is triggered by specifying MS_REMOUNT|MS_BIND + * to mount(2). + */ +static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags) +{ + struct super_block *sb = path->mnt->mnt_sb; + struct mount *mnt = real_mount(path->mnt); + int ret; + + if (!check_mnt(mnt)) + return -EINVAL; + + if (path->dentry != mnt->mnt.mnt_root) + return -EINVAL; + + if (!can_change_locked_flags(mnt, mnt_flags)) + return -EPERM; + + down_write(&sb->s_umount); + ret = change_mount_ro_state(mnt, mnt_flags); + if (ret == 0) + set_mount_attributes(mnt, mnt_flags); + up_write(&sb->s_umount); + return ret; } /* @@ -2239,50 +2306,19 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, if (path->dentry != path->mnt->mnt_root) return -EINVAL; - /* Don't allow changing of locked mnt flags. - * - * No locks need to be held here while testing the various - * MNT_LOCK flags because those flags can never be cleared - * once they are set. - */ - if ((mnt->mnt.mnt_flags & MNT_LOCK_READONLY) && - !(mnt_flags & MNT_READONLY)) { - return -EPERM; - } - if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) && - !(mnt_flags & MNT_NODEV)) { - return -EPERM; - } - if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) && - !(mnt_flags & MNT_NOSUID)) { - return -EPERM; - } - if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) && - !(mnt_flags & MNT_NOEXEC)) { + if (!can_change_locked_flags(mnt, mnt_flags)) return -EPERM; - } - if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) && - ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) { - return -EPERM; - } err = security_sb_remount(sb, data); if (err) return err; down_write(&sb->s_umount); - if (ms_flags & MS_BIND) - err = change_mount_flags(path->mnt, ms_flags); - else if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN)) - err = -EPERM; - else + err = -EPERM; + if (ns_capable(sb->s_user_ns, CAP_SYS_ADMIN)) { err = do_remount_sb(sb, sb_flags, data, 0); - if (!err) { - lock_mount_hash(); - mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK; - mnt->mnt.mnt_flags = mnt_flags; - touch_mnt_namespace(mnt->mnt_ns); - unlock_mount_hash(); + if (!err) + set_mount_attributes(mnt, mnt_flags); } up_write(&sb->s_umount); return err; @@ -2777,7 +2813,9 @@ long do_mount(const char *dev_name, const char __user *dir_name, SB_LAZYTIME | SB_I_VERSION); - if (flags & MS_REMOUNT) + if ((flags & (MS_REMOUNT | MS_BIND)) == (MS_REMOUNT | MS_BIND)) + retval = do_reconfigure_mnt(&path, mnt_flags); + else if (flags & MS_REMOUNT) retval = do_remount(&path, flags, sb_flags, mnt_flags, data_page); else if (flags & MS_BIND) -- cgit From c039bc3c2498724946304a8f964244a9b6af1043 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 1 Dec 2018 23:06:57 -0500 Subject: LSM: lift extracting and parsing LSM options into the caller of ->sb_remount() This paves the way for retaining the LSM options from a common filesystem mount context during a mount parameter parsing phase to be instituted prior to actual mount/reconfiguration actions. Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/namespace.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'fs/namespace.c') diff --git a/fs/namespace.c b/fs/namespace.c index 08cffdad6665..341793fbd390 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2299,6 +2299,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, int err; struct super_block *sb = path->mnt->mnt_sb; struct mount *mnt = real_mount(path->mnt); + struct security_mnt_opts opts; if (!check_mnt(mnt)) return -EINVAL; @@ -2309,7 +2310,23 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, if (!can_change_locked_flags(mnt, mnt_flags)) return -EPERM; - err = security_sb_remount(sb, data); + security_init_mnt_opts(&opts); + if (data && !(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) { + char *secdata = alloc_secdata(); + if (!secdata) + return -ENOMEM; + err = security_sb_copy_data(data, secdata); + if (err) { + free_secdata(secdata); + return err; + } + err = security_sb_parse_opts_str(secdata, &opts); + free_secdata(secdata); + if (err) + return err; + } + err = security_sb_remount(sb, &opts); + security_free_mnt_opts(&opts); if (err) return err; -- cgit From f5c0c26d9008b355babb6d16f3d7c4de3bada0e7 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 17 Nov 2018 12:09:18 -0500 Subject: new helper: security_sb_eat_lsm_opts() combination of alloc_secdata(), security_sb_copy_data(), security_sb_parse_opt_str() and free_secdata(). Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/namespace.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'fs/namespace.c') diff --git a/fs/namespace.c b/fs/namespace.c index 341793fbd390..39aca7b69c2e 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2312,16 +2312,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, security_init_mnt_opts(&opts); if (data && !(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) { - char *secdata = alloc_secdata(); - if (!secdata) - return -ENOMEM; - err = security_sb_copy_data(data, secdata); - if (err) { - free_secdata(secdata); - return err; - } - err = security_sb_parse_opts_str(secdata, &opts); - free_secdata(secdata); + err = security_sb_eat_lsm_opts(data, &opts); if (err) return err; } -- cgit From 204cc0ccf1d49c6292aeef4c8edd1b3d10ff933c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 13 Dec 2018 13:41:47 -0500 Subject: LSM: hide struct security_mnt_opts from any generic code Keep void * instead, allocate on demand (in parse_str_opts, at the moment). Eventually both selinux and smack will be better off with private structures with several strings in those, rather than this "counter and two pointers to dynamically allocated arrays" ugliness. This commit allows to do that at leisure, without disrupting anything outside of given module. Changes: * instead of struct security_mnt_opt use an opaque pointer initialized to NULL. * security_sb_eat_lsm_opts(), security_sb_parse_opts_str() and security_free_mnt_opts() take it as var argument (i.e. as void **); call sites are unchanged. * security_sb_set_mnt_opts() and security_sb_remount() take it by value (i.e. as void *). * new method: ->sb_free_mnt_opts(). Takes void *, does whatever freeing that needs to be done. * ->sb_set_mnt_opts() and ->sb_remount() might get NULL as mnt_opts argument, meaning "empty". Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/namespace.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'fs/namespace.c') diff --git a/fs/namespace.c b/fs/namespace.c index 39aca7b69c2e..badfd287358c 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2299,7 +2299,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, int err; struct super_block *sb = path->mnt->mnt_sb; struct mount *mnt = real_mount(path->mnt); - struct security_mnt_opts opts; + void *sec_opts = NULL; if (!check_mnt(mnt)) return -EINVAL; @@ -2310,14 +2310,13 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, if (!can_change_locked_flags(mnt, mnt_flags)) return -EPERM; - security_init_mnt_opts(&opts); if (data && !(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) { - err = security_sb_eat_lsm_opts(data, &opts); + err = security_sb_eat_lsm_opts(data, &sec_opts); if (err) return err; } - err = security_sb_remount(sb, &opts); - security_free_mnt_opts(&opts); + err = security_sb_remount(sb, sec_opts); + security_free_mnt_opts(&sec_opts); if (err) return err; -- cgit