From e80b18599a39a625bc8b2e39ba3004a62f78805a Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Fri, 12 Apr 2019 20:04:54 +0900 Subject: tomoyo: Add a kernel config option for fuzzing testing. syzbot is reporting kernel panic triggered by memory allocation fault injection before loading TOMOYO's policy [1]. To make the fuzzing tests useful, we need to assign a profile other than "disabled" (no-op) mode. Therefore, let's allow syzbot to load TOMOYO's built-in policy for "learning" mode using a kernel config option. This option must not be enabled for kernels built for production system, for this option also disables domain/program checks when modifying policy configuration via /sys/kernel/security/tomoyo/ interface. [1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95 Reported-by: syzbot Reported-by: syzbot Reported-by: syzbot Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/Kconfig | 10 ++++++++++ security/tomoyo/common.c | 13 ++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/security/tomoyo/Kconfig b/security/tomoyo/Kconfig index 404dce66952a..a00ab7eb6181 100644 --- a/security/tomoyo/Kconfig +++ b/security/tomoyo/Kconfig @@ -74,3 +74,13 @@ config SECURITY_TOMOYO_ACTIVATION_TRIGGER You can override this setting via TOMOYO_trigger= kernel command line option. For example, if you pass init=/bin/systemd option, you may want to also pass TOMOYO_trigger=/bin/systemd option. + +config SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING + bool "Use insecure built-in settings for fuzzing tests." + default n + depends on SECURITY_TOMOYO + select SECURITY_TOMOYO_OMIT_USERSPACE_LOADER + help + Enabling this option forces minimal built-in policy and disables + domain/program checks for run-time policy modifications. Please enable + this option only if this kernel is built for doing fuzzing tests. diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 57988d95d33d..dd3d5942e669 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -940,7 +940,7 @@ static bool tomoyo_manager(void) const char *exe; const struct task_struct *task = current; const struct tomoyo_path_info *domainname = tomoyo_domain()->domainname; - bool found = false; + bool found = IS_ENABLED(CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING); if (!tomoyo_policy_loaded) return true; @@ -2810,6 +2810,16 @@ void tomoyo_check_profile(void) */ void __init tomoyo_load_builtin_policy(void) { +#ifdef CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING + static char tomoyo_builtin_profile[] __initdata = + "PROFILE_VERSION=20150505\n" + "0-CONFIG={ mode=learning grant_log=no reject_log=yes }\n"; + static char tomoyo_builtin_exception_policy[] __initdata = + "aggregator proc:/self/exe /proc/self/exe\n"; + static char tomoyo_builtin_domain_policy[] __initdata = ""; + static char tomoyo_builtin_manager[] __initdata = ""; + static char tomoyo_builtin_stat[] __initdata = ""; +#else /* * This include file is manually created and contains built-in policy * named "tomoyo_builtin_profile", "tomoyo_builtin_exception_policy", @@ -2817,6 +2827,7 @@ void __init tomoyo_load_builtin_policy(void) * "tomoyo_builtin_stat" in the form of "static char [] __initdata". */ #include "builtin-policy.h" +#endif u8 i; const int idx = tomoyo_read_lock(); -- cgit From e6193f78bb689f3f424559bb45f4a091c8b314df Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Fri, 12 Apr 2019 19:59:36 +0900 Subject: tomoyo: Check address length before reading address family KMSAN will complain if valid address length passed to bind()/connect()/ sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes. Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/network.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/security/tomoyo/network.c b/security/tomoyo/network.c index 9094f4b3b367..f9ff121d7e1e 100644 --- a/security/tomoyo/network.c +++ b/security/tomoyo/network.c @@ -505,6 +505,8 @@ static int tomoyo_check_inet_address(const struct sockaddr *addr, { struct tomoyo_inet_addr_info *i = &address->inet; + if (addr_len < offsetofend(struct sockaddr, sa_family)) + return 0; switch (addr->sa_family) { case AF_INET6: if (addr_len < SIN6_LEN_RFC2133) @@ -594,6 +596,8 @@ static int tomoyo_check_unix_address(struct sockaddr *addr, { struct tomoyo_unix_addr_info *u = &address->unix0; + if (addr_len < offsetofend(struct sockaddr, sa_family)) + return 0; if (addr->sa_family != AF_UNIX) return 0; u->addr = ((struct sockaddr_un *) addr)->sun_path; -- cgit From 27df4b4a1b5fe2bef54ebc49d64bf5b39125f26a Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 27 Feb 2019 23:19:24 +0900 Subject: tomoyo: Change pathname calculation for read-only filesystems. Commit 5625f2e3266319fd ("TOMOYO: Change pathname for non-rename()able filesystems.") intended to be applied to filesystems where the content is not controllable from the userspace (e.g. proc, sysfs, securityfs), based on an assumption that such filesystems do not support rename() operation. But it turned out that read-only filesystems also do not support rename() operation despite the content is controllable from the userspace, and that commit is annoying TOMOYO users who want to use e.g. squashfs as the root filesystem due to use of local name which does not start with '/'. Therefore, based on an assumption that filesystems which require the device argument upon mount() request is an indication that the content is controllable from the userspace, do not use local name if a filesystem does not support rename() operation but requires the device argument upon mount() request. Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/realpath.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c index 85e6e31dd1e5..e7832448d721 100644 --- a/security/tomoyo/realpath.c +++ b/security/tomoyo/realpath.c @@ -295,7 +295,8 @@ char *tomoyo_realpath_from_path(const struct path *path) * or dentry without vfsmount. */ if (!path->mnt || - (!inode->i_op->rename)) + (!inode->i_op->rename && + !(sb->s_type->fs_flags & FS_REQUIRES_DEV))) pos = tomoyo_get_local_path(path->dentry, buf, buf_len - 1); /* Get absolute name for the rest. */ -- cgit From 4ad98ac46490d5f8441025930070eaf028cfd0f2 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Tue, 7 May 2019 20:34:22 +0900 Subject: tomoyo: Don't emit WARNING: string while fuzzing testing. Commit cff0e6c3ec3e6230 ("tomoyo: Add a kernel config option for fuzzing testing.") enabled the learning mode, but syzkaller is detecting any "WARNING:" string as a crash. Thus, disable TOMOYO's quota warning if built for fuzzing testing. Signed-off-by: Tetsuo Handa Cc: Dmitry Vyukov Signed-off-by: James Morris --- security/tomoyo/util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c index 0517cbdd7275..52752e1a84ed 100644 --- a/security/tomoyo/util.c +++ b/security/tomoyo/util.c @@ -1076,8 +1076,10 @@ bool tomoyo_domain_quota_is_ok(struct tomoyo_request_info *r) domain->flags[TOMOYO_DIF_QUOTA_WARNED] = true; /* r->granted = false; */ tomoyo_write_log(r, "%s", tomoyo_dif[TOMOYO_DIF_QUOTA_WARNED]); +#ifndef CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING pr_warn("WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.\n", domain->domainname->name); +#endif } return false; } -- cgit