From 78b7280cce23293f7570ad52c1ffe1485c6d9669 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 Mar 2011 17:57:23 +0000 Subject: KEYS: Improve /proc/keys Improve /proc/keys by: (1) Don't attempt to summarise the payload of a negated key. It won't have one. To this end, a helper function - key_is_instantiated() has been added that allows the caller to find out whether the key is positively instantiated (as opposed to being uninstantiated or negatively instantiated). (2) Do show keys that are negative, expired or revoked rather than hiding them. This requires an override flag (no_state_check) to be passed to search_my_process_keyrings() and keyring_search_aux() to suppress this check. Without this, keys that are possessed by the caller, but only grant permissions to the caller if possessed are skipped as the possession check fails. Keys that are visible due to user, group or other checks are visible with or without this patch. Signed-off-by: David Howells Signed-off-by: James Morris --- security/keys/internal.h | 4 +++- security/keys/keyring.c | 37 ++++++++++++++++++++++++------------- security/keys/proc.c | 2 +- security/keys/process_keys.c | 12 +++++++----- security/keys/request_key.c | 3 +-- security/keys/request_key_auth.c | 3 ++- security/keys/user_defined.c | 4 ++-- 7 files changed, 40 insertions(+), 25 deletions(-) (limited to 'security') diff --git a/security/keys/internal.h b/security/keys/internal.h index 07a025f8190..f375152a250 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -109,11 +109,13 @@ extern key_ref_t keyring_search_aux(key_ref_t keyring_ref, const struct cred *cred, struct key_type *type, const void *description, - key_match_func_t match); + key_match_func_t match, + bool no_state_check); extern key_ref_t search_my_process_keyrings(struct key_type *type, const void *description, key_match_func_t match, + bool no_state_check, const struct cred *cred); extern key_ref_t search_process_keyrings(struct key_type *type, const void *description, diff --git a/security/keys/keyring.c b/security/keys/keyring.c index cdd2f3f88c8..a06ffab3856 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -176,13 +176,15 @@ static void keyring_describe(const struct key *keyring, struct seq_file *m) else seq_puts(m, "[anon]"); - rcu_read_lock(); - klist = rcu_dereference(keyring->payload.subscriptions); - if (klist) - seq_printf(m, ": %u/%u", klist->nkeys, klist->maxkeys); - else - seq_puts(m, ": empty"); - rcu_read_unlock(); + if (key_is_instantiated(keyring)) { + rcu_read_lock(); + klist = rcu_dereference(keyring->payload.subscriptions); + if (klist) + seq_printf(m, ": %u/%u", klist->nkeys, klist->maxkeys); + else + seq_puts(m, ": empty"); + rcu_read_unlock(); + } } /* @@ -271,6 +273,7 @@ struct key *keyring_alloc(const char *description, uid_t uid, gid_t gid, * @type: The type of key to search for. * @description: Parameter for @match. * @match: Function to rule on whether or not a key is the one required. + * @no_state_check: Don't check if a matching key is bad * * Search the supplied keyring tree for a key that matches the criteria given. * The root keyring and any linked keyrings must grant Search permission to the @@ -303,7 +306,8 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref, const struct cred *cred, struct key_type *type, const void *description, - key_match_func_t match) + key_match_func_t match, + bool no_state_check) { struct { struct keyring_list *keylist; @@ -345,6 +349,8 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref, kflags = keyring->flags; if (keyring->type == type && match(keyring, description)) { key = keyring; + if (no_state_check) + goto found; /* check it isn't negative and hasn't expired or been * revoked */ @@ -384,11 +390,13 @@ descend: continue; /* skip revoked keys and expired keys */ - if (kflags & (1 << KEY_FLAG_REVOKED)) - continue; + if (!no_state_check) { + if (kflags & (1 << KEY_FLAG_REVOKED)) + continue; - if (key->expiry && now.tv_sec >= key->expiry) - continue; + if (key->expiry && now.tv_sec >= key->expiry) + continue; + } /* keys that don't match */ if (!match(key, description)) @@ -399,6 +407,9 @@ descend: cred, KEY_SEARCH) < 0) continue; + if (no_state_check) + goto found; + /* we set a different error code if we pass a negative key */ if (kflags & (1 << KEY_FLAG_NEGATIVE)) { err = key->type_data.reject_error; @@ -478,7 +489,7 @@ key_ref_t keyring_search(key_ref_t keyring, return ERR_PTR(-ENOKEY); return keyring_search_aux(keyring, current->cred, - type, description, type->match); + type, description, type->match, false); } EXPORT_SYMBOL(keyring_search); diff --git a/security/keys/proc.c b/security/keys/proc.c index 525cf8a29cd..49bbc97943a 100644 --- a/security/keys/proc.c +++ b/security/keys/proc.c @@ -199,7 +199,7 @@ static int proc_keys_show(struct seq_file *m, void *v) if (key->perm & KEY_POS_VIEW) { skey_ref = search_my_process_keyrings(key->type, key, lookup_user_key_possessed, - cred); + true, cred); if (!IS_ERR(skey_ref)) { key_ref_put(skey_ref); key_ref = make_key_ref(key, 1); diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 930634e4514..6c0480db888 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -331,6 +331,7 @@ void key_fsgid_changed(struct task_struct *tsk) key_ref_t search_my_process_keyrings(struct key_type *type, const void *description, key_match_func_t match, + bool no_state_check, const struct cred *cred) { key_ref_t key_ref, ret, err; @@ -350,7 +351,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, if (cred->thread_keyring) { key_ref = keyring_search_aux( make_key_ref(cred->thread_keyring, 1), - cred, type, description, match); + cred, type, description, match, no_state_check); if (!IS_ERR(key_ref)) goto found; @@ -371,7 +372,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, if (cred->tgcred->process_keyring) { key_ref = keyring_search_aux( make_key_ref(cred->tgcred->process_keyring, 1), - cred, type, description, match); + cred, type, description, match, no_state_check); if (!IS_ERR(key_ref)) goto found; @@ -395,7 +396,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, make_key_ref(rcu_dereference( cred->tgcred->session_keyring), 1), - cred, type, description, match); + cred, type, description, match, no_state_check); rcu_read_unlock(); if (!IS_ERR(key_ref)) @@ -417,7 +418,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, else if (cred->user->session_keyring) { key_ref = keyring_search_aux( make_key_ref(cred->user->session_keyring, 1), - cred, type, description, match); + cred, type, description, match, no_state_check); if (!IS_ERR(key_ref)) goto found; @@ -459,7 +460,8 @@ key_ref_t search_process_keyrings(struct key_type *type, might_sleep(); - key_ref = search_my_process_keyrings(type, description, match, cred); + key_ref = search_my_process_keyrings(type, description, match, + false, cred); if (!IS_ERR(key_ref)) goto found; err = key_ref; diff --git a/security/keys/request_key.c b/security/keys/request_key.c index df3c0417ee4..b18a7174590 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -530,8 +530,7 @@ struct key *request_key_and_link(struct key_type *type, dest_keyring, flags); /* search all the process keyrings for a key */ - key_ref = search_process_keyrings(type, description, type->match, - cred); + key_ref = search_process_keyrings(type, description, type->match, cred); if (!IS_ERR(key_ref)) { key = key_ref_to_ptr(key_ref); diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c index 68164031a74..f6337c9082e 100644 --- a/security/keys/request_key_auth.c +++ b/security/keys/request_key_auth.c @@ -59,7 +59,8 @@ static void request_key_auth_describe(const struct key *key, seq_puts(m, "key:"); seq_puts(m, key->description); - seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len); + if (key_is_instantiated(key)) + seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len); } /* diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c index c6ca8662a46..63bb1aaffc0 100644 --- a/security/keys/user_defined.c +++ b/security/keys/user_defined.c @@ -169,8 +169,8 @@ EXPORT_SYMBOL_GPL(user_destroy); void user_describe(const struct key *key, struct seq_file *m) { seq_puts(m, key->description); - - seq_printf(m, ": %u", key->datalen); + if (key_is_instantiated(key)) + seq_printf(m, ": %u", key->datalen); } EXPORT_SYMBOL_GPL(user_describe); -- cgit v1.2.3 From 4aab1e896a0a9d57420ff2867caa5a369123d8cb Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 Mar 2011 17:57:33 +0000 Subject: KEYS: Make request_key() and co. return an error for a negative key Make request_key() and co. return an error for a negative or rejected key. If the key was simply negated, then return ENOKEY, otherwise return the error with which it was rejected. Without this patch, the following command returns a key number (with the latest keyutils): [root@andromeda ~]# keyctl request2 user debug:foo rejected @s 586569904 Trying to print the key merely gets you a permission denied error: [root@andromeda ~]# keyctl print 586569904 keyctl_read_alloc: Permission denied Doing another request_key() call does get you the error, as long as it hasn't expired yet: [root@andromeda ~]# keyctl request user debug:foo request_key: Key was rejected by service Signed-off-by: David Howells Signed-off-by: James Morris --- security/keys/keyctl.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'security') diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 427fddcaeb1..eca51918c95 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -206,8 +206,14 @@ SYSCALL_DEFINE4(request_key, const char __user *, _type, goto error5; } + /* wait for the key to finish being constructed */ + ret = wait_for_key_construction(key, 1); + if (ret < 0) + goto error6; + ret = key->serial; +error6: key_put(key); error5: key_type_put(ktype); -- cgit v1.2.3 From 5806896019ceaa0a1e808182afb4bba33c948ad6 Mon Sep 17 00:00:00 2001 From: Paul Gortmaker Date: Mon, 14 Mar 2011 19:32:21 -0400 Subject: security: select correct default LSM_MMAP_MIN_ADDR on ARM. The default for this is universally set to 64k, but the help says: For most ia64, ppc64 and x86 users with lots of address space a value of 65536 is reasonable and should cause no problems. On arm and other archs it should not be higher than 32768. The text is right, in that we are seeing selinux-enabled ARM targets that fail to launch /sbin/init because selinux blocks a memory map. So select the right value if we know we are building ARM. Signed-off-by: Paul Gortmaker Signed-off-by: James Morris --- security/Kconfig | 1 + 1 file changed, 1 insertion(+) (limited to 'security') diff --git a/security/Kconfig b/security/Kconfig index 95accd442d5..e0f08b52e4a 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -167,6 +167,7 @@ config INTEL_TXT config LSM_MMAP_MIN_ADDR int "Low address space for LSM to protect from user allocation" depends on SECURITY && SECURITY_SELINUX + default 32768 if ARM default 65536 help This is the portion of low virtual memory which should be protected -- cgit v1.2.3 From cfc64fd91fabed099a4c3df58559f4b7efe9bcce Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Thu, 31 Mar 2011 00:27:32 +0900 Subject: tomoyo: fix memory leak in tomoyo_commit_ok() When memory used for policy exceeds the quota, tomoyo_memory_ok() return false. In this case, tomoyo_commit_ok() must call kfree() before returning NULL. This bug exists since 2.6.35. Signed-off-by: Xiaochen Wang Acked-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/memory.c | 1 + 1 file changed, 1 insertion(+) (limited to 'security') diff --git a/security/tomoyo/memory.c b/security/tomoyo/memory.c index 297612669c7..42a7b1ba8cb 100644 --- a/security/tomoyo/memory.c +++ b/security/tomoyo/memory.c @@ -75,6 +75,7 @@ void *tomoyo_commit_ok(void *data, const unsigned int size) memset(data, 0, size); return ptr; } + kfree(ptr); return NULL; } -- cgit v1.2.3 From 4bf2ea77dba76a22f49db3c10773896aaeeb8f66 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 1 Apr 2011 17:08:28 -0400 Subject: capabilities: do not special case exec of init When the global init task is exec'd we have special case logic to make sure the pE is not reduced. There is no reason for this. If init wants to drop it's pE is should be allowed to do so. Remove this special logic. Signed-off-by: Eric Paris Acked-by: Serge Hallyn Acked-by: David Howells Acked-by: Andrew G. Morgan Signed-off-by: James Morris --- security/commoncap.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index f20e984ccfb..a93b3b73307 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -529,15 +529,10 @@ skip: new->suid = new->fsuid = new->euid; new->sgid = new->fsgid = new->egid; - /* For init, we want to retain the capabilities set in the initial - * task. Thus we skip the usual capability rules - */ - if (!is_global_init(current)) { - if (effective) - new->cap_effective = new->cap_permitted; - else - cap_clear(new->cap_effective); - } + if (effective) + new->cap_effective = new->cap_permitted; + else + cap_clear(new->cap_effective); bprm->cap_effective = effective; /* -- cgit v1.2.3 From 2a086e5d3a23570735f75b784d29b93068070833 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sun, 3 Apr 2011 00:09:26 +0900 Subject: TOMOYO: Fix race on updating profile's comment line. In tomoyo_write_profile() since 2.6.34, a lock was by error missing when replacing profile's comment line. If multiple threads attempted echo '0-COMMENT=comment' > /sys/kernel/security/tomoyo/profile in parallel, garbage collector will fail to kfree() the old value. Protect the replacement using a lock. Also, keep the old value rather than replace with empty string when out of memory error has occurred. Signed-off-by: Xiaochen Wang Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/common.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 7556315c197..2b7b1a12360 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -459,8 +459,16 @@ static int tomoyo_write_profile(struct tomoyo_io_buffer *head) if (profile == &tomoyo_default_profile) return -EINVAL; if (!strcmp(data, "COMMENT")) { - const struct tomoyo_path_info *old_comment = profile->comment; - profile->comment = tomoyo_get_name(cp); + static DEFINE_SPINLOCK(lock); + const struct tomoyo_path_info *new_comment + = tomoyo_get_name(cp); + const struct tomoyo_path_info *old_comment; + if (!new_comment) + return -ENOMEM; + spin_lock(&lock); + old_comment = profile->comment; + profile->comment = new_comment; + spin_unlock(&lock); tomoyo_put_name(old_comment); return 0; } -- cgit v1.2.3 From e4f5f26d8336318a5aa0858223c81cf29fcf5f68 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sun, 3 Apr 2011 00:11:50 +0900 Subject: TOMOYO: Don't add / for allow_unmount permission check. "mount --bind /path/to/file1 /path/to/file2" is legal. Therefore, "umount /path/to/file2" is also legal. Do not automatically append trailing '/' if pathname to be unmounted does not end with '/'. Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/file.c | 1 - 1 file changed, 1 deletion(-) (limited to 'security') diff --git a/security/tomoyo/file.c b/security/tomoyo/file.c index cb09f1fce91..d64e8ecb6fb 100644 --- a/security/tomoyo/file.c +++ b/security/tomoyo/file.c @@ -1011,7 +1011,6 @@ int tomoyo_path_perm(const u8 operation, struct path *path) break; case TOMOYO_TYPE_RMDIR: case TOMOYO_TYPE_CHROOT: - case TOMOYO_TYPE_UMOUNT: tomoyo_add_slash(&buf); break; } -- cgit v1.2.3 From c0fa797ae6cd02ff87c0bfe0d509368a3b45640e Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sun, 3 Apr 2011 00:12:54 +0900 Subject: TOMOYO: Fix infinite loop bug when reading /sys/kernel/security/tomoyo/audit In tomoyo_flush(), head->r.w[0] holds pointer to string data to be printed. But head->r.w[0] was updated only when the string data was partially printed (because head->r.w[0] will be updated by head->r.w[1] later if completely printed). However, regarding /sys/kernel/security/tomoyo/query , an additional '\0' is printed after the string data was completely printed. But if free space for read buffer became 0 before printing the additional '\0', tomoyo_flush() was returning without updating head->r.w[0]. As a result, tomoyo_flush() forever reprints already printed string data. Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/common.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 2b7b1a12360..a0d09e56874 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -108,10 +108,9 @@ static bool tomoyo_flush(struct tomoyo_io_buffer *head) head->read_user_buf += len; w += len; } - if (*w) { - head->r.w[0] = w; + head->r.w[0] = w; + if (*w) return false; - } /* Add '\0' for query. */ if (head->poll) { if (!head->read_user_buf_avail || -- cgit v1.2.3 From db5ca356d8af8e43832c185ceec90850ff2ebb45 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 20 Apr 2011 06:49:15 +0900 Subject: TOMOYO: Fix refcount leak in tomoyo_mount_acl(). In tomoyo_mount_acl() since 2.6.36, reference to device file (e.g. /dev/sda1) was leaking. Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/mount.c | 1 + 1 file changed, 1 insertion(+) (limited to 'security') diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c index 82bf8c2390b..162a864dba2 100644 --- a/security/tomoyo/mount.c +++ b/security/tomoyo/mount.c @@ -143,6 +143,7 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r, char *dev_name, goto out; } requested_dev_name = tomoyo_realpath_from_path(&path); + path_put(&path); if (!requested_dev_name) { error = -ENOENT; goto out; -- cgit v1.2.3 From e77dc3460fa59be5759e9327ad882868eee9d61b Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Thu, 12 May 2011 06:40:51 +0900 Subject: TOMOYO: Fix wrong domainname validation. In tomoyo_correct_domain() since 2.6.36, TOMOYO was by error validating "" + "/foo/\" + "/bar" when " /foo/\* /bar" was given. As a result, legal domainnames like " /foo/\* /bar" are rejected. Reported-by: Hayama Yossihiro Signed-off-by: Tetsuo Handa Signed-off-by: James Morris --- security/tomoyo/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c index 9bfc1ee8222..6d5393204d9 100644 --- a/security/tomoyo/util.c +++ b/security/tomoyo/util.c @@ -390,7 +390,7 @@ bool tomoyo_correct_domain(const unsigned char *domainname) if (!cp) break; if (*domainname != '/' || - !tomoyo_correct_word2(domainname, cp - domainname - 1)) + !tomoyo_correct_word2(domainname, cp - domainname)) goto out; domainname = cp + 1; } -- cgit v1.2.3