From 26cb5a328c6b2bda9e859307ce4cfc60df3a2c28 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 9 Nov 2018 22:26:42 -0500 Subject: [PATCH 01/27] exofs_mount(): fix leaks on failure exits ... and don't abuse mount_nodev(), while we are at it. Signed-off-by: Al Viro Reviewed-by: David Howells --- fs/exofs/super.c | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/fs/exofs/super.c b/fs/exofs/super.c index 906839a4da8f..fc80c7233fa5 100644 --- a/fs/exofs/super.c +++ b/fs/exofs/super.c @@ -705,21 +705,18 @@ static int exofs_read_lookup_dev_table(struct exofs_sb_info *sbi, /* * Read the superblock from the OSD and fill in the fields */ -static int exofs_fill_super(struct super_block *sb, void *data, int silent) +static int exofs_fill_super(struct super_block *sb, + struct exofs_mountopt *opts, + struct exofs_sb_info *sbi, + int silent) { struct inode *root; - struct exofs_mountopt *opts = data; - struct exofs_sb_info *sbi; /*extended info */ struct osd_dev *od; /* Master device */ struct exofs_fscb fscb; /*on-disk superblock info */ struct ore_comp comp; unsigned table_count; int ret; - sbi = kzalloc(sizeof(*sbi), GFP_KERNEL); - if (!sbi) - return -ENOMEM; - /* use mount options to fill superblock */ if (opts->is_osdname) { struct osd_dev_info odi = {.systemid_len = 0}; @@ -863,7 +860,9 @@ static struct dentry *exofs_mount(struct file_system_type *type, int flags, const char *dev_name, void *data) { + struct super_block *s; struct exofs_mountopt opts; + struct exofs_sb_info *sbi; int ret; ret = parse_options(data, &opts); @@ -872,9 +871,31 @@ static struct dentry *exofs_mount(struct file_system_type *type, return ERR_PTR(ret); } + sbi = kzalloc(sizeof(*sbi), GFP_KERNEL); + if (!sbi) { + kfree(opts.dev_name); + return ERR_PTR(-ENOMEM); + } + + s = sget(type, NULL, set_anon_super, flags, NULL); + + if (IS_ERR(s)) { + kfree(opts.dev_name); + kfree(sbi); + return ERR_CAST(s); + } + if (!opts.dev_name) opts.dev_name = dev_name; - return mount_nodev(type, flags, &opts, exofs_fill_super); + + + ret = exofs_fill_super(s, &opts, sbi, flags & SB_SILENT ? 1 : 0); + if (ret) { + deactivate_locked_super(s); + return ERR_PTR(ret); + } + s->s_flags |= SB_ACTIVE; + return dget(s->s_root); } /* From e262e32d6bde0f77fb0c95d977482fc872c51996 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 1 Nov 2018 23:07:23 +0000 Subject: [PATCH 02/27] 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 --- arch/arc/kernel/setup.c | 1 + arch/arm/kernel/atags_parse.c | 1 + arch/sh/kernel/setup.c | 1 + arch/sparc/kernel/setup_32.c | 1 + arch/sparc/kernel/setup_64.c | 1 + arch/x86/kernel/setup.c | 1 + drivers/base/devtmpfs.c | 1 + fs/namespace.c | 1 + fs/pnode.c | 1 + fs/super.c | 1 + include/uapi/linux/fs.h | 56 +++------------------------------ include/uapi/linux/mount.h | 58 +++++++++++++++++++++++++++++++++++ init/do_mounts.c | 1 + init/do_mounts_initrd.c | 1 + security/apparmor/lsm.c | 1 + security/apparmor/mount.c | 1 + security/selinux/hooks.c | 1 + security/tomoyo/mount.c | 1 + 18 files changed, 79 insertions(+), 51 deletions(-) create mode 100644 include/uapi/linux/mount.h diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c index b2cae79a25d7..714dc5c2baf1 100644 --- a/arch/arc/kernel/setup.c +++ b/arch/arc/kernel/setup.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c index c10a3e8ee998..a8a4333929f5 100644 --- a/arch/arm/kernel/atags_parse.c +++ b/arch/arm/kernel/atags_parse.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c index c286cf5da6e7..2c0e0f37a318 100644 --- a/arch/sh/kernel/setup.c +++ b/arch/sh/kernel/setup.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include diff --git a/arch/sparc/kernel/setup_32.c b/arch/sparc/kernel/setup_32.c index 13664c377196..7df3d704284c 100644 --- a/arch/sparc/kernel/setup_32.c +++ b/arch/sparc/kernel/setup_32.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c index cd2825cb8420..014390950333 100644 --- a/arch/sparc/kernel/setup_64.c +++ b/arch/sparc/kernel/setup_64.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index b74e7bfed6ab..25a9802fffec 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c index b93fc862d365..0dbc43068eeb 100644 --- a/drivers/base/devtmpfs.c +++ b/drivers/base/devtmpfs.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "base.h" static struct task_struct *thread; 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" diff --git a/fs/pnode.c b/fs/pnode.c index 53d411a371ce..1100e810d855 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "internal.h" #include "pnode.h" diff --git a/fs/super.c b/fs/super.c index ca53a08497ed..6654de035893 100644 --- a/fs/super.c +++ b/fs/super.c @@ -35,6 +35,7 @@ #include #include #include +#include #include "internal.h" static int thaw_super_locked(struct super_block *sb); diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index a441ea1bfe6d..53a22e8e0408 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -14,6 +14,11 @@ #include #include +/* Use of MS_* flags within the kernel is restricted to core mount(2) code. */ +#if !defined(__KERNEL__) +#include +#endif + /* * It's silly to have NR_OPEN bigger than NR_FILE, but you can change * the file limit at runtime and only root can increase the per-process @@ -101,57 +106,6 @@ struct inodes_stat_t { #define NR_FILE 8192 /* this can well be larger on a larger system */ - -/* - * These are the fs-independent mount-flags: up to 32 flags are supported - */ -#define MS_RDONLY 1 /* Mount read-only */ -#define MS_NOSUID 2 /* Ignore suid and sgid bits */ -#define MS_NODEV 4 /* Disallow access to device special files */ -#define MS_NOEXEC 8 /* Disallow program execution */ -#define MS_SYNCHRONOUS 16 /* Writes are synced at once */ -#define MS_REMOUNT 32 /* Alter flags of a mounted FS */ -#define MS_MANDLOCK 64 /* Allow mandatory locks on an FS */ -#define MS_DIRSYNC 128 /* Directory modifications are synchronous */ -#define MS_NOATIME 1024 /* Do not update access times. */ -#define MS_NODIRATIME 2048 /* Do not update directory access times */ -#define MS_BIND 4096 -#define MS_MOVE 8192 -#define MS_REC 16384 -#define MS_VERBOSE 32768 /* War is peace. Verbosity is silence. - MS_VERBOSE is deprecated. */ -#define MS_SILENT 32768 -#define MS_POSIXACL (1<<16) /* VFS does not apply the umask */ -#define MS_UNBINDABLE (1<<17) /* change to unbindable */ -#define MS_PRIVATE (1<<18) /* change to private */ -#define MS_SLAVE (1<<19) /* change to slave */ -#define MS_SHARED (1<<20) /* change to shared */ -#define MS_RELATIME (1<<21) /* Update atime relative to mtime/ctime. */ -#define MS_KERNMOUNT (1<<22) /* this is a kern_mount call */ -#define MS_I_VERSION (1<<23) /* Update inode I_version field */ -#define MS_STRICTATIME (1<<24) /* Always perform atime updates */ -#define MS_LAZYTIME (1<<25) /* Update the on-disk [acm]times lazily */ - -/* These sb flags are internal to the kernel */ -#define MS_SUBMOUNT (1<<26) -#define MS_NOREMOTELOCK (1<<27) -#define MS_NOSEC (1<<28) -#define MS_BORN (1<<29) -#define MS_ACTIVE (1<<30) -#define MS_NOUSER (1<<31) - -/* - * Superblock flags that can be altered by MS_REMOUNT - */ -#define MS_RMT_MASK (MS_RDONLY|MS_SYNCHRONOUS|MS_MANDLOCK|MS_I_VERSION|\ - MS_LAZYTIME) - -/* - * Old magic mount flag and mask - */ -#define MS_MGC_VAL 0xC0ED0000 -#define MS_MGC_MSK 0xffff0000 - /* * Structure for FS_IOC_FSGETXATTR[A] and FS_IOC_FSSETXATTR. */ diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h new file mode 100644 index 000000000000..3f9ec42510b0 --- /dev/null +++ b/include/uapi/linux/mount.h @@ -0,0 +1,58 @@ +#ifndef _UAPI_LINUX_MOUNT_H +#define _UAPI_LINUX_MOUNT_H + +/* + * These are the fs-independent mount-flags: up to 32 flags are supported + * + * Usage of these is restricted within the kernel to core mount(2) code and + * callers of sys_mount() only. Filesystems should be using the SB_* + * equivalent instead. + */ +#define MS_RDONLY 1 /* Mount read-only */ +#define MS_NOSUID 2 /* Ignore suid and sgid bits */ +#define MS_NODEV 4 /* Disallow access to device special files */ +#define MS_NOEXEC 8 /* Disallow program execution */ +#define MS_SYNCHRONOUS 16 /* Writes are synced at once */ +#define MS_REMOUNT 32 /* Alter flags of a mounted FS */ +#define MS_MANDLOCK 64 /* Allow mandatory locks on an FS */ +#define MS_DIRSYNC 128 /* Directory modifications are synchronous */ +#define MS_NOATIME 1024 /* Do not update access times. */ +#define MS_NODIRATIME 2048 /* Do not update directory access times */ +#define MS_BIND 4096 +#define MS_MOVE 8192 +#define MS_REC 16384 +#define MS_VERBOSE 32768 /* War is peace. Verbosity is silence. + MS_VERBOSE is deprecated. */ +#define MS_SILENT 32768 +#define MS_POSIXACL (1<<16) /* VFS does not apply the umask */ +#define MS_UNBINDABLE (1<<17) /* change to unbindable */ +#define MS_PRIVATE (1<<18) /* change to private */ +#define MS_SLAVE (1<<19) /* change to slave */ +#define MS_SHARED (1<<20) /* change to shared */ +#define MS_RELATIME (1<<21) /* Update atime relative to mtime/ctime. */ +#define MS_KERNMOUNT (1<<22) /* this is a kern_mount call */ +#define MS_I_VERSION (1<<23) /* Update inode I_version field */ +#define MS_STRICTATIME (1<<24) /* Always perform atime updates */ +#define MS_LAZYTIME (1<<25) /* Update the on-disk [acm]times lazily */ + +/* These sb flags are internal to the kernel */ +#define MS_SUBMOUNT (1<<26) +#define MS_NOREMOTELOCK (1<<27) +#define MS_NOSEC (1<<28) +#define MS_BORN (1<<29) +#define MS_ACTIVE (1<<30) +#define MS_NOUSER (1<<31) + +/* + * Superblock flags that can be altered by MS_REMOUNT + */ +#define MS_RMT_MASK (MS_RDONLY|MS_SYNCHRONOUS|MS_MANDLOCK|MS_I_VERSION|\ + MS_LAZYTIME) + +/* + * Old magic mount flag and mask + */ +#define MS_MGC_VAL 0xC0ED0000 +#define MS_MGC_MSK 0xffff0000 + +#endif /* _UAPI_LINUX_MOUNT_H */ diff --git a/init/do_mounts.c b/init/do_mounts.c index a754e3ba9831..f8c230c77035 100644 --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "do_mounts.h" diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c index d1a5d885ce13..56a557403d39 100644 --- a/init/do_mounts_initrd.c +++ b/init/do_mounts_initrd.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "do_mounts.h" diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 42446a216f3b..2c010874329f 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "include/apparmor.h" #include "include/apparmorfs.h" diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index c1da22482bfb..8c3787399356 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "include/apparmor.h" #include "include/audit.h" diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 7ce683259357..f695438d985c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -88,6 +88,7 @@ #include #include #include +#include #include "avc.h" #include "objsec.h" diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c index 807fd91dbb54..7dc7f59b7dde 100644 --- a/security/tomoyo/mount.c +++ b/security/tomoyo/mount.c @@ -6,6 +6,7 @@ */ #include +#include #include "common.h" /* String table for special mount operations. */ From 43f5e655eff7e124d4e484515689cba374ab698e Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 1 Nov 2018 23:07:25 +0000 Subject: [PATCH 03/27] 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 ++++++++++++++++++++++++++---------------- include/linux/mount.h | 2 +- 2 files changed, 93 insertions(+), 55 deletions(-) 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 @@ static struct mount *alloc_vfsmnt(const char *name) * 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 @@ static int do_loopback(struct path *path, const char *old_name, 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) { - int error = 0; - int readonly_request = 0; + unsigned int fl = mnt->mnt.mnt_flags; - if (ms_flags & MS_RDONLY) - readonly_request = 1; - if (readonly_request == __mnt_is_readonly(mnt)) + 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) +{ + bool readonly_request = (mnt_flags & MNT_READONLY); + + 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)) { + if (!can_change_locked_flags(mnt, mnt_flags)) 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)) { - 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) diff --git a/include/linux/mount.h b/include/linux/mount.h index 45b1f56c6c2f..037eed52164b 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -81,7 +81,7 @@ extern void mnt_drop_write_file(struct file *file); extern void mntput(struct vfsmount *mnt); extern struct vfsmount *mntget(struct vfsmount *mnt); extern struct vfsmount *mnt_clone_internal(const struct path *path); -extern int __mnt_is_readonly(struct vfsmount *mnt); +extern bool __mnt_is_readonly(struct vfsmount *mnt); extern bool mnt_may_suid(struct vfsmount *mnt); struct path; From 17f3b556a3e3c9227549c3e7762c5c27a48e5c59 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 10 Nov 2018 23:46:07 -0500 Subject: [PATCH 04/27] selinux: expand superblock_doinit() calls Signed-off-by: Al Viro Reviewed-by: David Howells --- security/selinux/hooks.c | 54 ++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f695438d985c..4bd6f9435e2f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1202,33 +1202,6 @@ static int selinux_parse_opts_str(char *options, kfree(rootcontext); return rc; } -/* - * string mount options parsing and call set the sbsec - */ -static int superblock_doinit(struct super_block *sb, void *data) -{ - int rc = 0; - char *options = data; - struct security_mnt_opts opts; - - security_init_mnt_opts(&opts); - - if (!data) - goto out; - - BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA); - - rc = selinux_parse_opts_str(options, &opts); - if (rc) - goto out_err; - -out: - rc = selinux_set_mnt_opts(sb, &opts, 0, NULL); - -out_err: - security_free_mnt_opts(&opts); - return rc; -} static void selinux_write_opts(struct seq_file *m, struct security_mnt_opts *opts) @@ -2926,11 +2899,28 @@ static int selinux_sb_remount(struct super_block *sb, void *data) static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data) { + char *options = data; const struct cred *cred = current_cred(); struct common_audit_data ad; - int rc; + int rc = 0; + struct security_mnt_opts opts; - rc = superblock_doinit(sb, data); + security_init_mnt_opts(&opts); + + if (!data) + goto out; + + BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA); + + rc = selinux_parse_opts_str(options, &opts); + if (rc) + goto out_err; + +out: + rc = selinux_set_mnt_opts(sb, &opts, 0, NULL); + +out_err: + security_free_mnt_opts(&opts); if (rc) return rc; @@ -7194,7 +7184,11 @@ static __init int selinux_init(void) static void delayed_superblock_init(struct super_block *sb, void *unused) { - superblock_doinit(sb, NULL); + struct security_mnt_opts opts; + + security_init_mnt_opts(&opts); + selinux_set_mnt_opts(sb, &opts, 0, NULL); + security_free_mnt_opts(&opts); } void selinux_complete_init(void) From 6466f3d193a99426db067855345e763de2160f1c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 1 Dec 2018 22:55:40 -0500 Subject: [PATCH 05/27] smack: make smack_parse_opts_str() clean up on failure fixes e.g. a btrfs leak... Reviewed-by: David Howells Signed-off-by: Al Viro --- security/smack/smack_lsm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 81fb4c1631e9..73e41797960e 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -735,6 +735,7 @@ static int smack_parse_opts_str(char *options, kfree(fshat); kfree(fsroot); kfree(fstransmute); + security_free_mnt_opts(opts); return rc; } From 6be8750b4cba8c37170f46b29841d112f1be749b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 1 Dec 2018 22:42:44 -0500 Subject: [PATCH 06/27] LSM: lift parsing LSM options into the caller of ->sb_kern_mount() 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/super.c | 24 ++++++++++++++++-------- include/linux/lsm_hooks.h | 3 ++- include/linux/security.h | 6 ++++-- security/security.c | 5 +++-- security/selinux/hooks.c | 24 +++--------------------- security/smack/smack_lsm.c | 23 +++-------------------- 6 files changed, 31 insertions(+), 54 deletions(-) diff --git a/fs/super.c b/fs/super.c index 6654de035893..8d9c9199832d 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1246,17 +1246,26 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) { struct dentry *root; struct super_block *sb; - char *secdata = NULL; int error = -ENOMEM; + struct security_mnt_opts opts; + + security_init_mnt_opts(&opts); if (data && !(type->fs_flags & FS_BINARY_MOUNTDATA)) { - secdata = alloc_secdata(); + char *secdata = alloc_secdata(); if (!secdata) - goto out; + return ERR_PTR(-ENOMEM); error = security_sb_copy_data(data, secdata); + if (error) { + free_secdata(secdata); + return ERR_PTR(error); + } + + error = security_sb_parse_opts_str(secdata, &opts); + free_secdata(secdata); if (error) - goto out_free_secdata; + return ERR_PTR(error); } root = type->mount(type, flags, name, data); @@ -1277,7 +1286,7 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) smp_wmb(); sb->s_flags |= SB_BORN; - error = security_sb_kern_mount(sb, flags, secdata); + error = security_sb_kern_mount(sb, flags, &opts); if (error) goto out_sb; @@ -1291,14 +1300,13 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) "negative value (%lld)\n", type->name, sb->s_maxbytes); up_write(&sb->s_umount); - free_secdata(secdata); + security_free_mnt_opts(&opts); return root; out_sb: dput(root); deactivate_locked_super(sb); out_free_secdata: - free_secdata(secdata); -out: + security_free_mnt_opts(&opts); return ERR_PTR(error); } diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index aaeb7fa24dc4..c7f67341fd1d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1463,7 +1463,8 @@ union security_list_options { void (*sb_free_security)(struct super_block *sb); int (*sb_copy_data)(char *orig, char *copy); int (*sb_remount)(struct super_block *sb, void *data); - int (*sb_kern_mount)(struct super_block *sb, int flags, void *data); + int (*sb_kern_mount)(struct super_block *sb, int flags, + struct security_mnt_opts *opts); int (*sb_show_options)(struct seq_file *m, struct super_block *sb); int (*sb_statfs)(struct dentry *dentry); int (*sb_mount)(const char *dev_name, const struct path *path, diff --git a/include/linux/security.h b/include/linux/security.h index d170a5b031f3..f2f88e41f35f 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -250,7 +250,8 @@ int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); int security_sb_copy_data(char *orig, char *copy); int security_sb_remount(struct super_block *sb, void *data); -int security_sb_kern_mount(struct super_block *sb, int flags, void *data); +int security_sb_kern_mount(struct super_block *sb, int flags, + struct security_mnt_opts *opts); int security_sb_show_options(struct seq_file *m, struct super_block *sb); int security_sb_statfs(struct dentry *dentry); int security_sb_mount(const char *dev_name, const struct path *path, @@ -565,7 +566,8 @@ static inline int security_sb_remount(struct super_block *sb, void *data) return 0; } -static inline int security_sb_kern_mount(struct super_block *sb, int flags, void *data) +static inline int security_sb_kern_mount(struct super_block *sb, int flags, + struct security_mnt_opts *opts) { return 0; } diff --git a/security/security.c b/security/security.c index 04d173eb93f6..b5fc8e1e849c 100644 --- a/security/security.c +++ b/security/security.c @@ -395,9 +395,10 @@ int security_sb_remount(struct super_block *sb, void *data) return call_int_hook(sb_remount, 0, sb, data); } -int security_sb_kern_mount(struct super_block *sb, int flags, void *data) +int security_sb_kern_mount(struct super_block *sb, int flags, + struct security_mnt_opts *opts) { - return call_int_hook(sb_kern_mount, 0, sb, flags, data); + return call_int_hook(sb_kern_mount, 0, sb, flags, opts); } int security_sb_show_options(struct seq_file *m, struct super_block *sb) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4bd6f9435e2f..ba229d4a64d3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2897,30 +2897,12 @@ static int selinux_sb_remount(struct super_block *sb, void *data) goto out_free_opts; } -static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data) +static int selinux_sb_kern_mount(struct super_block *sb, int flags, + struct security_mnt_opts *opts) { - char *options = data; const struct cred *cred = current_cred(); struct common_audit_data ad; - int rc = 0; - struct security_mnt_opts opts; - - security_init_mnt_opts(&opts); - - if (!data) - goto out; - - BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA); - - rc = selinux_parse_opts_str(options, &opts); - if (rc) - goto out_err; - -out: - rc = selinux_set_mnt_opts(sb, &opts, 0, NULL); - -out_err: - security_free_mnt_opts(&opts); + int rc = selinux_set_mnt_opts(sb, opts, 0, NULL); if (rc) return rc; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 73e41797960e..1d465ae3d11c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -859,27 +859,10 @@ static int smack_set_mnt_opts(struct super_block *sb, * * Returns 0 on success, an error code on failure */ -static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data) +static int smack_sb_kern_mount(struct super_block *sb, int flags, + struct security_mnt_opts *opts) { - int rc = 0; - char *options = data; - struct security_mnt_opts opts; - - security_init_mnt_opts(&opts); - - if (!options) - goto out; - - rc = smack_parse_opts_str(options, &opts); - if (rc) - goto out_err; - -out: - rc = smack_set_mnt_opts(sb, &opts, 0, NULL); - -out_err: - security_free_mnt_opts(&opts); - return rc; + return smack_set_mnt_opts(sb, opts, 0, NULL); } /** From c039bc3c2498724946304a8f964244a9b6af1043 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 1 Dec 2018 23:06:57 -0500 Subject: [PATCH 07/27] 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 +++++++++++++++- include/linux/lsm_hooks.h | 3 ++- include/linux/security.h | 5 +++-- security/security.c | 5 +++-- security/selinux/hooks.c | 47 ++++++++++----------------------------- 5 files changed, 38 insertions(+), 41 deletions(-) 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; diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index c7f67341fd1d..e1a12a1e2b32 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1462,7 +1462,8 @@ union security_list_options { int (*sb_alloc_security)(struct super_block *sb); void (*sb_free_security)(struct super_block *sb); int (*sb_copy_data)(char *orig, char *copy); - int (*sb_remount)(struct super_block *sb, void *data); + int (*sb_remount)(struct super_block *sb, + struct security_mnt_opts *opts); int (*sb_kern_mount)(struct super_block *sb, int flags, struct security_mnt_opts *opts); int (*sb_show_options)(struct seq_file *m, struct super_block *sb); diff --git a/include/linux/security.h b/include/linux/security.h index f2f88e41f35f..4fc6d98bc7a6 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -249,7 +249,7 @@ void security_bprm_committed_creds(struct linux_binprm *bprm); int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); int security_sb_copy_data(char *orig, char *copy); -int security_sb_remount(struct super_block *sb, void *data); +int security_sb_remount(struct super_block *sb, struct security_mnt_opts *opts); int security_sb_kern_mount(struct super_block *sb, int flags, struct security_mnt_opts *opts); int security_sb_show_options(struct seq_file *m, struct super_block *sb); @@ -561,7 +561,8 @@ static inline int security_sb_copy_data(char *orig, char *copy) return 0; } -static inline int security_sb_remount(struct super_block *sb, void *data) +static inline int security_sb_remount(struct super_block *sb, + struct security_mnt_opts *opts) { return 0; } diff --git a/security/security.c b/security/security.c index b5fc8e1e849c..3f50beb30fb1 100644 --- a/security/security.c +++ b/security/security.c @@ -390,9 +390,10 @@ int security_sb_copy_data(char *orig, char *copy) } EXPORT_SYMBOL(security_sb_copy_data); -int security_sb_remount(struct super_block *sb, void *data) +int security_sb_remount(struct super_block *sb, + struct security_mnt_opts *opts) { - return call_int_hook(sb_remount, 0, sb, data); + return call_int_hook(sb_remount, 0, sb, opts); } int security_sb_kern_mount(struct super_block *sb, int flags, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ba229d4a64d3..ba3e2917bd24 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2812,39 +2812,22 @@ static int selinux_sb_copy_data(char *orig, char *copy) return rc; } -static int selinux_sb_remount(struct super_block *sb, void *data) +static int selinux_sb_remount(struct super_block *sb, + struct security_mnt_opts *opts) { - int rc, i, *flags; - struct security_mnt_opts opts; - char *secdata, **mount_options; + int i, *flags; + char **mount_options; struct superblock_security_struct *sbsec = sb->s_security; if (!(sbsec->flags & SE_SBINITIALIZED)) return 0; - if (!data) - return 0; + mount_options = opts->mnt_opts; + flags = opts->mnt_opts_flags; - if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) - return 0; - - security_init_mnt_opts(&opts); - secdata = alloc_secdata(); - if (!secdata) - return -ENOMEM; - rc = selinux_sb_copy_data(data, secdata); - if (rc) - goto out_free_secdata; - - rc = selinux_parse_opts_str(secdata, &opts); - if (rc) - goto out_free_secdata; - - mount_options = opts.mnt_opts; - flags = opts.mnt_opts_flags; - - for (i = 0; i < opts.num_mnt_opts; i++) { + for (i = 0; i < opts->num_mnt_opts; i++) { u32 sid; + int rc; if (flags[i] == SBLABEL_MNT) continue; @@ -2855,9 +2838,8 @@ static int selinux_sb_remount(struct super_block *sb, void *data) pr_warn("SELinux: security_context_str_to_sid" "(%s) failed for (dev %s, type %s) errno=%d\n", mount_options[i], sb->s_id, sb->s_type->name, rc); - goto out_free_opts; + return rc; } - rc = -EINVAL; switch (flags[i]) { case FSCONTEXT_MNT: if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) @@ -2880,21 +2862,16 @@ static int selinux_sb_remount(struct super_block *sb, void *data) goto out_bad_option; break; default: - goto out_free_opts; + return -EINVAL; } } + return 0; - rc = 0; -out_free_opts: - security_free_mnt_opts(&opts); -out_free_secdata: - free_secdata(secdata); - return rc; out_bad_option: pr_warn("SELinux: unable to change security options " "during remount (dev %s, type=%s)\n", sb->s_id, sb->s_type->name); - goto out_free_opts; + return -EINVAL; } static int selinux_sb_kern_mount(struct super_block *sb, int flags, From f5c0c26d9008b355babb6d16f3d7c4de3bada0e7 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 17 Nov 2018 12:09:18 -0500 Subject: [PATCH 08/27] 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/btrfs/super.c | 15 +-------------- fs/namespace.c | 11 +---------- fs/nfs/super.c | 15 ++------------- fs/super.c | 13 +------------ include/linux/security.h | 28 +++------------------------- security/security.c | 15 ++++++++++++--- 6 files changed, 20 insertions(+), 77 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index b362b45dd757..6fc8e963ad44 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1461,20 +1461,7 @@ static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid, static int parse_security_options(char *orig_opts, struct security_mnt_opts *sec_opts) { - char *secdata = NULL; - int ret = 0; - - secdata = alloc_secdata(); - if (!secdata) - return -ENOMEM; - ret = security_sb_copy_data(orig_opts, secdata); - if (ret) { - free_secdata(secdata); - return ret; - } - ret = security_sb_parse_opts_str(secdata, sec_opts); - free_secdata(secdata); - return ret; + return security_sb_eat_lsm_opts(orig_opts, sec_opts); } static int setup_security_options(struct btrfs_fs_info *fs_info, 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; } diff --git a/fs/nfs/super.c b/fs/nfs/super.c index ac4b2f005778..f9c8847171e8 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -1206,7 +1206,7 @@ static int nfs_get_option_ul_bound(substring_t args[], unsigned long *option, static int nfs_parse_mount_options(char *raw, struct nfs_parsed_mount_data *mnt) { - char *p, *string, *secdata; + char *p, *string; int rc, sloppy = 0, invalid_option = 0; unsigned short protofamily = AF_UNSPEC; unsigned short mountfamily = AF_UNSPEC; @@ -1217,20 +1217,10 @@ static int nfs_parse_mount_options(char *raw, } dfprintk(MOUNT, "NFS: nfs mount opts='%s'\n", raw); - secdata = alloc_secdata(); - if (!secdata) - goto out_nomem; - - rc = security_sb_copy_data(raw, secdata); + rc = security_sb_eat_lsm_opts(raw, &mnt->lsm_opts); if (rc) goto out_security_failure; - rc = security_sb_parse_opts_str(secdata, &mnt->lsm_opts); - if (rc) - goto out_security_failure; - - free_secdata(secdata); - while ((p = strsep(&raw, ",")) != NULL) { substring_t args[MAX_OPT_ARGS]; unsigned long option; @@ -1682,7 +1672,6 @@ static int nfs_parse_mount_options(char *raw, printk(KERN_INFO "NFS: not enough memory to parse option\n"); return 0; out_security_failure: - free_secdata(secdata); printk(KERN_INFO "NFS: security options invalid: %d\n", rc); return 0; } diff --git a/fs/super.c b/fs/super.c index 8d9c9199832d..d571527cb8b8 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1252,18 +1252,7 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) security_init_mnt_opts(&opts); if (data && !(type->fs_flags & FS_BINARY_MOUNTDATA)) { - char *secdata = alloc_secdata(); - if (!secdata) - return ERR_PTR(-ENOMEM); - - error = security_sb_copy_data(data, secdata); - if (error) { - free_secdata(secdata); - return ERR_PTR(error); - } - - error = security_sb_parse_opts_str(secdata, &opts); - free_secdata(secdata); + error = security_sb_eat_lsm_opts(data, &opts); if (error) return ERR_PTR(error); } diff --git a/include/linux/security.h b/include/linux/security.h index 4fc6d98bc7a6..262e59838803 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -248,7 +248,7 @@ void security_bprm_committing_creds(struct linux_binprm *bprm); void security_bprm_committed_creds(struct linux_binprm *bprm); int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); -int security_sb_copy_data(char *orig, char *copy); +int security_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts); int security_sb_remount(struct super_block *sb, struct security_mnt_opts *opts); int security_sb_kern_mount(struct super_block *sb, int flags, struct security_mnt_opts *opts); @@ -556,7 +556,8 @@ static inline int security_sb_alloc(struct super_block *sb) static inline void security_sb_free(struct super_block *sb) { } -static inline int security_sb_copy_data(char *orig, char *copy) +static inline int security_sb_eat_lsm_opts(char *options, + struct security_mnt_opts *opts) { return 0; } @@ -1823,28 +1824,5 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux) #endif /* CONFIG_SECURITY */ #endif /* CONFIG_BPF_SYSCALL */ -#ifdef CONFIG_SECURITY - -static inline char *alloc_secdata(void) -{ - return (char *)get_zeroed_page(GFP_KERNEL); -} - -static inline void free_secdata(void *secdata) -{ - free_page((unsigned long)secdata); -} - -#else - -static inline char *alloc_secdata(void) -{ - return (char *)1; -} - -static inline void free_secdata(void *secdata) -{ } -#endif /* CONFIG_SECURITY */ - #endif /* ! __LINUX_SECURITY_H */ diff --git a/security/security.c b/security/security.c index 3f50beb30fb1..02c656dd5c0c 100644 --- a/security/security.c +++ b/security/security.c @@ -384,11 +384,20 @@ void security_sb_free(struct super_block *sb) call_void_hook(sb_free_security, sb); } -int security_sb_copy_data(char *orig, char *copy) +int security_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) { - return call_int_hook(sb_copy_data, 0, orig, copy); + char *s = (char *)get_zeroed_page(GFP_KERNEL); + int err; + + if (!s) + return -ENOMEM; + err = call_int_hook(sb_copy_data, 0, options, s); + if (!err) + err = call_int_hook(sb_parse_opts_str, 0, s, opts); + free_page((unsigned long)s); + return err; } -EXPORT_SYMBOL(security_sb_copy_data); +EXPORT_SYMBOL(security_sb_eat_lsm_opts); int security_sb_remount(struct super_block *sb, struct security_mnt_opts *opts) From a10d7c22b34bcf744679019269bfb33ebf0b75ee Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 5 Dec 2018 11:58:35 -0500 Subject: [PATCH 09/27] LSM: split ->sb_set_mnt_opts() out of ->sb_kern_mount() ... leaving the "is it kernel-internal" logics in the caller. Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/super.c | 8 +++++++- include/linux/lsm_hooks.h | 3 +-- include/linux/security.h | 6 ++---- security/security.c | 5 ++--- security/selinux/hooks.c | 10 +--------- security/smack/smack_lsm.c | 15 --------------- 6 files changed, 13 insertions(+), 34 deletions(-) diff --git a/fs/super.c b/fs/super.c index d571527cb8b8..1f75fe312597 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1275,10 +1275,16 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) smp_wmb(); sb->s_flags |= SB_BORN; - error = security_sb_kern_mount(sb, flags, &opts); + error = security_sb_set_mnt_opts(sb, &opts, 0, NULL); if (error) goto out_sb; + if (!(flags & MS_KERNMOUNT)) { + error = security_sb_kern_mount(sb); + if (error) + goto out_sb; + } + /* * filesystems should never set s_maxbytes larger than MAX_LFS_FILESIZE * but s_maxbytes was an unsigned long long for many releases. Throw diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index e1a12a1e2b32..f432123af0e3 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1464,8 +1464,7 @@ union security_list_options { int (*sb_copy_data)(char *orig, char *copy); int (*sb_remount)(struct super_block *sb, struct security_mnt_opts *opts); - int (*sb_kern_mount)(struct super_block *sb, int flags, - struct security_mnt_opts *opts); + int (*sb_kern_mount)(struct super_block *sb); int (*sb_show_options)(struct seq_file *m, struct super_block *sb); int (*sb_statfs)(struct dentry *dentry); int (*sb_mount)(const char *dev_name, const struct path *path, diff --git a/include/linux/security.h b/include/linux/security.h index 262e59838803..d00093363570 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -250,8 +250,7 @@ int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); int security_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts); int security_sb_remount(struct super_block *sb, struct security_mnt_opts *opts); -int security_sb_kern_mount(struct super_block *sb, int flags, - struct security_mnt_opts *opts); +int security_sb_kern_mount(struct super_block *sb); int security_sb_show_options(struct seq_file *m, struct super_block *sb); int security_sb_statfs(struct dentry *dentry); int security_sb_mount(const char *dev_name, const struct path *path, @@ -568,8 +567,7 @@ static inline int security_sb_remount(struct super_block *sb, return 0; } -static inline int security_sb_kern_mount(struct super_block *sb, int flags, - struct security_mnt_opts *opts) +static inline int security_sb_kern_mount(struct super_block *sb) { return 0; } diff --git a/security/security.c b/security/security.c index 02c656dd5c0c..afb05646d41b 100644 --- a/security/security.c +++ b/security/security.c @@ -405,10 +405,9 @@ int security_sb_remount(struct super_block *sb, return call_int_hook(sb_remount, 0, sb, opts); } -int security_sb_kern_mount(struct super_block *sb, int flags, - struct security_mnt_opts *opts) +int security_sb_kern_mount(struct super_block *sb) { - return call_int_hook(sb_kern_mount, 0, sb, flags, opts); + return call_int_hook(sb_kern_mount, 0, sb); } int security_sb_show_options(struct seq_file *m, struct super_block *sb) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ba3e2917bd24..59b164d7134d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2874,18 +2874,10 @@ static int selinux_sb_remount(struct super_block *sb, return -EINVAL; } -static int selinux_sb_kern_mount(struct super_block *sb, int flags, - struct security_mnt_opts *opts) +static int selinux_sb_kern_mount(struct super_block *sb) { const struct cred *cred = current_cred(); struct common_audit_data ad; - int rc = selinux_set_mnt_opts(sb, opts, 0, NULL); - if (rc) - return rc; - - /* Allow all mounts performed by the kernel */ - if (flags & MS_KERNMOUNT) - return 0; ad.type = LSM_AUDIT_DATA_DENTRY; ad.u.dentry = sb->s_root; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 1d465ae3d11c..50e6e88bfe70 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -851,20 +851,6 @@ static int smack_set_mnt_opts(struct super_block *sb, return 0; } -/** - * smack_sb_kern_mount - Smack specific mount processing - * @sb: the file system superblock - * @flags: the mount flags - * @data: the smack mount options - * - * Returns 0 on success, an error code on failure - */ -static int smack_sb_kern_mount(struct super_block *sb, int flags, - struct security_mnt_opts *opts) -{ - return smack_set_mnt_opts(sb, opts, 0, NULL); -} - /** * smack_sb_statfs - Smack check on statfs * @dentry: identifies the file system in question @@ -4652,7 +4638,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_alloc_security, smack_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, smack_sb_free_security), LSM_HOOK_INIT(sb_copy_data, smack_sb_copy_data), - LSM_HOOK_INIT(sb_kern_mount, smack_sb_kern_mount), LSM_HOOK_INIT(sb_statfs, smack_sb_statfs), LSM_HOOK_INIT(sb_set_mnt_opts, smack_set_mnt_opts), LSM_HOOK_INIT(sb_parse_opts_str, smack_parse_opts_str), From 8d64124a6a93ec68fda6f781e48a7b95d9dd17d9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 10 Dec 2018 15:34:12 -0500 Subject: [PATCH 10/27] selinux; don't open-code a loop in sb_finish_set_opts() Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 59b164d7134d..630fe8883957 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -571,10 +571,9 @@ static int sb_finish_set_opts(struct super_block *sb) during get_sb by a pseudo filesystem that directly populates itself. */ spin_lock(&sbsec->isec_lock); -next_inode: - if (!list_empty(&sbsec->isec_head)) { + while (!list_empty(&sbsec->isec_head)) { struct inode_security_struct *isec = - list_entry(sbsec->isec_head.next, + list_first_entry(&sbsec->isec_head, struct inode_security_struct, list); struct inode *inode = isec->inode; list_del_init(&isec->list); @@ -586,7 +585,6 @@ static int sb_finish_set_opts(struct super_block *sb) iput(inode); } spin_lock(&sbsec->isec_lock); - goto next_inode; } spin_unlock(&sbsec->isec_lock); out: From a65001e8a4d465693d0191297a6fd864c96b3147 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 10 Dec 2018 17:19:21 -0500 Subject: [PATCH 11/27] btrfs: sanitize security_mnt_opts use 1) keeping a copy in btrfs_fs_info is completely pointless - we never use it for anything. Getting rid of that allows for simpler calling conventions for setup_security_options() (caller is responsible for freeing mnt_opts in all cases). 2) on remount we want to use ->sb_remount(), not ->sb_set_mnt_opts(), same as we would if not for FS_BINARY_MOUNTDATA. Behaviours *are* close (in fact, selinux sb_set_mnt_opts() ought to punt to sb_remount() in "already initialized" case), but let's handle that uniformly. And the only reason why the original btrfs changes didn't go for security_sb_remount() in btrfs_remount() case is that it hadn't been exported. Let's export it for a while - it'll be going away soon anyway. Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/btrfs/ctree.h | 4 --- fs/btrfs/super.c | 63 +++++++-------------------------------------- security/security.c | 1 + 3 files changed, 10 insertions(+), 58 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 80953528572d..f7ec833e6c7a 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1100,9 +1100,6 @@ struct btrfs_fs_info { struct mutex unused_bg_unpin_mutex; struct mutex delete_unused_bgs_mutex; - /* For btrfs to record security options */ - struct security_mnt_opts security_opts; - /* * Chunks that can't be freed yet (under a trim/discard operation) * and will be latter freed. Protected by fs_info->chunk_mutex. @@ -2959,7 +2956,6 @@ static inline void free_fs_info(struct btrfs_fs_info *fs_info) kfree(fs_info->free_space_root); kfree(fs_info->super_copy); kfree(fs_info->super_for_commit); - security_free_mnt_opts(&fs_info->security_opts); kvfree(fs_info); } diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 6fc8e963ad44..3b04e7735b5f 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1458,43 +1458,6 @@ static struct dentry *mount_subvol(const char *subvol_name, u64 subvol_objectid, return root; } -static int parse_security_options(char *orig_opts, - struct security_mnt_opts *sec_opts) -{ - return security_sb_eat_lsm_opts(orig_opts, sec_opts); -} - -static int setup_security_options(struct btrfs_fs_info *fs_info, - struct super_block *sb, - struct security_mnt_opts *sec_opts) -{ - int ret = 0; - - /* - * Call security_sb_set_mnt_opts() to check whether new sec_opts - * is valid. - */ - ret = security_sb_set_mnt_opts(sb, sec_opts, 0, NULL); - if (ret) - return ret; - -#ifdef CONFIG_SECURITY - if (!fs_info->security_opts.num_mnt_opts) { - /* first time security setup, copy sec_opts to fs_info */ - memcpy(&fs_info->security_opts, sec_opts, sizeof(*sec_opts)); - } else { - /* - * Since SELinux (the only one supporting security_mnt_opts) - * does NOT support changing context during remount/mount of - * the same sb, this must be the same or part of the same - * security options, just free it. - */ - security_free_mnt_opts(sec_opts); - } -#endif - return ret; -} - /* * Find a superblock for the given device / mount point. * @@ -1518,7 +1481,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, security_init_mnt_opts(&new_sec_opts); if (data) { - error = parse_security_options(data, &new_sec_opts); + error = security_sb_eat_lsm_opts(data, &new_sec_opts); if (error) return ERR_PTR(error); } @@ -1537,7 +1500,6 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, fs_info->super_copy = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_KERNEL); fs_info->super_for_commit = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_KERNEL); - security_init_mnt_opts(&fs_info->security_opts); if (!fs_info->super_copy || !fs_info->super_for_commit) { error = -ENOMEM; goto error_fs_info; @@ -1588,16 +1550,12 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, btrfs_sb(s)->bdev_holder = fs_type; error = btrfs_fill_super(s, fs_devices, data); } + if (!error) + error = security_sb_set_mnt_opts(s, &new_sec_opts, 0, NULL); + security_free_mnt_opts(&new_sec_opts); if (error) { deactivate_locked_super(s); - goto error_sec_opts; - } - - fs_info = btrfs_sb(s); - error = setup_security_options(fs_info, s, &new_sec_opts); - if (error) { - deactivate_locked_super(s); - goto error_sec_opts; + return ERR_PTR(error); } return dget(s->s_root); @@ -1769,15 +1727,12 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) struct security_mnt_opts new_sec_opts; security_init_mnt_opts(&new_sec_opts); - ret = parse_security_options(data, &new_sec_opts); + ret = security_sb_eat_lsm_opts(data, &new_sec_opts); + if (!ret) + ret = security_sb_remount(sb, &new_sec_opts); + security_free_mnt_opts(&new_sec_opts); if (ret) goto restore; - ret = setup_security_options(fs_info, sb, - &new_sec_opts); - if (ret) { - security_free_mnt_opts(&new_sec_opts); - goto restore; - } } ret = btrfs_parse_options(fs_info, data, *flags); diff --git a/security/security.c b/security/security.c index afb05646d41b..3d8b72904e00 100644 --- a/security/security.c +++ b/security/security.c @@ -404,6 +404,7 @@ int security_sb_remount(struct super_block *sb, { return call_int_hook(sb_remount, 0, sb, opts); } +EXPORT_SYMBOL(security_sb_remount); int security_sb_kern_mount(struct super_block *sb) { From 6a0440e5b7562512c021aa1b5a706fcc545773db Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 10 Dec 2018 17:30:41 -0500 Subject: [PATCH 12/27] nfs_remount(): don't leak, don't ignore LSM options quietly * if mount(2) passes something like "context=foo" with MS_REMOUNT in flags (/sbin/mount.nfs will _not_ do that - you need to issue the syscall manually), you'll get leaked copies for LSM options. The reason is that instead of nfs_{alloc,free}_parsed_mount_data() nfs_remount() uses kzalloc/kfree, which lacks the needed cleanup. * selinux options are not changed on remount (as for any other fs), but in case of NFS the failure is quiet - they are not compared to what we used to have, with complaint in case of attempted changes. Trivially fixed by converting to use of security_sb_remount(). Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/nfs/super.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index f9c8847171e8..300bdd1d4a09 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -2254,7 +2254,7 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data) options->version <= 6)))) return 0; - data = kzalloc(sizeof(*data), GFP_KERNEL); + data = nfs_alloc_parsed_mount_data(); if (data == NULL) return -ENOMEM; @@ -2293,8 +2293,10 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data) /* compare new mount options with old ones */ error = nfs_compare_remount_data(nfss, data); + if (!error) + error = security_sb_remount(sb, &data->lsm_opts); out: - kfree(data); + nfs_free_parsed_mount_data(data); return error; } EXPORT_SYMBOL_GPL(nfs_remount); From 5b4002391153acebce2557af318bbdc17e235134 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 12 Dec 2018 20:13:29 -0500 Subject: [PATCH 13/27] LSM: turn sb_eat_lsm_opts() into a method Kill ->sb_copy_data() - it's used only in combination with immediately following ->sb_parse_opts_str(). Turn that combination into a new method. This is just a mechanical move - cleanups will be the next step. Reviewed-by: David Howells Signed-off-by: Al Viro --- include/linux/lsm_hooks.h | 4 ++-- security/security.c | 11 +---------- security/selinux/hooks.c | 16 +++++++++++++++- security/smack/smack_lsm.c | 16 +++++++++++++++- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index f432123af0e3..c418909c178c 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1461,7 +1461,7 @@ union security_list_options { int (*sb_alloc_security)(struct super_block *sb); void (*sb_free_security)(struct super_block *sb); - int (*sb_copy_data)(char *orig, char *copy); + int (*sb_eat_lsm_opts)(char *orig, struct security_mnt_opts *opts); int (*sb_remount)(struct super_block *sb, struct security_mnt_opts *opts); int (*sb_kern_mount)(struct super_block *sb); @@ -1801,7 +1801,7 @@ struct security_hook_heads { struct hlist_head bprm_committed_creds; struct hlist_head sb_alloc_security; struct hlist_head sb_free_security; - struct hlist_head sb_copy_data; + struct hlist_head sb_eat_lsm_opts; struct hlist_head sb_remount; struct hlist_head sb_kern_mount; struct hlist_head sb_show_options; diff --git a/security/security.c b/security/security.c index 3d8b72904e00..feb18c925349 100644 --- a/security/security.c +++ b/security/security.c @@ -386,16 +386,7 @@ void security_sb_free(struct super_block *sb) int security_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) { - char *s = (char *)get_zeroed_page(GFP_KERNEL); - int err; - - if (!s) - return -ENOMEM; - err = call_int_hook(sb_copy_data, 0, options, s); - if (!err) - err = call_int_hook(sb_parse_opts_str, 0, s, opts); - free_page((unsigned long)s); - return err; + return call_int_hook(sb_eat_lsm_opts, 0, options, opts); } EXPORT_SYMBOL(security_sb_eat_lsm_opts); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 630fe8883957..ce0511f024e0 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2810,6 +2810,20 @@ static int selinux_sb_copy_data(char *orig, char *copy) return rc; } +static int selinux_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) +{ + char *s = (char *)get_zeroed_page(GFP_KERNEL); + int err; + + if (!s) + return -ENOMEM; + err = selinux_sb_copy_data(options, s); + if (!err) + err = selinux_parse_opts_str(s, opts); + free_page((unsigned long)s); + return err; +} + static int selinux_sb_remount(struct super_block *sb, struct security_mnt_opts *opts) { @@ -6863,7 +6877,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security), - LSM_HOOK_INIT(sb_copy_data, selinux_sb_copy_data), + LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts), LSM_HOOK_INIT(sb_remount, selinux_sb_remount), LSM_HOOK_INIT(sb_kern_mount, selinux_sb_kern_mount), LSM_HOOK_INIT(sb_show_options, selinux_sb_show_options), diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 50e6e88bfe70..835cca277c2a 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -739,6 +739,20 @@ static int smack_parse_opts_str(char *options, return rc; } +static int smack_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) +{ + char *s = (char *)get_zeroed_page(GFP_KERNEL); + int err; + + if (!s) + return -ENOMEM; + err = smack_sb_copy_data(options, s); + if (!err) + err = smack_parse_opts_str(s, opts); + free_page((unsigned long)s); + return err; +} + /** * smack_set_mnt_opts - set Smack specific mount options * @sb: the file system superblock @@ -4637,7 +4651,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_alloc_security, smack_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, smack_sb_free_security), - LSM_HOOK_INIT(sb_copy_data, smack_sb_copy_data), + LSM_HOOK_INIT(sb_eat_lsm_opts, smack_sb_eat_lsm_opts), LSM_HOOK_INIT(sb_statfs, smack_sb_statfs), LSM_HOOK_INIT(sb_set_mnt_opts, smack_set_mnt_opts), LSM_HOOK_INIT(sb_parse_opts_str, smack_parse_opts_str), From e3489f8974e178d723259a842a1e61708dd7dc1e Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 13 Dec 2018 00:24:36 -0500 Subject: [PATCH 14/27] selinux: kill selinux_sb_get_mnt_opts() it's much easier to just do the right thing in ->sb_show_options(), without bothering with allocating and populating arrays, etc. Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 200 ++++++++++----------------------------- 1 file changed, 52 insertions(+), 148 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ce0511f024e0..11cf2feb27b3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -591,105 +591,6 @@ static int sb_finish_set_opts(struct super_block *sb) return rc; } -/* - * This function should allow an FS to ask what it's mount security - * options were so it can use those later for submounts, displaying - * mount options, or whatever. - */ -static int selinux_get_mnt_opts(const struct super_block *sb, - struct security_mnt_opts *opts) -{ - int rc = 0, i; - struct superblock_security_struct *sbsec = sb->s_security; - char *context = NULL; - u32 len; - char tmp; - - security_init_mnt_opts(opts); - - if (!(sbsec->flags & SE_SBINITIALIZED)) - return -EINVAL; - - if (!selinux_state.initialized) - return -EINVAL; - - /* make sure we always check enough bits to cover the mask */ - BUILD_BUG_ON(SE_MNTMASK >= (1 << NUM_SEL_MNT_OPTS)); - - tmp = sbsec->flags & SE_MNTMASK; - /* count the number of mount options for this sb */ - for (i = 0; i < NUM_SEL_MNT_OPTS; i++) { - if (tmp & 0x01) - opts->num_mnt_opts++; - tmp >>= 1; - } - /* Check if the Label support flag is set */ - if (sbsec->flags & SBLABEL_MNT) - opts->num_mnt_opts++; - - opts->mnt_opts = kcalloc(opts->num_mnt_opts, sizeof(char *), GFP_ATOMIC); - if (!opts->mnt_opts) { - rc = -ENOMEM; - goto out_free; - } - - opts->mnt_opts_flags = kcalloc(opts->num_mnt_opts, sizeof(int), GFP_ATOMIC); - if (!opts->mnt_opts_flags) { - rc = -ENOMEM; - goto out_free; - } - - i = 0; - if (sbsec->flags & FSCONTEXT_MNT) { - rc = security_sid_to_context(&selinux_state, sbsec->sid, - &context, &len); - if (rc) - goto out_free; - opts->mnt_opts[i] = context; - opts->mnt_opts_flags[i++] = FSCONTEXT_MNT; - } - if (sbsec->flags & CONTEXT_MNT) { - rc = security_sid_to_context(&selinux_state, - sbsec->mntpoint_sid, - &context, &len); - if (rc) - goto out_free; - opts->mnt_opts[i] = context; - opts->mnt_opts_flags[i++] = CONTEXT_MNT; - } - if (sbsec->flags & DEFCONTEXT_MNT) { - rc = security_sid_to_context(&selinux_state, sbsec->def_sid, - &context, &len); - if (rc) - goto out_free; - opts->mnt_opts[i] = context; - opts->mnt_opts_flags[i++] = DEFCONTEXT_MNT; - } - if (sbsec->flags & ROOTCONTEXT_MNT) { - struct dentry *root = sbsec->sb->s_root; - struct inode_security_struct *isec = backing_inode_security(root); - - rc = security_sid_to_context(&selinux_state, isec->sid, - &context, &len); - if (rc) - goto out_free; - opts->mnt_opts[i] = context; - opts->mnt_opts_flags[i++] = ROOTCONTEXT_MNT; - } - if (sbsec->flags & SBLABEL_MNT) { - opts->mnt_opts[i] = NULL; - opts->mnt_opts_flags[i++] = SBLABEL_MNT; - } - - BUG_ON(i != opts->num_mnt_opts); - - return 0; - -out_free: - security_free_mnt_opts(opts); - return rc; -} - static int bad_option(struct superblock_security_struct *sbsec, char flag, u32 old_sid, u32 new_sid) { @@ -1201,70 +1102,73 @@ static int selinux_parse_opts_str(char *options, return rc; } -static void selinux_write_opts(struct seq_file *m, - struct security_mnt_opts *opts) +static int show_sid(struct seq_file *m, u32 sid) { - int i; - char *prefix; + char *context = NULL; + u32 len; + int rc; - for (i = 0; i < opts->num_mnt_opts; i++) { - char *has_comma; + rc = security_sid_to_context(&selinux_state, sid, + &context, &len); + if (!rc) { + bool has_comma = context && strchr(context, ','); - if (opts->mnt_opts[i]) - has_comma = strchr(opts->mnt_opts[i], ','); - else - has_comma = NULL; - - switch (opts->mnt_opts_flags[i]) { - case CONTEXT_MNT: - prefix = CONTEXT_STR; - break; - case FSCONTEXT_MNT: - prefix = FSCONTEXT_STR; - break; - case ROOTCONTEXT_MNT: - prefix = ROOTCONTEXT_STR; - break; - case DEFCONTEXT_MNT: - prefix = DEFCONTEXT_STR; - break; - case SBLABEL_MNT: - seq_putc(m, ','); - seq_puts(m, LABELSUPP_STR); - continue; - default: - BUG(); - return; - }; - /* we need a comma before each option */ - seq_putc(m, ','); - seq_puts(m, prefix); if (has_comma) seq_putc(m, '\"'); - seq_escape(m, opts->mnt_opts[i], "\"\n\\"); + seq_escape(m, context, "\"\n\\"); if (has_comma) seq_putc(m, '\"'); } + kfree(context); + return rc; } static int selinux_sb_show_options(struct seq_file *m, struct super_block *sb) { - struct security_mnt_opts opts; + struct superblock_security_struct *sbsec = sb->s_security; int rc; - rc = selinux_get_mnt_opts(sb, &opts); - if (rc) { - /* before policy load we may get EINVAL, don't show anything */ - if (rc == -EINVAL) - rc = 0; - return rc; + if (!(sbsec->flags & SE_SBINITIALIZED)) + return 0; + + if (!selinux_state.initialized) + return 0; + + if (sbsec->flags & FSCONTEXT_MNT) { + seq_putc(m, ','); + seq_puts(m, FSCONTEXT_STR); + rc = show_sid(m, sbsec->sid); + if (rc) + return rc; } - - selinux_write_opts(m, &opts); - - security_free_mnt_opts(&opts); - - return rc; + if (sbsec->flags & CONTEXT_MNT) { + seq_putc(m, ','); + seq_puts(m, CONTEXT_STR); + rc = show_sid(m, sbsec->mntpoint_sid); + if (rc) + return rc; + } + if (sbsec->flags & DEFCONTEXT_MNT) { + seq_putc(m, ','); + seq_puts(m, DEFCONTEXT_STR); + rc = show_sid(m, sbsec->def_sid); + if (rc) + return rc; + } + if (sbsec->flags & ROOTCONTEXT_MNT) { + struct dentry *root = sbsec->sb->s_root; + struct inode_security_struct *isec = backing_inode_security(root); + seq_putc(m, ','); + seq_puts(m, ROOTCONTEXT_STR); + rc = show_sid(m, isec->sid); + if (rc) + return rc; + } + if (sbsec->flags & SBLABEL_MNT) { + seq_putc(m, ','); + seq_puts(m, LABELSUPP_STR); + } + return 0; } static inline u16 inode_mode_to_security_class(umode_t mode) From 204cc0ccf1d49c6292aeef4c8edd1b3d10ff933c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 13 Dec 2018 13:41:47 -0500 Subject: [PATCH 15/27] 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/btrfs/super.c | 10 +++----- fs/namespace.c | 9 +++---- fs/nfs/internal.h | 2 +- fs/nfs/super.c | 6 ++--- fs/super.c | 12 ++++----- include/linux/lsm_hooks.h | 11 ++++---- include/linux/security.h | 43 ++++++++----------------------- security/security.c | 27 +++++++++++++------- security/selinux/hooks.c | 52 +++++++++++++++++++++++++++----------- security/smack/smack_lsm.c | 38 ++++++++++++++++++++++------ 10 files changed, 118 insertions(+), 92 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 3b04e7735b5f..e90c4616ed6a 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1472,14 +1472,13 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, struct btrfs_device *device = NULL; struct btrfs_fs_devices *fs_devices = NULL; struct btrfs_fs_info *fs_info = NULL; - struct security_mnt_opts new_sec_opts; + void *new_sec_opts = NULL; fmode_t mode = FMODE_READ; int error = 0; if (!(flags & SB_RDONLY)) mode |= FMODE_WRITE; - security_init_mnt_opts(&new_sec_opts); if (data) { error = security_sb_eat_lsm_opts(data, &new_sec_opts); if (error) @@ -1551,7 +1550,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, error = btrfs_fill_super(s, fs_devices, data); } if (!error) - error = security_sb_set_mnt_opts(s, &new_sec_opts, 0, NULL); + error = security_sb_set_mnt_opts(s, new_sec_opts, 0, NULL); security_free_mnt_opts(&new_sec_opts); if (error) { deactivate_locked_super(s); @@ -1724,12 +1723,11 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data) btrfs_remount_prepare(fs_info); if (data) { - struct security_mnt_opts new_sec_opts; + void *new_sec_opts = NULL; - security_init_mnt_opts(&new_sec_opts); ret = security_sb_eat_lsm_opts(data, &new_sec_opts); if (!ret) - ret = security_sb_remount(sb, &new_sec_opts); + ret = security_sb_remount(sb, new_sec_opts); security_free_mnt_opts(&new_sec_opts); if (ret) goto restore; 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; diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h index 8357ff69962f..97e1dcefe561 100644 --- a/fs/nfs/internal.h +++ b/fs/nfs/internal.h @@ -123,7 +123,7 @@ struct nfs_parsed_mount_data { unsigned short protocol; } nfs_server; - struct security_mnt_opts lsm_opts; + void *lsm_opts; struct net *net; }; diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 300bdd1d4a09..1943de8f9d29 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -929,7 +929,7 @@ static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void) data->minorversion = 0; data->need_mount = true; data->net = current->nsproxy->net_ns; - security_init_mnt_opts(&data->lsm_opts); + data->lsm_opts = NULL; } return data; } @@ -2294,7 +2294,7 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data) /* compare new mount options with old ones */ error = nfs_compare_remount_data(nfss, data); if (!error) - error = security_sb_remount(sb, &data->lsm_opts); + error = security_sb_remount(sb, data->lsm_opts); out: nfs_free_parsed_mount_data(data); return error; @@ -2534,7 +2534,7 @@ int nfs_set_sb_security(struct super_block *s, struct dentry *mntroot, if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL) kflags |= SECURITY_LSM_NATIVE_LABELS; - error = security_sb_set_mnt_opts(s, &mount_info->parsed->lsm_opts, + error = security_sb_set_mnt_opts(s, mount_info->parsed->lsm_opts, kflags, &kflags_out); if (error) goto err; diff --git a/fs/super.c b/fs/super.c index 1f75fe312597..a5511c4ba69b 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1247,12 +1247,10 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) struct dentry *root; struct super_block *sb; int error = -ENOMEM; - struct security_mnt_opts opts; - - security_init_mnt_opts(&opts); + void *sec_opts = NULL; if (data && !(type->fs_flags & FS_BINARY_MOUNTDATA)) { - error = security_sb_eat_lsm_opts(data, &opts); + error = security_sb_eat_lsm_opts(data, &sec_opts); if (error) return ERR_PTR(error); } @@ -1275,7 +1273,7 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) smp_wmb(); sb->s_flags |= SB_BORN; - error = security_sb_set_mnt_opts(sb, &opts, 0, NULL); + error = security_sb_set_mnt_opts(sb, sec_opts, 0, NULL); if (error) goto out_sb; @@ -1295,13 +1293,13 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) "negative value (%lld)\n", type->name, sb->s_maxbytes); up_write(&sb->s_umount); - security_free_mnt_opts(&opts); + security_free_mnt_opts(&sec_opts); return root; out_sb: dput(root); deactivate_locked_super(sb); out_free_secdata: - security_free_mnt_opts(&opts); + security_free_mnt_opts(&sec_opts); return ERR_PTR(error); } diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index c418909c178c..a9c541f5732e 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1461,9 +1461,9 @@ union security_list_options { int (*sb_alloc_security)(struct super_block *sb); void (*sb_free_security)(struct super_block *sb); - int (*sb_eat_lsm_opts)(char *orig, struct security_mnt_opts *opts); - int (*sb_remount)(struct super_block *sb, - struct security_mnt_opts *opts); + void (*sb_free_mnt_opts)(void *mnt_opts); + int (*sb_eat_lsm_opts)(char *orig, void **mnt_opts); + int (*sb_remount)(struct super_block *sb, void *mnt_opts); int (*sb_kern_mount)(struct super_block *sb); int (*sb_show_options)(struct seq_file *m, struct super_block *sb); int (*sb_statfs)(struct dentry *dentry); @@ -1472,14 +1472,14 @@ union security_list_options { int (*sb_umount)(struct vfsmount *mnt, int flags); int (*sb_pivotroot)(const struct path *old_path, const struct path *new_path); int (*sb_set_mnt_opts)(struct super_block *sb, - struct security_mnt_opts *opts, + void *mnt_opts, unsigned long kern_flags, unsigned long *set_kern_flags); int (*sb_clone_mnt_opts)(const struct super_block *oldsb, struct super_block *newsb, unsigned long kern_flags, unsigned long *set_kern_flags); - int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts); + int (*sb_parse_opts_str)(char *options, void **mnt_opts); int (*dentry_init_security)(struct dentry *dentry, int mode, const struct qstr *name, void **ctx, u32 *ctxlen); @@ -1801,6 +1801,7 @@ struct security_hook_heads { struct hlist_head bprm_committed_creds; struct hlist_head sb_alloc_security; struct hlist_head sb_free_security; + struct hlist_head sb_free_mnt_opts; struct hlist_head sb_eat_lsm_opts; struct hlist_head sb_remount; struct hlist_head sb_kern_mount; diff --git a/include/linux/security.h b/include/linux/security.h index d00093363570..4bca0be95b7a 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -192,26 +192,6 @@ int call_lsm_notifier(enum lsm_event event, void *data); int register_lsm_notifier(struct notifier_block *nb); int unregister_lsm_notifier(struct notifier_block *nb); -static inline void security_init_mnt_opts(struct security_mnt_opts *opts) -{ - opts->mnt_opts = NULL; - opts->mnt_opts_flags = NULL; - opts->num_mnt_opts = 0; -} - -static inline void security_free_mnt_opts(struct security_mnt_opts *opts) -{ - int i; - if (opts->mnt_opts) - for (i = 0; i < opts->num_mnt_opts; i++) - kfree(opts->mnt_opts[i]); - kfree(opts->mnt_opts); - opts->mnt_opts = NULL; - kfree(opts->mnt_opts_flags); - opts->mnt_opts_flags = NULL; - opts->num_mnt_opts = 0; -} - /* prototypes */ extern int security_init(void); @@ -248,8 +228,9 @@ void security_bprm_committing_creds(struct linux_binprm *bprm); void security_bprm_committed_creds(struct linux_binprm *bprm); int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); -int security_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts); -int security_sb_remount(struct super_block *sb, struct security_mnt_opts *opts); +void security_free_mnt_opts(void **mnt_opts); +int security_sb_eat_lsm_opts(char *options, void **mnt_opts); +int security_sb_remount(struct super_block *sb, void *mnt_opts); int security_sb_kern_mount(struct super_block *sb); int security_sb_show_options(struct seq_file *m, struct super_block *sb); int security_sb_statfs(struct dentry *dentry); @@ -258,14 +239,14 @@ int security_sb_mount(const char *dev_name, const struct path *path, int security_sb_umount(struct vfsmount *mnt, int flags); int security_sb_pivotroot(const struct path *old_path, const struct path *new_path); int security_sb_set_mnt_opts(struct super_block *sb, - struct security_mnt_opts *opts, + void *mnt_opts, unsigned long kern_flags, unsigned long *set_kern_flags); int security_sb_clone_mnt_opts(const struct super_block *oldsb, struct super_block *newsb, unsigned long kern_flags, unsigned long *set_kern_flags); -int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts); +int security_sb_parse_opts_str(char *options, void **mnt_opts); int security_dentry_init_security(struct dentry *dentry, int mode, const struct qstr *name, void **ctx, u32 *ctxlen); @@ -421,11 +402,7 @@ static inline int unregister_lsm_notifier(struct notifier_block *nb) return 0; } -static inline void security_init_mnt_opts(struct security_mnt_opts *opts) -{ -} - -static inline void security_free_mnt_opts(struct security_mnt_opts *opts) +static inline void security_free_mnt_opts(void **mnt_opts) { } @@ -556,13 +533,13 @@ static inline void security_sb_free(struct super_block *sb) { } static inline int security_sb_eat_lsm_opts(char *options, - struct security_mnt_opts *opts) + void **mnt_opts) { return 0; } static inline int security_sb_remount(struct super_block *sb, - struct security_mnt_opts *opts) + void *mnt_opts) { return 0; } @@ -602,7 +579,7 @@ static inline int security_sb_pivotroot(const struct path *old_path, } static inline int security_sb_set_mnt_opts(struct super_block *sb, - struct security_mnt_opts *opts, + void *mnt_opts, unsigned long kern_flags, unsigned long *set_kern_flags) { @@ -617,7 +594,7 @@ static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb, return 0; } -static inline int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts) +static inline int security_sb_parse_opts_str(char *options, void **mnt_opts) { return 0; } diff --git a/security/security.c b/security/security.c index feb18c925349..b7a5a0051807 100644 --- a/security/security.c +++ b/security/security.c @@ -384,16 +384,25 @@ void security_sb_free(struct super_block *sb) call_void_hook(sb_free_security, sb); } -int security_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) +void security_free_mnt_opts(void **mnt_opts) { - return call_int_hook(sb_eat_lsm_opts, 0, options, opts); + if (!*mnt_opts) + return; + call_void_hook(sb_free_mnt_opts, *mnt_opts); + *mnt_opts = NULL; +} +EXPORT_SYMBOL(security_free_mnt_opts); + +int security_sb_eat_lsm_opts(char *options, void **mnt_opts) +{ + return call_int_hook(sb_eat_lsm_opts, 0, options, mnt_opts); } EXPORT_SYMBOL(security_sb_eat_lsm_opts); int security_sb_remount(struct super_block *sb, - struct security_mnt_opts *opts) + void *mnt_opts) { - return call_int_hook(sb_remount, 0, sb, opts); + return call_int_hook(sb_remount, 0, sb, mnt_opts); } EXPORT_SYMBOL(security_sb_remount); @@ -429,13 +438,13 @@ int security_sb_pivotroot(const struct path *old_path, const struct path *new_pa } int security_sb_set_mnt_opts(struct super_block *sb, - struct security_mnt_opts *opts, + void *mnt_opts, unsigned long kern_flags, unsigned long *set_kern_flags) { return call_int_hook(sb_set_mnt_opts, - opts->num_mnt_opts ? -EOPNOTSUPP : 0, sb, - opts, kern_flags, set_kern_flags); + mnt_opts ? -EOPNOTSUPP : 0, sb, + mnt_opts, kern_flags, set_kern_flags); } EXPORT_SYMBOL(security_sb_set_mnt_opts); @@ -449,9 +458,9 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb, } EXPORT_SYMBOL(security_sb_clone_mnt_opts); -int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts) +int security_sb_parse_opts_str(char *options, void **mnt_opts) { - return call_int_hook(sb_parse_opts_str, 0, options, opts); + return call_int_hook(sb_parse_opts_str, 0, options, mnt_opts); } EXPORT_SYMBOL(security_sb_parse_opts_str); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 11cf2feb27b3..caf7ca7abfc1 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -433,6 +433,19 @@ static void superblock_free_security(struct super_block *sb) kfree(sbsec); } +static void selinux_free_mnt_opts(void *mnt_opts) +{ + struct security_mnt_opts *opts = mnt_opts; + int i; + + if (opts->mnt_opts) + for (i = 0; i < opts->num_mnt_opts; i++) + kfree(opts->mnt_opts[i]); + kfree(opts->mnt_opts); + kfree(opts->mnt_opts_flags); + kfree(opts); +} + static inline int inode_doinit(struct inode *inode) { return inode_doinit_with_dentry(inode, NULL); @@ -616,7 +629,7 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, * labeling information. */ static int selinux_set_mnt_opts(struct super_block *sb, - struct security_mnt_opts *opts, + void *mnt_opts, unsigned long kern_flags, unsigned long *set_kern_flags) { @@ -628,9 +641,10 @@ static int selinux_set_mnt_opts(struct super_block *sb, struct inode_security_struct *root_isec; u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0; u32 defcontext_sid = 0; - char **mount_options = opts->mnt_opts; - int *flags = opts->mnt_opts_flags; - int num_opts = opts->num_mnt_opts; + struct security_mnt_opts *opts = mnt_opts; + char **mount_options = opts ? opts->mnt_opts : NULL; + int *flags = opts ? opts->mnt_opts_flags : NULL; + int num_opts = opts ? opts->num_mnt_opts : 0; mutex_lock(&sbsec->lock); @@ -982,12 +996,20 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, } static int selinux_parse_opts_str(char *options, - struct security_mnt_opts *opts) + void **mnt_opts) { char *p; char *context = NULL, *defcontext = NULL; char *fscontext = NULL, *rootcontext = NULL; int rc, num_mnt_opts = 0; + struct security_mnt_opts *opts = *mnt_opts; + + if (!opts) { + opts = kzalloc(sizeof(struct security_mnt_opts), GFP_KERNEL); + *mnt_opts = opts; + if (!opts) + return -ENOMEM; + } opts->num_mnt_opts = 0; @@ -1094,7 +1116,7 @@ static int selinux_parse_opts_str(char *options, return 0; out_err: - security_free_mnt_opts(opts); + security_free_mnt_opts(mnt_opts); kfree(context); kfree(defcontext); kfree(fscontext); @@ -2714,7 +2736,7 @@ static int selinux_sb_copy_data(char *orig, char *copy) return rc; } -static int selinux_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) +static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts) { char *s = (char *)get_zeroed_page(GFP_KERNEL); int err; @@ -2723,14 +2745,14 @@ static int selinux_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts return -ENOMEM; err = selinux_sb_copy_data(options, s); if (!err) - err = selinux_parse_opts_str(s, opts); + err = selinux_parse_opts_str(s, mnt_opts); free_page((unsigned long)s); return err; } -static int selinux_sb_remount(struct super_block *sb, - struct security_mnt_opts *opts) +static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) { + struct security_mnt_opts *opts = mnt_opts; int i, *flags; char **mount_options; struct superblock_security_struct *sbsec = sb->s_security; @@ -2738,6 +2760,9 @@ static int selinux_sb_remount(struct super_block *sb, if (!(sbsec->flags & SE_SBINITIALIZED)) return 0; + if (!opts) + return 0; + mount_options = opts->mnt_opts; flags = opts->mnt_opts_flags; @@ -6782,6 +6807,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security), LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts), + LSM_HOOK_INIT(sb_free_mnt_opts, selinux_free_mnt_opts), LSM_HOOK_INIT(sb_remount, selinux_sb_remount), LSM_HOOK_INIT(sb_kern_mount, selinux_sb_kern_mount), LSM_HOOK_INIT(sb_show_options, selinux_sb_show_options), @@ -7051,11 +7077,7 @@ static __init int selinux_init(void) static void delayed_superblock_init(struct super_block *sb, void *unused) { - struct security_mnt_opts opts; - - security_init_mnt_opts(&opts); - selinux_set_mnt_opts(sb, &opts, 0, NULL); - security_free_mnt_opts(&opts); + selinux_set_mnt_opts(sb, NULL, 0, NULL); } void selinux_complete_init(void) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 835cca277c2a..81a8112975d4 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -567,6 +567,19 @@ static void smack_sb_free_security(struct super_block *sb) sb->s_security = NULL; } +static void smack_free_mnt_opts(void *mnt_opts) +{ + struct security_mnt_opts *opts = mnt_opts; + int i; + + if (opts->mnt_opts) + for (i = 0; i < opts->num_mnt_opts; i++) + kfree(opts->mnt_opts[i]); + kfree(opts->mnt_opts); + kfree(opts->mnt_opts_flags); + kfree(opts); +} + /** * smack_sb_copy_data - copy mount options data for processing * @orig: where to start @@ -624,8 +637,9 @@ static int smack_sb_copy_data(char *orig, char *smackopts) * converts Smack specific mount options to generic security option format */ static int smack_parse_opts_str(char *options, - struct security_mnt_opts *opts) + void **mnt_opts) { + struct security_mnt_opts *opts = *mnt_opts; char *p; char *fsdefault = NULL; char *fsfloor = NULL; @@ -636,11 +650,17 @@ static int smack_parse_opts_str(char *options, int num_mnt_opts = 0; int token; - opts->num_mnt_opts = 0; - if (!options) return 0; + if (!opts) { + opts = kzalloc(sizeof(struct security_mnt_opts), GFP_KERNEL); + *mnt_opts = opts; + if (!opts) + return -ENOMEM; + } + opts->num_mnt_opts = 0; + while ((p = strsep(&options, ",")) != NULL) { substring_t args[MAX_OPT_ARGS]; @@ -735,11 +755,11 @@ static int smack_parse_opts_str(char *options, kfree(fshat); kfree(fsroot); kfree(fstransmute); - security_free_mnt_opts(opts); + security_free_mnt_opts(mnt_opts); return rc; } -static int smack_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) +static int smack_sb_eat_lsm_opts(char *options, void **mnt_opts) { char *s = (char *)get_zeroed_page(GFP_KERNEL); int err; @@ -748,7 +768,7 @@ static int smack_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) return -ENOMEM; err = smack_sb_copy_data(options, s); if (!err) - err = smack_parse_opts_str(s, opts); + err = smack_parse_opts_str(s, mnt_opts); free_page((unsigned long)s); return err; } @@ -766,7 +786,7 @@ static int smack_sb_eat_lsm_opts(char *options, struct security_mnt_opts *opts) * labels. */ static int smack_set_mnt_opts(struct super_block *sb, - struct security_mnt_opts *opts, + void *mnt_opts, unsigned long kern_flags, unsigned long *set_kern_flags) { @@ -776,7 +796,8 @@ static int smack_set_mnt_opts(struct super_block *sb, struct inode_smack *isp; struct smack_known *skp; int i; - int num_opts = opts->num_mnt_opts; + struct security_mnt_opts *opts = mnt_opts; + int num_opts = opts ? opts->num_mnt_opts : 0; int transmute = 0; if (sp->smk_flags & SMK_SB_INITIALIZED) @@ -4651,6 +4672,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_alloc_security, smack_sb_alloc_security), LSM_HOOK_INIT(sb_free_security, smack_sb_free_security), + LSM_HOOK_INIT(sb_free_mnt_opts, smack_free_mnt_opts), LSM_HOOK_INIT(sb_eat_lsm_opts, smack_sb_eat_lsm_opts), LSM_HOOK_INIT(sb_statfs, smack_sb_statfs), LSM_HOOK_INIT(sb_set_mnt_opts, smack_set_mnt_opts), From bd3236557bb256d6491df125e5e9d0393c70e4d2 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 13 Dec 2018 15:04:59 -0500 Subject: [PATCH 16/27] selinux: switch to private struct selinux_mnt_opts none of the convolutions needed, just 4 strings, TYVM... Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 262 +++++++++++++++------------------------ 1 file changed, 101 insertions(+), 161 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index caf7ca7abfc1..238907d69e8b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -433,16 +433,17 @@ static void superblock_free_security(struct super_block *sb) kfree(sbsec); } +struct selinux_mnt_opts { + const char *fscontext, *context, *rootcontext, *defcontext; +}; + static void selinux_free_mnt_opts(void *mnt_opts) { - struct security_mnt_opts *opts = mnt_opts; - int i; - - if (opts->mnt_opts) - for (i = 0; i < opts->num_mnt_opts; i++) - kfree(opts->mnt_opts[i]); - kfree(opts->mnt_opts); - kfree(opts->mnt_opts_flags); + struct selinux_mnt_opts *opts = mnt_opts; + kfree(opts->fscontext); + kfree(opts->context); + kfree(opts->rootcontext); + kfree(opts->defcontext); kfree(opts); } @@ -624,6 +625,17 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, return 0; } +static int parse_sid(struct super_block *sb, const char *s, u32 *sid) +{ + int rc = security_context_str_to_sid(&selinux_state, s, + sid, GFP_KERNEL); + if (rc) + pr_warn("SELinux: security_context_str_to_sid" + "(%s) failed for (dev %s, type %s) errno=%d\n", + s, sb->s_id, sb->s_type->name, rc); + return rc; +} + /* * Allow filesystems with binary mount data to explicitly set mount point * labeling information. @@ -634,22 +646,18 @@ static int selinux_set_mnt_opts(struct super_block *sb, unsigned long *set_kern_flags) { const struct cred *cred = current_cred(); - int rc = 0, i; struct superblock_security_struct *sbsec = sb->s_security; - const char *name = sb->s_type->name; struct dentry *root = sbsec->sb->s_root; + struct selinux_mnt_opts *opts = mnt_opts; struct inode_security_struct *root_isec; u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0; u32 defcontext_sid = 0; - struct security_mnt_opts *opts = mnt_opts; - char **mount_options = opts ? opts->mnt_opts : NULL; - int *flags = opts ? opts->mnt_opts_flags : NULL; - int num_opts = opts ? opts->num_mnt_opts : 0; + int rc = 0; mutex_lock(&sbsec->lock); if (!selinux_state.initialized) { - if (!num_opts) { + if (!opts) { /* Defer initialization until selinux_complete_init, after the initial policy is loaded and the security server is ready to handle calls. */ @@ -679,7 +687,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, * will be used for both mounts) */ if ((sbsec->flags & SE_SBINITIALIZED) && (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) - && (num_opts == 0)) + && !opts) goto out; root_isec = backing_inode_security_novalidate(root); @@ -689,68 +697,48 @@ static int selinux_set_mnt_opts(struct super_block *sb, * also check if someone is trying to mount the same sb more * than once with different security options. */ - for (i = 0; i < num_opts; i++) { - u32 sid; - - if (flags[i] == SBLABEL_MNT) - continue; - rc = security_context_str_to_sid(&selinux_state, - mount_options[i], &sid, - GFP_KERNEL); - if (rc) { - pr_warn("SELinux: security_context_str_to_sid" - "(%s) failed for (dev %s, type %s) errno=%d\n", - mount_options[i], sb->s_id, name, rc); - goto out; - } - switch (flags[i]) { - case FSCONTEXT_MNT: - fscontext_sid = sid; - + if (opts) { + if (opts->fscontext) { + rc = parse_sid(sb, opts->fscontext, &fscontext_sid); + if (rc) + goto out; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, fscontext_sid)) goto out_double_mount; - sbsec->flags |= FSCONTEXT_MNT; - break; - case CONTEXT_MNT: - context_sid = sid; - + } + if (opts->context) { + rc = parse_sid(sb, opts->context, &context_sid); + if (rc) + goto out; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, context_sid)) goto out_double_mount; - sbsec->flags |= CONTEXT_MNT; - break; - case ROOTCONTEXT_MNT: - rootcontext_sid = sid; - + } + if (opts->rootcontext) { + rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid); + if (rc) + goto out; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, rootcontext_sid)) goto out_double_mount; - sbsec->flags |= ROOTCONTEXT_MNT; - - break; - case DEFCONTEXT_MNT: - defcontext_sid = sid; - + } + if (opts->defcontext) { + rc = parse_sid(sb, opts->defcontext, &defcontext_sid); + if (rc) + goto out; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, defcontext_sid)) goto out_double_mount; - sbsec->flags |= DEFCONTEXT_MNT; - - break; - default: - rc = -EINVAL; - goto out; } } if (sbsec->flags & SE_SBINITIALIZED) { /* previously mounted with options, but not on this attempt? */ - if ((sbsec->flags & SE_MNTMASK) && !num_opts) + if ((sbsec->flags & SE_MNTMASK) && !opts) goto out_double_mount; rc = 0; goto out; @@ -883,7 +871,8 @@ static int selinux_set_mnt_opts(struct super_block *sb, out_double_mount: rc = -EINVAL; pr_warn("SELinux: mount invalid. Same superblock, different " - "security settings for (dev %s, type %s)\n", sb->s_id, name); + "security settings for (dev %s, type %s)\n", sb->s_id, + sb->s_type->name); goto out; } @@ -998,20 +987,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, static int selinux_parse_opts_str(char *options, void **mnt_opts) { + struct selinux_mnt_opts *opts = *mnt_opts; char *p; - char *context = NULL, *defcontext = NULL; - char *fscontext = NULL, *rootcontext = NULL; - int rc, num_mnt_opts = 0; - struct security_mnt_opts *opts = *mnt_opts; - - if (!opts) { - opts = kzalloc(sizeof(struct security_mnt_opts), GFP_KERNEL); - *mnt_opts = opts; - if (!opts) - return -ENOMEM; - } - - opts->num_mnt_opts = 0; + int rc; /* Standard string-based options. */ while ((p = strsep(&options, "|")) != NULL) { @@ -1023,54 +1001,60 @@ static int selinux_parse_opts_str(char *options, token = match_token(p, tokens, args); + if (!opts) { + opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); + if (!opts) + return -ENOMEM; + } + switch (token) { case Opt_context: - if (context || defcontext) { + if (opts->context || opts->defcontext) { rc = -EINVAL; pr_warn(SEL_MOUNT_FAIL_MSG); goto out_err; } - context = match_strdup(&args[0]); - if (!context) { + opts->context = match_strdup(&args[0]); + if (!opts->context) { rc = -ENOMEM; goto out_err; } break; case Opt_fscontext: - if (fscontext) { + if (opts->fscontext) { rc = -EINVAL; pr_warn(SEL_MOUNT_FAIL_MSG); goto out_err; } - fscontext = match_strdup(&args[0]); - if (!fscontext) { + opts->fscontext = match_strdup(&args[0]); + if (!opts->fscontext) { rc = -ENOMEM; goto out_err; } break; case Opt_rootcontext: - if (rootcontext) { + if (opts->rootcontext) { rc = -EINVAL; pr_warn(SEL_MOUNT_FAIL_MSG); goto out_err; } - rootcontext = match_strdup(&args[0]); - if (!rootcontext) { + opts->rootcontext = match_strdup(&args[0]); + if (!opts->rootcontext) { rc = -ENOMEM; goto out_err; } break; case Opt_defcontext: - if (context || defcontext) { + if (opts->context || opts->defcontext) { rc = -EINVAL; pr_warn(SEL_MOUNT_FAIL_MSG); goto out_err; } - defcontext = match_strdup(&args[0]); - if (!defcontext) { + opts->defcontext = match_strdup(&args[0]); + if (!opts->defcontext) { rc = -ENOMEM; goto out_err; } @@ -1084,43 +1068,12 @@ static int selinux_parse_opts_str(char *options, } } - - rc = -ENOMEM; - opts->mnt_opts = kcalloc(NUM_SEL_MNT_OPTS, sizeof(char *), GFP_KERNEL); - if (!opts->mnt_opts) - goto out_err; - - opts->mnt_opts_flags = kcalloc(NUM_SEL_MNT_OPTS, sizeof(int), - GFP_KERNEL); - if (!opts->mnt_opts_flags) - goto out_err; - - if (fscontext) { - opts->mnt_opts[num_mnt_opts] = fscontext; - opts->mnt_opts_flags[num_mnt_opts++] = FSCONTEXT_MNT; - } - if (context) { - opts->mnt_opts[num_mnt_opts] = context; - opts->mnt_opts_flags[num_mnt_opts++] = CONTEXT_MNT; - } - if (rootcontext) { - opts->mnt_opts[num_mnt_opts] = rootcontext; - opts->mnt_opts_flags[num_mnt_opts++] = ROOTCONTEXT_MNT; - } - if (defcontext) { - opts->mnt_opts[num_mnt_opts] = defcontext; - opts->mnt_opts_flags[num_mnt_opts++] = DEFCONTEXT_MNT; - } - - opts->num_mnt_opts = num_mnt_opts; + *mnt_opts = opts; return 0; out_err: - security_free_mnt_opts(mnt_opts); - kfree(context); - kfree(defcontext); - kfree(fscontext); - kfree(rootcontext); + if (opts) + selinux_free_mnt_opts(opts); return rc; } @@ -2752,10 +2705,10 @@ static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts) static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) { - struct security_mnt_opts *opts = mnt_opts; - int i, *flags; - char **mount_options; + struct selinux_mnt_opts *opts = mnt_opts; struct superblock_security_struct *sbsec = sb->s_security; + u32 sid; + int rc; if (!(sbsec->flags & SE_SBINITIALIZED)) return 0; @@ -2763,48 +2716,35 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) if (!opts) return 0; - mount_options = opts->mnt_opts; - flags = opts->mnt_opts_flags; - - for (i = 0; i < opts->num_mnt_opts; i++) { - u32 sid; - int rc; - - if (flags[i] == SBLABEL_MNT) - continue; - rc = security_context_str_to_sid(&selinux_state, - mount_options[i], &sid, - GFP_KERNEL); - if (rc) { - pr_warn("SELinux: security_context_str_to_sid" - "(%s) failed for (dev %s, type %s) errno=%d\n", - mount_options[i], sb->s_id, sb->s_type->name, rc); + if (opts->fscontext) { + rc = parse_sid(sb, opts->fscontext, &sid); + if (rc) return rc; - } - switch (flags[i]) { - case FSCONTEXT_MNT: - if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) - goto out_bad_option; - break; - case CONTEXT_MNT: - if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, sid)) - goto out_bad_option; - break; - case ROOTCONTEXT_MNT: { - struct inode_security_struct *root_isec; - root_isec = backing_inode_security(sb->s_root); - - if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, sid)) - goto out_bad_option; - break; - } - case DEFCONTEXT_MNT: - if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, sid)) - goto out_bad_option; - break; - default: - return -EINVAL; - } + if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) + goto out_bad_option; + } + if (opts->context) { + rc = parse_sid(sb, opts->context, &sid); + if (rc) + return rc; + if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, sid)) + goto out_bad_option; + } + if (opts->rootcontext) { + struct inode_security_struct *root_isec; + root_isec = backing_inode_security(sb->s_root); + rc = parse_sid(sb, opts->rootcontext, &sid); + if (rc) + return rc; + if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, sid)) + goto out_bad_option; + } + if (opts->defcontext) { + rc = parse_sid(sb, opts->defcontext, &sid); + if (rc) + return rc; + if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, sid)) + goto out_bad_option; } return 0; From 12085b14a4440a6d12ff7966702c010df87caef0 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 13 Dec 2018 15:18:05 -0500 Subject: [PATCH 17/27] smack: switch to private smack_mnt_opts Reviewed-by: David Howells Signed-off-by: Al Viro --- security/smack/smack_lsm.c | 157 +++++++++++++------------------------ 1 file changed, 55 insertions(+), 102 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 81a8112975d4..99aec9f42be3 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -567,16 +567,18 @@ static void smack_sb_free_security(struct super_block *sb) sb->s_security = NULL; } +struct smack_mnt_opts { + const char *fsdefault, *fsfloor, *fshat, *fsroot, *fstransmute; +}; + static void smack_free_mnt_opts(void *mnt_opts) { - struct security_mnt_opts *opts = mnt_opts; - int i; - - if (opts->mnt_opts) - for (i = 0; i < opts->num_mnt_opts; i++) - kfree(opts->mnt_opts[i]); - kfree(opts->mnt_opts); - kfree(opts->mnt_opts_flags); + struct smack_mnt_opts *opts = mnt_opts; + kfree(opts->fsdefault); + kfree(opts->fsfloor); + kfree(opts->fshat); + kfree(opts->fsroot); + kfree(opts->fstransmute); kfree(opts); } @@ -639,28 +641,14 @@ static int smack_sb_copy_data(char *orig, char *smackopts) static int smack_parse_opts_str(char *options, void **mnt_opts) { - struct security_mnt_opts *opts = *mnt_opts; + struct smack_mnt_opts *opts = *mnt_opts; char *p; - char *fsdefault = NULL; - char *fsfloor = NULL; - char *fshat = NULL; - char *fsroot = NULL; - char *fstransmute = NULL; int rc = -ENOMEM; - int num_mnt_opts = 0; int token; if (!options) return 0; - if (!opts) { - opts = kzalloc(sizeof(struct security_mnt_opts), GFP_KERNEL); - *mnt_opts = opts; - if (!opts) - return -ENOMEM; - } - opts->num_mnt_opts = 0; - while ((p = strsep(&options, ",")) != NULL) { substring_t args[MAX_OPT_ARGS]; @@ -669,40 +657,46 @@ static int smack_parse_opts_str(char *options, token = match_token(p, smk_mount_tokens, args); + if (!opts) { + opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL); + if (!opts) + return -ENOMEM; + } + switch (token) { case Opt_fsdefault: - if (fsdefault) + if (opts->fsdefault) goto out_opt_err; - fsdefault = match_strdup(&args[0]); - if (!fsdefault) + opts->fsdefault = match_strdup(&args[0]); + if (!opts->fsdefault) goto out_err; break; case Opt_fsfloor: - if (fsfloor) + if (opts->fsfloor) goto out_opt_err; - fsfloor = match_strdup(&args[0]); - if (!fsfloor) + opts->fsfloor = match_strdup(&args[0]); + if (!opts->fsfloor) goto out_err; break; case Opt_fshat: - if (fshat) + if (opts->fshat) goto out_opt_err; - fshat = match_strdup(&args[0]); - if (!fshat) + opts->fshat = match_strdup(&args[0]); + if (!opts->fshat) goto out_err; break; case Opt_fsroot: - if (fsroot) + if (opts->fsroot) goto out_opt_err; - fsroot = match_strdup(&args[0]); - if (!fsroot) + opts->fsroot = match_strdup(&args[0]); + if (!opts->fsroot) goto out_err; break; case Opt_fstransmute: - if (fstransmute) + if (opts->fstransmute) goto out_opt_err; - fstransmute = match_strdup(&args[0]); - if (!fstransmute) + opts->fstransmute = match_strdup(&args[0]); + if (!opts->fstransmute) goto out_err; break; default: @@ -711,38 +705,7 @@ static int smack_parse_opts_str(char *options, goto out_err; } } - - opts->mnt_opts = kcalloc(NUM_SMK_MNT_OPTS, sizeof(char *), GFP_KERNEL); - if (!opts->mnt_opts) - goto out_err; - - opts->mnt_opts_flags = kcalloc(NUM_SMK_MNT_OPTS, sizeof(int), - GFP_KERNEL); - if (!opts->mnt_opts_flags) - goto out_err; - - if (fsdefault) { - opts->mnt_opts[num_mnt_opts] = fsdefault; - opts->mnt_opts_flags[num_mnt_opts++] = FSDEFAULT_MNT; - } - if (fsfloor) { - opts->mnt_opts[num_mnt_opts] = fsfloor; - opts->mnt_opts_flags[num_mnt_opts++] = FSFLOOR_MNT; - } - if (fshat) { - opts->mnt_opts[num_mnt_opts] = fshat; - opts->mnt_opts_flags[num_mnt_opts++] = FSHAT_MNT; - } - if (fsroot) { - opts->mnt_opts[num_mnt_opts] = fsroot; - opts->mnt_opts_flags[num_mnt_opts++] = FSROOT_MNT; - } - if (fstransmute) { - opts->mnt_opts[num_mnt_opts] = fstransmute; - opts->mnt_opts_flags[num_mnt_opts++] = FSTRANS_MNT; - } - - opts->num_mnt_opts = num_mnt_opts; + *mnt_opts = opts; return 0; out_opt_err: @@ -750,12 +713,8 @@ static int smack_parse_opts_str(char *options, pr_warn("Smack: duplicate mount options\n"); out_err: - kfree(fsdefault); - kfree(fsfloor); - kfree(fshat); - kfree(fsroot); - kfree(fstransmute); - security_free_mnt_opts(mnt_opts); + if (opts) + smack_free_mnt_opts(opts); return rc; } @@ -795,10 +754,8 @@ static int smack_set_mnt_opts(struct super_block *sb, struct superblock_smack *sp = sb->s_security; struct inode_smack *isp; struct smack_known *skp; - int i; - struct security_mnt_opts *opts = mnt_opts; - int num_opts = opts ? opts->num_mnt_opts : 0; - int transmute = 0; + struct smack_mnt_opts *opts = mnt_opts; + bool transmute = false; if (sp->smk_flags & SMK_SB_INITIALIZED) return 0; @@ -807,7 +764,7 @@ static int smack_set_mnt_opts(struct super_block *sb, /* * Unprivileged mounts don't get to specify Smack values. */ - if (num_opts) + if (opts) return -EPERM; /* * Unprivileged mounts get root and default from the caller. @@ -823,48 +780,44 @@ static int smack_set_mnt_opts(struct super_block *sb, if (sb->s_user_ns != &init_user_ns && sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC && sb->s_magic != RAMFS_MAGIC) { - transmute = 1; + transmute = true; sp->smk_flags |= SMK_SB_UNTRUSTED; } } sp->smk_flags |= SMK_SB_INITIALIZED; - for (i = 0; i < num_opts; i++) { - switch (opts->mnt_opts_flags[i]) { - case FSDEFAULT_MNT: - skp = smk_import_entry(opts->mnt_opts[i], 0); + if (opts) { + if (opts->fsdefault) { + skp = smk_import_entry(opts->fsdefault, 0); if (IS_ERR(skp)) return PTR_ERR(skp); sp->smk_default = skp; - break; - case FSFLOOR_MNT: - skp = smk_import_entry(opts->mnt_opts[i], 0); + } + if (opts->fsfloor) { + skp = smk_import_entry(opts->fsfloor, 0); if (IS_ERR(skp)) return PTR_ERR(skp); sp->smk_floor = skp; - break; - case FSHAT_MNT: - skp = smk_import_entry(opts->mnt_opts[i], 0); + } + if (opts->fshat) { + skp = smk_import_entry(opts->fshat, 0); if (IS_ERR(skp)) return PTR_ERR(skp); sp->smk_hat = skp; - break; - case FSROOT_MNT: - skp = smk_import_entry(opts->mnt_opts[i], 0); + } + if (opts->fsroot) { + skp = smk_import_entry(opts->fsroot, 0); if (IS_ERR(skp)) return PTR_ERR(skp); sp->smk_root = skp; - break; - case FSTRANS_MNT: - skp = smk_import_entry(opts->mnt_opts[i], 0); + } + if (opts->fstransmute) { + skp = smk_import_entry(opts->fstransmute, 0); if (IS_ERR(skp)) return PTR_ERR(skp); sp->smk_root = skp; - transmute = 1; - break; - default: - break; + transmute = true; } } From 84d8c4a5ef696ca96fa7a8d64db9222658b9d142 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 13 Dec 2018 15:18:44 -0500 Subject: [PATCH 18/27] LSM: bury struct security_mnt_opts no users left Reviewed-by: David Howells Signed-off-by: Al Viro --- include/linux/security.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/include/linux/security.h b/include/linux/security.h index 4bca0be95b7a..ae8d5ac5882e 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -182,12 +182,6 @@ static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id) #ifdef CONFIG_SECURITY -struct security_mnt_opts { - char **mnt_opts; - int *mnt_opts_flags; - int num_mnt_opts; -}; - int call_lsm_notifier(enum lsm_event event, void *data); int register_lsm_notifier(struct notifier_block *nb); int unregister_lsm_notifier(struct notifier_block *nb); @@ -384,8 +378,6 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); #else /* CONFIG_SECURITY */ -struct security_mnt_opts { -}; static inline int call_lsm_notifier(enum lsm_event event, void *data) { From ba6418623385abf19a6c15cf0b1cfaacfdf9afc8 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Dec 2018 20:28:15 -0500 Subject: [PATCH 19/27] selinux: new helper - selinux_add_opt() the guts of the loop in selinux_parse_opts_str() - takes one (already recognized) option and adds it to growing selinux_mnt_opts. Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 126 ++++++++++++++++----------------------- 1 file changed, 53 insertions(+), 73 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 238907d69e8b..26ec7d67e15d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -984,97 +984,77 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, return rc; } +static int selinux_add_opt(int token, const char *s, void **mnt_opts) +{ + struct selinux_mnt_opts *opts = *mnt_opts; + + if (!opts) { + opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); + if (!opts) + return -ENOMEM; + *mnt_opts = opts; + } + if (!s) + return -ENOMEM; + switch (token) { + case Opt_context: + if (opts->context || opts->defcontext) + goto Einval; + opts->context = s; + break; + case Opt_fscontext: + if (opts->fscontext) + goto Einval; + opts->fscontext = s; + break; + case Opt_rootcontext: + if (opts->rootcontext) + goto Einval; + opts->rootcontext = s; + break; + case Opt_defcontext: + if (opts->context || opts->defcontext) + goto Einval; + opts->defcontext = s; + break; + } + return 0; +Einval: + pr_warn(SEL_MOUNT_FAIL_MSG); + kfree(s); + return -EINVAL; +} + static int selinux_parse_opts_str(char *options, void **mnt_opts) { - struct selinux_mnt_opts *opts = *mnt_opts; char *p; - int rc; /* Standard string-based options. */ while ((p = strsep(&options, "|")) != NULL) { - int token; + int token, rc; substring_t args[MAX_OPT_ARGS]; + const char *arg; if (!*p) continue; token = match_token(p, tokens, args); - if (!opts) { - opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); - if (!opts) - return -ENOMEM; - } - - switch (token) { - case Opt_context: - if (opts->context || opts->defcontext) { - rc = -EINVAL; - pr_warn(SEL_MOUNT_FAIL_MSG); - goto out_err; + if (token == Opt_labelsupport) /* eaten and completely ignored */ + continue; + arg = match_strdup(&args[0]); + rc = selinux_add_opt(token, arg, mnt_opts); + if (unlikely(rc)) { + kfree(arg); + if (*mnt_opts) { + selinux_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; } - opts->context = match_strdup(&args[0]); - if (!opts->context) { - rc = -ENOMEM; - goto out_err; - } - break; - - case Opt_fscontext: - if (opts->fscontext) { - rc = -EINVAL; - pr_warn(SEL_MOUNT_FAIL_MSG); - goto out_err; - } - opts->fscontext = match_strdup(&args[0]); - if (!opts->fscontext) { - rc = -ENOMEM; - goto out_err; - } - break; - - case Opt_rootcontext: - if (opts->rootcontext) { - rc = -EINVAL; - pr_warn(SEL_MOUNT_FAIL_MSG); - goto out_err; - } - opts->rootcontext = match_strdup(&args[0]); - if (!opts->rootcontext) { - rc = -ENOMEM; - goto out_err; - } - break; - - case Opt_defcontext: - if (opts->context || opts->defcontext) { - rc = -EINVAL; - pr_warn(SEL_MOUNT_FAIL_MSG); - goto out_err; - } - opts->defcontext = match_strdup(&args[0]); - if (!opts->defcontext) { - rc = -ENOMEM; - goto out_err; - } - break; - case Opt_labelsupport: - break; - default: - rc = -EINVAL; - pr_warn("SELinux: unknown mount option\n"); - goto out_err; - + return rc; } } - *mnt_opts = opts; return 0; - -out_err: - if (opts) - selinux_free_mnt_opts(opts); - return rc; } static int show_sid(struct seq_file *m, u32 sid) From 169d68efb03b728588c209c682f14328eec485c0 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Dec 2018 22:44:50 -0500 Subject: [PATCH 20/27] selinux: switch away from match_token() It's not a good fit, unfortunately, and the next step will make it even less so. Open-code what we need here. Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 82 +++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 26ec7d67e15d..8f2285cb9029 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -459,19 +459,41 @@ enum { Opt_defcontext = 3, Opt_rootcontext = 4, Opt_labelsupport = 5, - Opt_nextmntopt = 6, }; -#define NUM_SEL_MNT_OPTS (Opt_nextmntopt - 1) - -static const match_table_t tokens = { - {Opt_context, CONTEXT_STR "%s"}, - {Opt_fscontext, FSCONTEXT_STR "%s"}, - {Opt_defcontext, DEFCONTEXT_STR "%s"}, - {Opt_rootcontext, ROOTCONTEXT_STR "%s"}, - {Opt_labelsupport, LABELSUPP_STR}, - {Opt_error, NULL}, +#define A(s, opt, has_arg) {s, sizeof(s) - 1, opt, has_arg} +static struct { + const char *name; + int len; + int opt; + bool has_arg; +} tokens[] = { + A("context", Opt_context, true), + A("fscontext", Opt_fscontext, true), + A("defcontext", Opt_defcontext, true), + A("rootcontext", Opt_rootcontext, true), + A("seclabel", Opt_labelsupport, false), }; +#undef A + +static int match_opt_prefix(char *s, int l, char **arg) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(tokens); i++) { + size_t len = tokens[i].len; + if (len > l || memcmp(s, tokens[i].name, len)) + continue; + if (tokens[i].has_arg) { + if (len == l || s[len] != '=') + continue; + *arg = s + len + 1; + } else if (len != l) + continue; + return tokens[i].opt; + } + return Opt_error; +} #define SEL_MOUNT_FAIL_MSG "SELinux: duplicate or incompatible mount options\n" @@ -988,6 +1010,9 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; + if (token == Opt_labelsupport) /* eaten and completely ignored */ + return 0; + if (!opts) { opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); if (!opts) @@ -1021,36 +1046,39 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) return 0; Einval: pr_warn(SEL_MOUNT_FAIL_MSG); - kfree(s); return -EINVAL; } static int selinux_parse_opts_str(char *options, void **mnt_opts) { - char *p; + char *p = options, *next; + int rc; /* Standard string-based options. */ - while ((p = strsep(&options, "|")) != NULL) { - int token, rc; - substring_t args[MAX_OPT_ARGS]; - const char *arg; + for (p = options; *p; p = next) { + int token, len; + char *arg = NULL; - if (!*p) + next = strchr(p, '|'); + if (next) { + len = next++ - p; + } else { + len = strlen(p); + next = p + len; + } + + if (!len) continue; - token = match_token(p, tokens, args); - - if (token == Opt_labelsupport) /* eaten and completely ignored */ - continue; - arg = match_strdup(&args[0]); + token = match_opt_prefix(p, len, &arg); + if (arg) + arg = kmemdup_nul(arg, p + len - arg, GFP_KERNEL); rc = selinux_add_opt(token, arg, mnt_opts); - if (unlikely(rc)) { + if (rc) { kfree(arg); - if (*mnt_opts) { - selinux_free_mnt_opts(*mnt_opts); - *mnt_opts = NULL; - } + selinux_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; return rc; } } From da3d76abb2e74c07b1cd620ee5e3b31227846c7c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 17 Dec 2018 10:14:16 -0500 Subject: [PATCH 21/27] selinux: regularize Opt_... names a bit Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8f2285cb9029..9b350070ed9e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -458,21 +458,21 @@ enum { Opt_fscontext = 2, Opt_defcontext = 3, Opt_rootcontext = 4, - Opt_labelsupport = 5, + Opt_seclabel = 5, }; -#define A(s, opt, has_arg) {s, sizeof(s) - 1, opt, has_arg} +#define A(s, has_arg) {#s, sizeof(#s) - 1, Opt_##s, has_arg} static struct { const char *name; int len; int opt; bool has_arg; } tokens[] = { - A("context", Opt_context, true), - A("fscontext", Opt_fscontext, true), - A("defcontext", Opt_defcontext, true), - A("rootcontext", Opt_rootcontext, true), - A("seclabel", Opt_labelsupport, false), + A(context, true), + A(fscontext, true), + A(defcontext, true), + A(rootcontext, true), + A(seclabel, false), }; #undef A @@ -1010,7 +1010,7 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; - if (token == Opt_labelsupport) /* eaten and completely ignored */ + if (token == Opt_seclabel) /* eaten and completely ignored */ return 0; if (!opts) { From 99dbbb593fe6b39153c15ea9b9c63ea911864cf2 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Dec 2018 21:56:23 -0500 Subject: [PATCH 22/27] selinux: rewrite selinux_sb_eat_lsm_opts() make it use selinux_add_opt() and avoid separate copies - gather non-LSM options by memmove() in place Reviewed-by: David Howells Signed-off-by: Al Viro --- security/selinux/hooks.c | 152 +++++++++++++++------------------------ 1 file changed, 57 insertions(+), 95 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9b350070ed9e..5336d6671c5c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2606,109 +2606,71 @@ static void selinux_sb_free_security(struct super_block *sb) superblock_free_security(sb); } -static inline int match_prefix(char *prefix, int plen, char *option, int olen) +static inline int opt_len(const char *s) { - if (plen > olen) - return 0; + bool open_quote = false; + int len; + char c; - return !memcmp(prefix, option, plen); -} - -static inline int selinux_option(char *option, int len) -{ - return (match_prefix(CONTEXT_STR, sizeof(CONTEXT_STR)-1, option, len) || - match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option, len) || - match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option, len) || - match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1, option, len) || - match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option, len)); -} - -static inline void take_option(char **to, char *from, int *first, int len) -{ - if (!*first) { - **to = ','; - *to += 1; - } else - *first = 0; - memcpy(*to, from, len); - *to += len; -} - -static inline void take_selinux_option(char **to, char *from, int *first, - int len) -{ - int current_size = 0; - - if (!*first) { - **to = '|'; - *to += 1; - } else - *first = 0; - - while (current_size < len) { - if (*from != '"') { - **to = *from; - *to += 1; - } - from += 1; - current_size += 1; - } -} - -static int selinux_sb_copy_data(char *orig, char *copy) -{ - int fnosec, fsec, rc = 0; - char *in_save, *in_curr, *in_end; - char *sec_curr, *nosec_save, *nosec; - int open_quote = 0; - - in_curr = orig; - sec_curr = copy; - - nosec = (char *)get_zeroed_page(GFP_KERNEL); - if (!nosec) { - rc = -ENOMEM; - goto out; - } - - nosec_save = nosec; - fnosec = fsec = 1; - in_save = in_end = orig; - - do { - if (*in_end == '"') + for (len = 0; (c = s[len]) != '\0'; len++) { + if (c == '"') open_quote = !open_quote; - if ((*in_end == ',' && open_quote == 0) || - *in_end == '\0') { - int len = in_end - in_curr; - - if (selinux_option(in_curr, len)) - take_selinux_option(&sec_curr, in_curr, &fsec, len); - else - take_option(&nosec, in_curr, &fnosec, len); - - in_curr = in_end + 1; - } - } while (*in_end++); - - strcpy(in_save, nosec_save); - free_page((unsigned long)nosec_save); -out: - return rc; + if (c == ',' && !open_quote) + break; + } + return len; } static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts) { - char *s = (char *)get_zeroed_page(GFP_KERNEL); - int err; + char *from = options; + char *to = options; + bool first = true; - if (!s) - return -ENOMEM; - err = selinux_sb_copy_data(options, s); - if (!err) - err = selinux_parse_opts_str(s, mnt_opts); - free_page((unsigned long)s); - return err; + while (1) { + int len = opt_len(from); + int token, rc; + char *arg = NULL; + + token = match_opt_prefix(from, len, &arg); + + if (token != Opt_error) { + char *p, *q; + + /* strip quotes */ + if (arg) { + for (p = q = arg; p < from + len; p++) { + char c = *p; + if (c != '"') + *q++ = c; + } + arg = kmemdup_nul(arg, q - arg, GFP_KERNEL); + } + rc = selinux_add_opt(token, arg, mnt_opts); + if (unlikely(rc)) { + kfree(arg); + if (*mnt_opts) { + selinux_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; + } + return rc; + } + } else { + if (!first) { // copy with preceding comma + from--; + len++; + } + if (to != from) + memmove(to, from, len); + to += len; + first = false; + } + if (!from[len]) + break; + from += len + 1; + } + *to = '\0'; + return 0; } static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) From 757cbe597fe8490c7c0a9650ebe5d60195f151d4 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Dec 2018 23:42:21 -0500 Subject: [PATCH 23/27] LSM: new method: ->sb_add_mnt_opt() Adding options to growing mnt_opts. NFS kludge with passing context= down into non-text-options mount switched to it, and with that the last use of ->sb_parse_opts_str() is gone. Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/nfs/super.c | 9 ++----- include/linux/lsm_hooks.h | 5 ++-- include/linux/security.h | 6 +++-- security/security.c | 8 +++--- security/selinux/hooks.c | 55 +++++++++++++++++--------------------- security/smack/smack_lsm.c | 1 - 6 files changed, 38 insertions(+), 46 deletions(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 1943de8f9d29..073eec2366f8 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -2070,14 +2070,9 @@ static int nfs23_validate_mount_data(void *options, if (data->context[0]){ #ifdef CONFIG_SECURITY_SELINUX int rc; - char *opts_str = kmalloc(sizeof(data->context) + 8, GFP_KERNEL); - if (!opts_str) - return -ENOMEM; - strcpy(opts_str, "context="); data->context[NFS_MAX_CONTEXT_LEN] = '\0'; - strcat(opts_str, &data->context[0]); - rc = security_sb_parse_opts_str(opts_str, &args->lsm_opts); - kfree(opts_str); + rc = security_add_mnt_opt("context", data->context, + strlen(data->context), &args->lsm_opts); if (rc) return rc; #else diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index a9c541f5732e..9a0bdf91e646 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1479,7 +1479,8 @@ union security_list_options { struct super_block *newsb, unsigned long kern_flags, unsigned long *set_kern_flags); - int (*sb_parse_opts_str)(char *options, void **mnt_opts); + int (*sb_add_mnt_opt)(const char *option, const char *val, int len, + void **mnt_opts); int (*dentry_init_security)(struct dentry *dentry, int mode, const struct qstr *name, void **ctx, u32 *ctxlen); @@ -1812,7 +1813,7 @@ struct security_hook_heads { struct hlist_head sb_pivotroot; struct hlist_head sb_set_mnt_opts; struct hlist_head sb_clone_mnt_opts; - struct hlist_head sb_parse_opts_str; + struct hlist_head sb_add_mnt_opt; struct hlist_head dentry_init_security; struct hlist_head dentry_create_files_as; #ifdef CONFIG_SECURITY_PATH diff --git a/include/linux/security.h b/include/linux/security.h index ae8d5ac5882e..dbfb5a66babb 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -240,7 +240,8 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb, struct super_block *newsb, unsigned long kern_flags, unsigned long *set_kern_flags); -int security_sb_parse_opts_str(char *options, void **mnt_opts); +int security_add_mnt_opt(const char *option, const char *val, + int len, void **mnt_opts); int security_dentry_init_security(struct dentry *dentry, int mode, const struct qstr *name, void **ctx, u32 *ctxlen); @@ -586,7 +587,8 @@ static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb, return 0; } -static inline int security_sb_parse_opts_str(char *options, void **mnt_opts) +static inline int security_add_mnt_opt(const char *option, const char *val, + int len, void **mnt_opts) { return 0; } diff --git a/security/security.c b/security/security.c index b7a5a0051807..c251278b0297 100644 --- a/security/security.c +++ b/security/security.c @@ -458,11 +458,13 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb, } EXPORT_SYMBOL(security_sb_clone_mnt_opts); -int security_sb_parse_opts_str(char *options, void **mnt_opts) +int security_add_mnt_opt(const char *option, const char *val, int len, + void **mnt_opts) { - return call_int_hook(sb_parse_opts_str, 0, options, mnt_opts); + return call_int_hook(sb_add_mnt_opt, -EINVAL, + option, val, len, mnt_opts); } -EXPORT_SYMBOL(security_sb_parse_opts_str); +EXPORT_SYMBOL(security_add_mnt_opt); int security_inode_alloc(struct inode *inode) { diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5336d6671c5c..5bc230327bc0 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1049,40 +1049,33 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) return -EINVAL; } -static int selinux_parse_opts_str(char *options, - void **mnt_opts) +static int selinux_add_mnt_opt(const char *option, const char *val, int len, + void **mnt_opts) { - char *p = options, *next; - int rc; + int token = Opt_error; + int rc, i; - /* Standard string-based options. */ - for (p = options; *p; p = next) { - int token, len; - char *arg = NULL; - - next = strchr(p, '|'); - if (next) { - len = next++ - p; - } else { - len = strlen(p); - next = p + len; - } - - if (!len) - continue; - - token = match_opt_prefix(p, len, &arg); - if (arg) - arg = kmemdup_nul(arg, p + len - arg, GFP_KERNEL); - rc = selinux_add_opt(token, arg, mnt_opts); - if (rc) { - kfree(arg); - selinux_free_mnt_opts(*mnt_opts); - *mnt_opts = NULL; - return rc; + for (i = 0; i < ARRAY_SIZE(tokens); i++) { + if (strcmp(option, tokens[i].name) == 0) { + token = tokens[i].opt; + break; } } - return 0; + + if (token == Opt_error) + return -EINVAL; + + if (token != Opt_seclabel) + val = kmemdup_nul(val, len, GFP_KERNEL); + rc = selinux_add_opt(token, val, mnt_opts); + if (unlikely(rc)) { + kfree(val); + if (*mnt_opts) { + selinux_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; + } + } + return rc; } static int show_sid(struct seq_file *m, u32 sid) @@ -6726,7 +6719,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_umount, selinux_umount), LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts), LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts), - LSM_HOOK_INIT(sb_parse_opts_str, selinux_parse_opts_str), + LSM_HOOK_INIT(sb_add_mnt_opt, selinux_add_mnt_opt), LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security), LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as), diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 99aec9f42be3..b607b1151e30 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4629,7 +4629,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_eat_lsm_opts, smack_sb_eat_lsm_opts), LSM_HOOK_INIT(sb_statfs, smack_sb_statfs), LSM_HOOK_INIT(sb_set_mnt_opts, smack_set_mnt_opts), - LSM_HOOK_INIT(sb_parse_opts_str, smack_parse_opts_str), LSM_HOOK_INIT(bprm_set_creds, smack_bprm_set_creds), From 55c0e5bd078eba2d41d76fa25d5d5e55f1ff09ee Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 16 Dec 2018 01:09:45 -0500 Subject: [PATCH 24/27] smack: take the guts of smack_parse_opts_str() into a new helper smack_add_opt() adds an already matched option to growing smack_mnt_options Reviewed-by: David Howells Signed-off-by: Al Viro --- security/smack/smack_lsm.c | 114 ++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index b607b1151e30..dba7bc53d86a 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -629,6 +629,54 @@ static int smack_sb_copy_data(char *orig, char *smackopts) return 0; } +static int smack_add_opt(int token, const char *s, void **mnt_opts) +{ + struct smack_mnt_opts *opts = *mnt_opts; + + if (!opts) { + opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL); + if (!opts) + return -ENOMEM; + *mnt_opts = opts; + } + + if (!s) + return -ENOMEM; + + switch (token) { + case Opt_fsdefault: + if (opts->fsdefault) + goto out_opt_err; + opts->fsdefault = s; + break; + case Opt_fsfloor: + if (opts->fsfloor) + goto out_opt_err; + opts->fsfloor = s; + break; + case Opt_fshat: + if (opts->fshat) + goto out_opt_err; + opts->fshat = s; + break; + case Opt_fsroot: + if (opts->fsroot) + goto out_opt_err; + opts->fsroot = s; + break; + case Opt_fstransmute: + if (opts->fstransmute) + goto out_opt_err; + opts->fstransmute = s; + break; + } + return 0; + +out_opt_err: + pr_warn("Smack: duplicate mount options\n"); + return -EINVAL; +} + /** * smack_parse_opts_str - parse Smack specific mount options * @options: mount options string @@ -641,7 +689,6 @@ static int smack_sb_copy_data(char *orig, char *smackopts) static int smack_parse_opts_str(char *options, void **mnt_opts) { - struct smack_mnt_opts *opts = *mnt_opts; char *p; int rc = -ENOMEM; int token; @@ -651,71 +698,24 @@ static int smack_parse_opts_str(char *options, while ((p = strsep(&options, ",")) != NULL) { substring_t args[MAX_OPT_ARGS]; + const char *arg; if (!*p) continue; token = match_token(p, smk_mount_tokens, args); - if (!opts) { - opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL); - if (!opts) - return -ENOMEM; - } - - switch (token) { - case Opt_fsdefault: - if (opts->fsdefault) - goto out_opt_err; - opts->fsdefault = match_strdup(&args[0]); - if (!opts->fsdefault) - goto out_err; - break; - case Opt_fsfloor: - if (opts->fsfloor) - goto out_opt_err; - opts->fsfloor = match_strdup(&args[0]); - if (!opts->fsfloor) - goto out_err; - break; - case Opt_fshat: - if (opts->fshat) - goto out_opt_err; - opts->fshat = match_strdup(&args[0]); - if (!opts->fshat) - goto out_err; - break; - case Opt_fsroot: - if (opts->fsroot) - goto out_opt_err; - opts->fsroot = match_strdup(&args[0]); - if (!opts->fsroot) - goto out_err; - break; - case Opt_fstransmute: - if (opts->fstransmute) - goto out_opt_err; - opts->fstransmute = match_strdup(&args[0]); - if (!opts->fstransmute) - goto out_err; - break; - default: - rc = -EINVAL; - pr_warn("Smack: unknown mount option\n"); - goto out_err; + arg = match_strdup(&args[0]); + rc = smack_add_opt(token, arg, mnt_opts); + if (unlikely(rc)) { + kfree(arg); + if (*mnt_opts) + smack_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; + return rc; } } - *mnt_opts = opts; return 0; - -out_opt_err: - rc = -EINVAL; - pr_warn("Smack: duplicate mount options\n"); - -out_err: - if (opts) - smack_free_mnt_opts(opts); - return rc; } static int smack_sb_eat_lsm_opts(char *options, void **mnt_opts) From c3300aaf95fb4e5be41e731fa6427d0d996d32ac Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 16 Dec 2018 01:52:24 -0500 Subject: [PATCH 25/27] smack: get rid of match_token() same issue as with selinux... [fix by Andrei Vagin folded in] Reviewed-by: David Howells Signed-off-by: Al Viro --- security/smack/smack_lsm.c | 56 ++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index dba7bc53d86a..d479def4d6a0 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -59,14 +59,31 @@ static LIST_HEAD(smk_ipv6_port_list); static struct kmem_cache *smack_inode_cache; int smack_enabled; -static const match_table_t smk_mount_tokens = { - {Opt_fsdefault, SMK_FSDEFAULT "%s"}, - {Opt_fsfloor, SMK_FSFLOOR "%s"}, - {Opt_fshat, SMK_FSHAT "%s"}, - {Opt_fsroot, SMK_FSROOT "%s"}, - {Opt_fstransmute, SMK_FSTRANS "%s"}, - {Opt_error, NULL}, +#define A(s) {"smack"#s, sizeof("smack"#s) - 1, Opt_##s} +static struct { + const char *name; + int len; + int opt; +} smk_mount_opts[] = { + A(fsdefault), A(fsfloor), A(fshat), A(fsroot), A(fstransmute) }; +#undef A + +static int match_opt_prefix(char *s, int l, char **arg) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(smk_mount_opts); i++) { + size_t len = smk_mount_opts[i].len; + if (len > l || memcmp(s, smk_mount_opts[i].name, len)) + continue; + if (len == l || s[len] != '=') + continue; + *arg = s + len + 1; + return smk_mount_opts[i].opt; + } + return Opt_error; +} #ifdef CONFIG_SECURITY_SMACK_BRINGUP static char *smk_bu_mess[] = { @@ -689,23 +706,23 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) static int smack_parse_opts_str(char *options, void **mnt_opts) { - char *p; - int rc = -ENOMEM; - int token; + char *from = options; if (!options) return 0; - while ((p = strsep(&options, ",")) != NULL) { - substring_t args[MAX_OPT_ARGS]; - const char *arg; + while (1) { + char *next = strchr(from, ','); + int token, len, rc; + char *arg = NULL; - if (!*p) - continue; + if (next) + len = next - from; + else + len = strlen(from); - token = match_token(p, smk_mount_tokens, args); - - arg = match_strdup(&args[0]); + token = match_opt_prefix(from, len, &arg); + arg = kmemdup_nul(arg, from + len - arg, GFP_KERNEL); rc = smack_add_opt(token, arg, mnt_opts); if (unlikely(rc)) { kfree(arg); @@ -714,6 +731,9 @@ static int smack_parse_opts_str(char *options, *mnt_opts = NULL; return rc; } + if (!from[len]) + break; + from += len + 1; } return 0; } From d2497e12e230c3f1be8ca6a0609a98c8c609fb80 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 16 Dec 2018 01:37:06 -0500 Subject: [PATCH 26/27] smack: rewrite smack_sb_eat_lsm_opts() make it use smack_add_opt() and avoid separate copies - gather non-LSM options by memmove() in place Reviewed-by: David Howells Signed-off-by: Al Viro --- security/smack/smack_lsm.c | 108 ++++++++----------------------------- 1 file changed, 23 insertions(+), 85 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index d479def4d6a0..24a00d93d8c3 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -599,53 +599,6 @@ static void smack_free_mnt_opts(void *mnt_opts) kfree(opts); } -/** - * smack_sb_copy_data - copy mount options data for processing - * @orig: where to start - * @smackopts: mount options string - * - * Returns 0 on success or -ENOMEM on error. - * - * Copy the Smack specific mount options out of the mount - * options list. - */ -static int smack_sb_copy_data(char *orig, char *smackopts) -{ - char *cp, *commap, *otheropts, *dp; - - otheropts = (char *)get_zeroed_page(GFP_KERNEL); - if (otheropts == NULL) - return -ENOMEM; - - for (cp = orig, commap = orig; commap != NULL; cp = commap + 1) { - if (strstr(cp, SMK_FSDEFAULT) == cp) - dp = smackopts; - else if (strstr(cp, SMK_FSFLOOR) == cp) - dp = smackopts; - else if (strstr(cp, SMK_FSHAT) == cp) - dp = smackopts; - else if (strstr(cp, SMK_FSROOT) == cp) - dp = smackopts; - else if (strstr(cp, SMK_FSTRANS) == cp) - dp = smackopts; - else - dp = otheropts; - - commap = strchr(cp, ','); - if (commap != NULL) - *commap = '\0'; - - if (*dp != '\0') - strcat(dp, ","); - strcat(dp, cp); - } - - strcpy(orig, otheropts); - free_page((unsigned long)otheropts); - - return 0; -} - static int smack_add_opt(int token, const char *s, void **mnt_opts) { struct smack_mnt_opts *opts = *mnt_opts; @@ -656,7 +609,6 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) return -ENOMEM; *mnt_opts = opts; } - if (!s) return -ENOMEM; @@ -694,22 +646,10 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) return -EINVAL; } -/** - * smack_parse_opts_str - parse Smack specific mount options - * @options: mount options string - * @opts: where to store converted mount opts - * - * Returns 0 on success or -ENOMEM on error. - * - * converts Smack specific mount options to generic security option format - */ -static int smack_parse_opts_str(char *options, - void **mnt_opts) +static int smack_sb_eat_lsm_opts(char *options, void **mnt_opts) { - char *from = options; - - if (!options) - return 0; + char *from = options, *to = options; + bool first = true; while (1) { char *next = strchr(from, ','); @@ -722,36 +662,34 @@ static int smack_parse_opts_str(char *options, len = strlen(from); token = match_opt_prefix(from, len, &arg); - arg = kmemdup_nul(arg, from + len - arg, GFP_KERNEL); - rc = smack_add_opt(token, arg, mnt_opts); - if (unlikely(rc)) { - kfree(arg); - if (*mnt_opts) - smack_free_mnt_opts(*mnt_opts); - *mnt_opts = NULL; - return rc; + if (token != Opt_error) { + arg = kmemdup_nul(arg, from + len - arg, GFP_KERNEL); + rc = smack_add_opt(token, arg, mnt_opts); + if (unlikely(rc)) { + kfree(arg); + if (*mnt_opts) + smack_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; + return rc; + } + } else { + if (!first) { // copy with preceding comma + from--; + len++; + } + if (to != from) + memmove(to, from, len); + to += len; + first = false; } if (!from[len]) break; from += len + 1; } + *to = '\0'; return 0; } -static int smack_sb_eat_lsm_opts(char *options, void **mnt_opts) -{ - char *s = (char *)get_zeroed_page(GFP_KERNEL); - int err; - - if (!s) - return -ENOMEM; - err = smack_sb_copy_data(options, s); - if (!err) - err = smack_parse_opts_str(s, mnt_opts); - free_page((unsigned long)s); - return err; -} - /** * smack_set_mnt_opts - set Smack specific mount options * @sb: the file system superblock From 718c43038f287e843c2f63d946977de90014cb11 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 20 Dec 2018 03:16:27 -0500 Subject: [PATCH 27/27] mount_fs: suppress MAC on MS_SUBMOUNT as well as MS_KERNMOUNT Reviewed-by: David Howells Signed-off-by: Al Viro --- fs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/super.c b/fs/super.c index a5511c4ba69b..48e25eba8465 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1277,7 +1277,7 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) if (error) goto out_sb; - if (!(flags & MS_KERNMOUNT)) { + if (!(flags & (MS_KERNMOUNT|MS_SUBMOUNT))) { error = security_sb_kern_mount(sb); if (error) goto out_sb;