From 5384d92e4e02661d2b08e9e20c7b2cd3b0b82db6 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 9 Sep 2020 20:05:58 +0900 Subject: tomoyo: Loosen pathname/domainname validation. Since commit e2dc9bf3f5275ca3 ("umd: Transform fork_usermode_blob into fork_usermode_driver") started calling execve() on a program written in a local mount which is not connected to mount tree, tomoyo_realpath_from_path() started returning a pathname in "$fsname:/$pathname" format which violates TOMOYO's domainname rule that it must start with "<$namespace>" followed by zero or more repetitions of pathnames which start with '/'. Since $fsname must not contain '.' since commit 79c0b2df79eb56fc ("add filesystem subtype support"), tomoyo_correct_path() can recognize a token which appears '/' before '.' appears (e.g. proc:/self/exe ) as a pathname while rejecting a token which appears '.' before '/' appears (e.g. exec.realpath="/bin/bash" ) as a condition parameter. Therefore, accept domainnames which contain pathnames which do not start with '/' but contain '/' before '.' (e.g. tmpfs:/bpfilter_umh ). Signed-off-by: Tetsuo Handa --- security/tomoyo/util.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c index eba0b3395851..a40abb0b91ee 100644 --- a/security/tomoyo/util.c +++ b/security/tomoyo/util.c @@ -143,6 +143,8 @@ char *tomoyo_read_token(struct tomoyo_acl_param *param) return pos; } +static bool tomoyo_correct_path2(const char *filename, const size_t len); + /** * tomoyo_get_domainname - Read a domainname from a line. * @@ -157,10 +159,10 @@ const struct tomoyo_path_info *tomoyo_get_domainname char *pos = start; while (*pos) { - if (*pos++ != ' ' || *pos++ == '/') + if (*pos++ != ' ' || + tomoyo_correct_path2(pos, strchrnul(pos, ' ') - pos)) continue; - pos -= 2; - *pos++ = '\0'; + *(pos - 1) = '\0'; break; } param->data = pos; @@ -513,6 +515,22 @@ bool tomoyo_correct_word(const char *string) return tomoyo_correct_word2(string, strlen(string)); } +/** + * tomoyo_correct_path2 - Check whether the given pathname follows the naming rules. + * + * @filename: The pathname to check. + * @len: Length of @filename. + * + * Returns true if @filename follows the naming rules, false otherwise. + */ +static bool tomoyo_correct_path2(const char *filename, const size_t len) +{ + const char *cp1 = memchr(filename, '/', len); + const char *cp2 = memchr(filename, '.', len); + + return cp1 && (!cp2 || (cp1 < cp2)) && tomoyo_correct_word2(filename, len); +} + /** * tomoyo_correct_path - Validate a pathname. * @@ -523,7 +541,7 @@ bool tomoyo_correct_word(const char *string) */ bool tomoyo_correct_path(const char *filename) { - return *filename == '/' && tomoyo_correct_word(filename); + return tomoyo_correct_path2(filename, strlen(filename)); } /** @@ -545,8 +563,7 @@ bool tomoyo_correct_domain(const unsigned char *domainname) if (!cp) break; - if (*domainname != '/' || - !tomoyo_correct_word2(domainname, cp - domainname)) + if (!tomoyo_correct_path2(domainname, cp - domainname)) return false; domainname = cp + 1; } -- cgit v1.2.3 From d9594e0409651a237903a13c9718df889f43d43b Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 28 Oct 2020 23:14:04 +0900 Subject: tomoyo: fix clang pointer arithmetic warning clang warns about additions on NULL pointers being undefined in C: security/tomoyo/securityfs_if.c:226:59: warning: arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension [-Wnull-pointer-arithmetic] securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, Change the code to instead use a cast through uintptr_t to avoid the warning. Signed-off-by: Arnd Bergmann Signed-off-by: Tetsuo Handa --- security/tomoyo/securityfs_if.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/tomoyo/securityfs_if.c b/security/tomoyo/securityfs_if.c index 546281c5b233..065f4941c4d8 100644 --- a/security/tomoyo/securityfs_if.c +++ b/security/tomoyo/securityfs_if.c @@ -131,8 +131,8 @@ static const struct file_operations tomoyo_self_operations = { */ static int tomoyo_open(struct inode *inode, struct file *file) { - const int key = ((u8 *) file_inode(file)->i_private) - - ((u8 *) NULL); + const u8 key = (uintptr_t) file_inode(file)->i_private; + return tomoyo_open_control(key, file); } @@ -223,7 +223,7 @@ static const struct file_operations tomoyo_operations = { static void __init tomoyo_create_entry(const char *name, const umode_t mode, struct dentry *parent, const u8 key) { - securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, + securityfs_create_file(name, mode, parent, (void *) (uintptr_t) key, &tomoyo_operations); } -- cgit v1.2.3 From e991a40b3d0000a2f48729aea4ce03acf679b5ee Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Tue, 3 Nov 2020 13:17:40 +0900 Subject: tomoyo: Limit wildcard recursion depth. Since wildcards that need recursion consume kernel stack memory (or might cause CPU stall warning problem), we cannot allow infinite recursion. Since TOMOYO 1.8 survived with 20 recursions limit for 5 years, nobody would complain if applying this limit to TOMOYO 2.6. Signed-off-by: Tetsuo Handa --- security/tomoyo/util.c | 55 +++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 25 deletions(-) (limited to 'security') diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c index a40abb0b91ee..176b803ebcfc 100644 --- a/security/tomoyo/util.c +++ b/security/tomoyo/util.c @@ -434,59 +434,64 @@ void tomoyo_normalize_line(unsigned char *buffer) */ static bool tomoyo_correct_word2(const char *string, size_t len) { + u8 recursion = 20; const char *const start = string; bool in_repetition = false; - unsigned char c; - unsigned char d; - unsigned char e; if (!len) goto out; while (len--) { - c = *string++; + unsigned char c = *string++; + if (c == '\\') { if (!len--) goto out; c = *string++; + if (c >= '0' && c <= '3') { + unsigned char d; + unsigned char e; + + if (!len-- || !len--) + goto out; + d = *string++; + e = *string++; + if (d < '0' || d > '7' || e < '0' || e > '7') + goto out; + c = tomoyo_make_byte(c, d, e); + if (c <= ' ' || c >= 127) + continue; + goto out; + } switch (c) { case '\\': /* "\\" */ - continue; - case '$': /* "\$" */ case '+': /* "\+" */ case '?': /* "\?" */ + case 'x': /* "\x" */ + case 'a': /* "\a" */ + case '-': /* "\-" */ + continue; + } + if (!recursion--) + goto out; + switch (c) { case '*': /* "\*" */ case '@': /* "\@" */ - case 'x': /* "\x" */ + case '$': /* "\$" */ case 'X': /* "\X" */ - case 'a': /* "\a" */ case 'A': /* "\A" */ - case '-': /* "\-" */ continue; case '{': /* "/\{" */ if (string - 3 < start || *(string - 3) != '/') - break; + goto out; in_repetition = true; continue; case '}': /* "\}/" */ if (*string != '/') - break; + goto out; if (!in_repetition) - break; + goto out; in_repetition = false; continue; - case '0': /* "\ooo" */ - case '1': - case '2': - case '3': - if (!len-- || !len--) - break; - d = *string++; - e = *string++; - if (d < '0' || d > '7' || e < '0' || e > '7') - break; - c = tomoyo_make_byte(c, d, e); - if (c <= ' ' || c >= 127) - continue; } goto out; } else if (in_repetition && c == '/') { -- cgit v1.2.3 From 1b6b924efeb9e46f0ca2ebe5b9bb6b276defe52d Mon Sep 17 00:00:00 2001 From: Zheng Zengkai Date: Thu, 26 Nov 2020 22:38:15 +0800 Subject: tomoyo: Fix null pointer check Since tomoyo_memory_ok() will check for null pointer returned by kzalloc() in tomoyo_assign_profile(), tomoyo_assign_namespace(), tomoyo_get_name() and tomoyo_commit_ok(), then emit OOM warnings if needed. And this is the expected behavior as informed by Tetsuo Handa. Let's add __GFP_NOWARN to kzalloc() in those related functions. Besides, to achieve this goal, remove the null check for entry right after kzalloc() in tomoyo_assign_namespace(). Reported-by: Hulk Robot Suggested-by: Tetsuo Handa Signed-off-by: Zheng Zengkai Signed-off-by: Tetsuo Handa --- security/tomoyo/common.c | 2 +- security/tomoyo/domain.c | 4 +--- security/tomoyo/memory.c | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 4bee32bfe16d..bc54d3c8c70a 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -498,7 +498,7 @@ static struct tomoyo_profile *tomoyo_assign_profile ptr = ns->profile_ptr[profile]; if (ptr) return ptr; - entry = kzalloc(sizeof(*entry), GFP_NOFS); + entry = kzalloc(sizeof(*entry), GFP_NOFS | __GFP_NOWARN); if (mutex_lock_interruptible(&tomoyo_policy_lock)) goto out; ptr = ns->profile_ptr[profile]; diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c index dc4ecc0b2038..c6e5cc5cc7cd 100644 --- a/security/tomoyo/domain.c +++ b/security/tomoyo/domain.c @@ -473,9 +473,7 @@ struct tomoyo_policy_namespace *tomoyo_assign_namespace(const char *domainname) return ptr; if (len >= TOMOYO_EXEC_TMPSIZE - 10 || !tomoyo_domain_def(domainname)) return NULL; - entry = kzalloc(sizeof(*entry) + len + 1, GFP_NOFS); - if (!entry) - return NULL; + entry = kzalloc(sizeof(*entry) + len + 1, GFP_NOFS | __GFP_NOWARN); if (mutex_lock_interruptible(&tomoyo_policy_lock)) goto out; ptr = tomoyo_find_namespace(domainname, len); diff --git a/security/tomoyo/memory.c b/security/tomoyo/memory.c index 2e7fcfa923c9..1b570bde7a3b 100644 --- a/security/tomoyo/memory.c +++ b/security/tomoyo/memory.c @@ -73,7 +73,7 @@ bool tomoyo_memory_ok(void *ptr) */ void *tomoyo_commit_ok(void *data, const unsigned int size) { - void *ptr = kzalloc(size, GFP_NOFS); + void *ptr = kzalloc(size, GFP_NOFS | __GFP_NOWARN); if (tomoyo_memory_ok(ptr)) { memmove(ptr, data, size); @@ -170,7 +170,7 @@ const struct tomoyo_path_info *tomoyo_get_name(const char *name) atomic_inc(&ptr->head.users); goto out; } - ptr = kzalloc(sizeof(*ptr) + len, GFP_NOFS); + ptr = kzalloc(sizeof(*ptr) + len, GFP_NOFS | __GFP_NOWARN); if (tomoyo_memory_ok(ptr)) { ptr->entry.name = ((char *) ptr) + sizeof(*ptr); memmove((char *) ptr->entry.name, name, len); -- cgit v1.2.3 From 15269fb193108ba8a3774507d0bbd70949ab610d Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sun, 6 Dec 2020 13:44:57 +0900 Subject: tomoyo: Fix typo in comments. Spotted by developers and codespell program. Co-developed-by: Xiaoming Ni Signed-off-by: Xiaoming Ni Co-developed-by: Souptick Joarder Signed-off-by: Souptick Joarder Signed-off-by: Tetsuo Handa --- security/tomoyo/audit.c | 2 +- security/tomoyo/common.c | 6 +++--- security/tomoyo/condition.c | 2 +- security/tomoyo/domain.c | 2 +- security/tomoyo/gc.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c index 3c96e8402e94..b51bad121c11 100644 --- a/security/tomoyo/audit.c +++ b/security/tomoyo/audit.c @@ -311,7 +311,7 @@ static LIST_HEAD(tomoyo_log); /* Lock for "struct list_head tomoyo_log". */ static DEFINE_SPINLOCK(tomoyo_log_lock); -/* Length of "stuct list_head tomoyo_log". */ +/* Length of "struct list_head tomoyo_log". */ static unsigned int tomoyo_log_count; /** diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index bc54d3c8c70a..5c64927bf2b3 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -635,7 +635,7 @@ static int tomoyo_set_mode(char *name, const char *value, if (strstr(value, tomoyo_mode[mode])) /* * Update lower 3 bits in order to distinguish - * 'config' from 'TOMOYO_CONFIG_USE_DEAFULT'. + * 'config' from 'TOMOYO_CONFIG_USE_DEFAULT'. */ config = (config & ~7) | mode; if (config != TOMOYO_CONFIG_USE_DEFAULT) { @@ -2574,7 +2574,7 @@ static inline bool tomoyo_has_more_namespace(struct tomoyo_io_buffer *head) * tomoyo_read_control - read() for /sys/kernel/security/tomoyo/ interface. * * @head: Pointer to "struct tomoyo_io_buffer". - * @buffer: Poiner to buffer to write to. + * @buffer: Pointer to buffer to write to. * @buffer_len: Size of @buffer. * * Returns bytes read on success, negative value otherwise. @@ -2608,7 +2608,7 @@ ssize_t tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer, /** * tomoyo_parse_policy - Parse a policy line. * - * @head: Poiter to "struct tomoyo_io_buffer". + * @head: Pointer to "struct tomoyo_io_buffer". * @line: Line to parse. * * Returns 0 on success, negative value otherwise. diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c index 8f6d57c15df6..f8bcc083bb0d 100644 --- a/security/tomoyo/condition.c +++ b/security/tomoyo/condition.c @@ -98,7 +98,7 @@ static bool tomoyo_envp(const char *env_name, const char *env_value, * @argc: Length of @argc. * @argv: Pointer to "struct tomoyo_argv". * @envc: Length of @envp. - * @envp: Poiner to "struct tomoyo_envp". + * @envp: Pointer to "struct tomoyo_envp". * * Returns true on success, false otherwise. */ diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c index c6e5cc5cc7cd..98d985895ec8 100644 --- a/security/tomoyo/domain.c +++ b/security/tomoyo/domain.c @@ -889,7 +889,7 @@ force_jump_domain: * * @bprm: Pointer to "struct linux_binprm". * @pos: Location to dump. - * @dump: Poiner to "struct tomoyo_page_dump". + * @dump: Pointer to "struct tomoyo_page_dump". * * Returns true on success, false otherwise. */ diff --git a/security/tomoyo/gc.c b/security/tomoyo/gc.c index 9537832fca18..026e29ea3796 100644 --- a/security/tomoyo/gc.c +++ b/security/tomoyo/gc.c @@ -463,7 +463,7 @@ static void tomoyo_try_to_gc(const enum tomoyo_policy_id type, return; reinject: /* - * We can safely reinject this element here bacause + * We can safely reinject this element here because * (1) Appending list elements and removing list elements are protected * by tomoyo_policy_lock mutex. * (2) Only this function removes list elements and this function is -- cgit v1.2.3