From ed868a56988464cd31de0302426a5e94d3127f10 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Sat, 12 Sep 2009 22:54:10 -0400 Subject: [PATCH 1/7] Creds: creds->security can be NULL is selinux is disabled __validate_process_creds should check if selinux is actually enabled before running tests on the selinux portion of the credentials struct. Signed-off-by: Eric Paris Signed-off-by: James Morris --- include/linux/cred.h | 13 ++++++++----- include/linux/selinux.h | 9 +++++++++ security/selinux/exports.c | 6 ++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/include/linux/cred.h b/include/linux/cred.h index 24520a539c6f..fb371601a3b4 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -15,6 +15,7 @@ #include #include #include +#include #include struct user_struct; @@ -182,11 +183,13 @@ static inline bool creds_are_invalid(const struct cred *cred) if (atomic_read(&cred->usage) < atomic_read(&cred->subscribers)) return true; #ifdef CONFIG_SECURITY_SELINUX - if ((unsigned long) cred->security < PAGE_SIZE) - return true; - if ((*(u32*)cred->security & 0xffffff00) == - (POISON_FREE << 24 | POISON_FREE << 16 | POISON_FREE << 8)) - return true; + if (selinux_is_enabled()) { + if ((unsigned long) cred->security < PAGE_SIZE) + return true; + if ((*(u32 *)cred->security & 0xffffff00) == + (POISON_FREE << 24 | POISON_FREE << 16 | POISON_FREE << 8)) + return true; + } #endif return false; } diff --git a/include/linux/selinux.h b/include/linux/selinux.h index 20f965d4b041..223d06a6feb1 100644 --- a/include/linux/selinux.h +++ b/include/linux/selinux.h @@ -61,6 +61,11 @@ void selinux_secmark_refcount_inc(void); * existing SECMARK targets has been removed/flushed. */ void selinux_secmark_refcount_dec(void); + +/** + * selinux_is_enabled - is SELinux enabled? + */ +bool selinux_is_enabled(void); #else static inline int selinux_string_to_sid(const char *str, u32 *sid) @@ -84,6 +89,10 @@ static inline void selinux_secmark_refcount_dec(void) return; } +static bool selinux_is_enabled(void) +{ + return false; +} #endif /* CONFIG_SECURITY_SELINUX */ #endif /* _LINUX_SELINUX_H */ diff --git a/security/selinux/exports.c b/security/selinux/exports.c index c73aeaa008e8..c0a454aee1e0 100644 --- a/security/selinux/exports.c +++ b/security/selinux/exports.c @@ -63,3 +63,9 @@ void selinux_secmark_refcount_dec(void) atomic_dec(&selinux_secmark_refcount); } EXPORT_SYMBOL_GPL(selinux_secmark_refcount_dec); + +bool selinux_is_enabled(void) +{ + return selinux_enabled; +} +EXPORT_SYMBOL_GPL(selinux_is_enabled); From 008574b11171a1ee9583a00188e27ff9e0432061 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Sat, 12 Sep 2009 22:54:17 -0400 Subject: [PATCH 2/7] SELinux: seperate avc_cache flushing Move the avc_cache flushing into it's own function so it can be reused when disabling SELinux. Signed-off-by: Eric Paris Signed-off-by: James Morris --- security/selinux/avc.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index e3d19014259b..f60124623645 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -709,18 +709,16 @@ out: } /** - * avc_ss_reset - Flush the cache and revalidate migrated permissions. - * @seqno: policy sequence number + * avc_flush - Flush the cache */ -int avc_ss_reset(u32 seqno) +static void avc_flush(void) { - struct avc_callback_node *c; - int i, rc = 0, tmprc; - unsigned long flag; - struct avc_node *node; struct hlist_head *head; struct hlist_node *next; + struct avc_node *node; spinlock_t *lock; + unsigned long flag; + int i; for (i = 0; i < AVC_CACHE_SLOTS; i++) { head = &avc_cache.slots[i]; @@ -737,6 +735,18 @@ int avc_ss_reset(u32 seqno) rcu_read_unlock(); spin_unlock_irqrestore(lock, flag); } +} + +/** + * avc_ss_reset - Flush the cache and revalidate migrated permissions. + * @seqno: policy sequence number + */ +int avc_ss_reset(u32 seqno) +{ + struct avc_callback_node *c; + int rc = 0, tmprc; + + avc_flush(); for (c = avc_callbacks; c; c = c->next) { if (c->events & AVC_CALLBACK_RESET) { From 4e6d0bffd3d72a32b620525c9007d2482c731775 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Sat, 12 Sep 2009 22:54:23 -0400 Subject: [PATCH 3/7] SELinux: flush the avc before disabling SELinux Before SELinux is disabled at boot it can create AVC entries. This patch will flush those entries before disabling SELinux. Signed-off-by: Eric Paris Signed-off-by: James Morris --- security/selinux/avc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index f60124623645..1ed0f076aadc 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -868,6 +868,8 @@ u32 avc_policy_seqno(void) void avc_disable(void) { + avc_flush(); + synchronize_rcu(); if (avc_node_cachep) kmem_cache_destroy(avc_node_cachep); } From 4a5d6ba1914d1bf1fcfb5e15834c29d84a879219 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2009 12:45:39 +0100 Subject: [PATCH 4/7] CRED: Allow put_cred() to cope with a NULL groups list put_cred() will oops if given a NULL groups list, but that is now possible with the existence of cred_alloc_blank(), as used in keyctl_session_to_parent(). Added in commit: commit ee18d64c1f632043a02e6f5ba5e045bb26a5465f Author: David Howells Date: Wed Sep 2 09:14:21 2009 +0100 KEYS: Add a keyctl to install a process's session keyring on its parent [try #6] Reported-by: Marc Dionne Signed-off-by: David Howells Signed-off-by: James Morris --- kernel/cred.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/cred.c b/kernel/cred.c index 006fcab009d5..d7f7a01082eb 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -147,7 +147,8 @@ static void put_cred_rcu(struct rcu_head *rcu) key_put(cred->thread_keyring); key_put(cred->request_key_auth); release_tgcred(cred); - put_group_info(cred->group_info); + if (cred->group_info) + put_group_info(cred->group_info); free_uid(cred->user); kmem_cache_free(cred_jar, cred); } From 5c84342a3e147a23752276650340801c237d0e56 Mon Sep 17 00:00:00 2001 From: Marc Dionne Date: Mon, 14 Sep 2009 12:46:23 +0100 Subject: [PATCH 5/7] KEYS: Unlock tasklist when exiting early from keyctl_session_to_parent When we exit early from keyctl_session_to_parent because of permissions or because the session keyring is the same as the parent, we need to unlock the tasklist. The missing unlock causes the system to hang completely when using keyctl(KEYCTL_SESSION_TO_PARENT) with a keyring shared with the parent. Signed-off-by: Marc Dionne Signed-off-by: David Howells Signed-off-by: James Morris --- security/keys/keyctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 74c968524592..60983f38852e 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -1319,6 +1319,7 @@ long keyctl_session_to_parent(void) already_same: ret = 0; not_permitted: + write_unlock_irq(&tasklist_lock); put_cred(cred); return ret; From c08ef808ef24df32e25fbd949fe5310172f3c408 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2009 17:26:13 +0100 Subject: [PATCH 6/7] KEYS: Fix garbage collector Fix a number of problems with the new key garbage collector: (1) A rogue semicolon in keyring_gc() was causing the initial count of dead keys to be miscalculated. (2) A missing return in keyring_gc() meant that under certain circumstances, the keyring semaphore would be unlocked twice. (3) The key serial tree iterator (key_garbage_collector()) part of the garbage collector has been modified to: (a) Complete each scan of the keyrings before setting the new timer. (b) Only set the new timer for keys that have yet to expire. This means that the new timer is now calculated correctly, and the gc doesn't get into a loop continually scanning for keys that have expired, and preventing other things from happening, like RCU cleaning up the old keyring contents. (c) Perform an extra scan if any keys were garbage collected in this one as a key might become garbage during a scan, and (b) could mean we don't set the timer again. (4) Made key_schedule_gc() take the time at which to do a collection run, rather than the time at which the key expires. This means the collection of dead keys (key type unregistered) can happen immediately. Signed-off-by: David Howells Signed-off-by: James Morris --- security/keys/gc.c | 78 +++++++++++++++++++++++++++-------------- security/keys/key.c | 4 +-- security/keys/keyctl.c | 2 +- security/keys/keyring.c | 24 ++++++++++--- 4 files changed, 73 insertions(+), 35 deletions(-) diff --git a/security/keys/gc.c b/security/keys/gc.c index 1e616aef55fd..485fc6233c38 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -26,8 +26,10 @@ static void key_garbage_collector(struct work_struct *); static DEFINE_TIMER(key_gc_timer, key_gc_timer_func, 0, 0); static DECLARE_WORK(key_gc_work, key_garbage_collector); static key_serial_t key_gc_cursor; /* the last key the gc considered */ +static bool key_gc_again; static unsigned long key_gc_executing; static time_t key_gc_next_run = LONG_MAX; +static time_t key_gc_new_timer; /* * Schedule a garbage collection run @@ -40,9 +42,7 @@ void key_schedule_gc(time_t gc_at) kenter("%ld", gc_at - now); - gc_at += key_gc_delay; - - if (now >= gc_at) { + if (gc_at <= now) { schedule_work(&key_gc_work); } else if (gc_at < key_gc_next_run) { expires = jiffies + (gc_at - now) * HZ; @@ -112,16 +112,18 @@ static void key_garbage_collector(struct work_struct *work) struct rb_node *rb; key_serial_t cursor; struct key *key, *xkey; - time_t new_timer = LONG_MAX, limit; + time_t new_timer = LONG_MAX, limit, now; - kenter(""); + now = current_kernel_time().tv_sec; + kenter("[%x,%ld]", key_gc_cursor, key_gc_new_timer - now); if (test_and_set_bit(0, &key_gc_executing)) { - key_schedule_gc(current_kernel_time().tv_sec); + key_schedule_gc(current_kernel_time().tv_sec + 1); + kleave(" [busy; deferring]"); return; } - limit = current_kernel_time().tv_sec; + limit = now; if (limit > key_gc_delay) limit -= key_gc_delay; else @@ -129,12 +131,19 @@ static void key_garbage_collector(struct work_struct *work) spin_lock(&key_serial_lock); - if (RB_EMPTY_ROOT(&key_serial_tree)) - goto reached_the_end; + if (unlikely(RB_EMPTY_ROOT(&key_serial_tree))) { + spin_unlock(&key_serial_lock); + clear_bit(0, &key_gc_executing); + return; + } cursor = key_gc_cursor; if (cursor < 0) cursor = 0; + if (cursor > 0) + new_timer = key_gc_new_timer; + else + key_gc_again = false; /* find the first key above the cursor */ key = NULL; @@ -160,35 +169,50 @@ static void key_garbage_collector(struct work_struct *work) /* trawl through the keys looking for keyrings */ for (;;) { - if (key->expiry > 0 && key->expiry < new_timer) + if (key->expiry > now && key->expiry < new_timer) { + kdebug("will expire %x in %ld", + key_serial(key), key->expiry - now); new_timer = key->expiry; + } if (key->type == &key_type_keyring && - key_gc_keyring(key, limit)) { - /* the gc ate our lock */ - schedule_work(&key_gc_work); - goto no_unlock; - } + key_gc_keyring(key, limit)) + /* the gc had to release our lock so that the keyring + * could be modified, so we have to get it again */ + goto gc_released_our_lock; rb = rb_next(&key->serial_node); - if (!rb) { - key_gc_cursor = 0; - break; - } + if (!rb) + goto reached_the_end; key = rb_entry(rb, struct key, serial_node); } -out: - spin_unlock(&key_serial_lock); -no_unlock: +gc_released_our_lock: + kdebug("gc_released_our_lock"); + key_gc_new_timer = new_timer; + key_gc_again = true; clear_bit(0, &key_gc_executing); - if (new_timer < LONG_MAX) - key_schedule_gc(new_timer); - - kleave(""); + schedule_work(&key_gc_work); + kleave(" [continue]"); return; + /* when we reach the end of the run, we set the timer for the next one */ reached_the_end: + kdebug("reached_the_end"); + spin_unlock(&key_serial_lock); + key_gc_new_timer = new_timer; key_gc_cursor = 0; - goto out; + clear_bit(0, &key_gc_executing); + + if (key_gc_again) { + /* there may have been a key that expired whilst we were + * scanning, so if we discarded any links we should do another + * scan */ + new_timer = now + 1; + key_schedule_gc(new_timer); + } else if (new_timer < LONG_MAX) { + new_timer += key_gc_delay; + key_schedule_gc(new_timer); + } + kleave(" [end]"); } diff --git a/security/keys/key.c b/security/keys/key.c index 08531ad0f252..e50d264c9ad1 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -500,7 +500,7 @@ int key_negate_and_link(struct key *key, set_bit(KEY_FLAG_INSTANTIATED, &key->flags); now = current_kernel_time(); key->expiry = now.tv_sec + timeout; - key_schedule_gc(key->expiry); + key_schedule_gc(key->expiry + key_gc_delay); if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags)) awaken = 1; @@ -909,7 +909,7 @@ void key_revoke(struct key *key) time = now.tv_sec; if (key->revoked_at == 0 || key->revoked_at > time) { key->revoked_at = time; - key_schedule_gc(key->revoked_at); + key_schedule_gc(key->revoked_at + key_gc_delay); } up_write(&key->sem); diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 60983f38852e..2fb28efc5326 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -1115,7 +1115,7 @@ long keyctl_set_timeout(key_serial_t id, unsigned timeout) } key->expiry = expiry; - key_schedule_gc(key->expiry); + key_schedule_gc(key->expiry + key_gc_delay); up_write(&key->sem); key_put(key); diff --git a/security/keys/keyring.c b/security/keys/keyring.c index ac977f661a79..8ec02746ca99 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -1019,18 +1019,18 @@ void keyring_gc(struct key *keyring, time_t limit) struct key *key; int loop, keep, max; - kenter("%x", key_serial(keyring)); + kenter("{%x,%s}", key_serial(keyring), keyring->description); down_write(&keyring->sem); klist = keyring->payload.subscriptions; if (!klist) - goto just_return; + goto no_klist; /* work out how many subscriptions we're keeping */ keep = 0; for (loop = klist->nkeys - 1; loop >= 0; loop--) - if (!key_is_dead(klist->keys[loop], limit)); + if (!key_is_dead(klist->keys[loop], limit)) keep++; if (keep == klist->nkeys) @@ -1041,7 +1041,7 @@ void keyring_gc(struct key *keyring, time_t limit) new = kmalloc(sizeof(struct keyring_list) + max * sizeof(struct key *), GFP_KERNEL); if (!new) - goto just_return; + goto nomem; new->maxkeys = max; new->nkeys = 0; new->delkey = 0; @@ -1081,7 +1081,21 @@ void keyring_gc(struct key *keyring, time_t limit) discard_new: new->nkeys = keep; keyring_clear_rcu_disposal(&new->rcu); + up_write(&keyring->sem); + kleave(" [discard]"); + return; + just_return: up_write(&keyring->sem); - kleave(" [no]"); + kleave(" [no dead]"); + return; + +no_klist: + up_write(&keyring->sem); + kleave(" [no_klist]"); + return; + +nomem: + up_write(&keyring->sem); + kleave(" [oom]"); } From 8a478905adbb2e09a59644e76f7fe7e0ab644204 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 14 Sep 2009 20:59:48 -0400 Subject: [PATCH 7/7] SELinux: inline selinux_is_enabled in !CONFIG_SECURITY_SELINUX Without this patch building a kernel emits millions of warning like: include/linux/selinux.h:92: warning: ?selinux_is_enabled? defined but not used When it is build without CONFIG_SECURITY_SELINUX. This is harmless, but the function should be inlined, so it gets compiled out. Reported-by: Linus Torvalds Signed-off-by: Eric Paris Signed-off-by: James Morris --- include/linux/selinux.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/selinux.h b/include/linux/selinux.h index 223d06a6feb1..82e0f26a1299 100644 --- a/include/linux/selinux.h +++ b/include/linux/selinux.h @@ -89,7 +89,7 @@ static inline void selinux_secmark_refcount_dec(void) return; } -static bool selinux_is_enabled(void) +static inline bool selinux_is_enabled(void) { return false; }