From 62fe318256befbd1b4a6765e71d9c997f768fe79 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 14 Nov 2013 13:02:31 +0000 Subject: KEYS: Fix keyring content gc scanner Key pointers stored in the keyring are marked in bit 1 to indicate if they point to a keyring. We need to strip off this bit before using the pointer when iterating over the keyring for the purpose of looking for links to garbage collect. This means that expirable keyrings aren't correctly expiring because the checker is seeing their key pointer with 2 added to it. Since the fix for this involves knowing about the internals of the keyring, key_gc_keyring() is moved to keyring.c and merged into keyring_gc(). This can be tested by: echo 2 >/proc/sys/kernel/keys/gc_delay keyctl timeout `keyctl add keyring qwerty "" @s` 2 cat /proc/keys sleep 5; cat /proc/keys which should see a keyring called "qwerty" appear in the session keyring and then disappear after it expires, and: echo 2 >/proc/sys/kernel/keys/gc_delay a=`keyctl get_persistent @s` b=`keyctl add keyring 0 "" $a` keyctl add user a a $b keyctl timeout $b 2 cat /proc/keys sleep 5; cat /proc/keys which should see a keyring called "0" with a key called "a" in it appear in the user's persistent keyring (which will be attached to the session keyring) and then both the "0" keyring and the "a" key should disappear when the "0" keyring expires. Signed-off-by: David Howells Acked-by: Simo Sorce --- security/keys/keyring.c | 45 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) (limited to 'security/keys/keyring.c') diff --git a/security/keys/keyring.c b/security/keys/keyring.c index d80311e571c3..69f0cb7bab7e 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -1304,7 +1304,7 @@ static void keyring_revoke(struct key *keyring) } } -static bool gc_iterator(void *object, void *iterator_data) +static bool keyring_gc_select_iterator(void *object, void *iterator_data) { struct key *key = keyring_ptr_to_key(object); time_t *limit = iterator_data; @@ -1315,22 +1315,47 @@ static bool gc_iterator(void *object, void *iterator_data) return true; } +static int keyring_gc_check_iterator(const void *object, void *iterator_data) +{ + const struct key *key = keyring_ptr_to_key(object); + time_t *limit = iterator_data; + + key_check(key); + return key_is_dead(key, *limit); +} + /* - * Collect garbage from the contents of a keyring, replacing the old list with - * a new one with the pointers all shuffled down. + * Garbage collect pointers from a keyring. * - * Dead keys are classed as oned that are flagged as being dead or are revoked, - * expired or negative keys that were revoked or expired before the specified - * limit. + * Not called with any locks held. The keyring's key struct will not be + * deallocated under us as only our caller may deallocate it. */ void keyring_gc(struct key *keyring, time_t limit) { - kenter("{%x,%s}", key_serial(keyring), keyring->description); + int result; + + kenter("%x{%s}", keyring->serial, keyring->description ?: ""); + if (keyring->flags & ((1 << KEY_FLAG_INVALIDATED) | + (1 << KEY_FLAG_REVOKED))) + goto dont_gc; + + /* scan the keyring looking for dead keys */ + rcu_read_lock(); + result = assoc_array_iterate(&keyring->keys, + keyring_gc_check_iterator, &limit); + rcu_read_unlock(); + if (result == true) + goto do_gc; + +dont_gc: + kleave(" [no gc]"); + return; + +do_gc: down_write(&keyring->sem); assoc_array_gc(&keyring->keys, &keyring_assoc_array_ops, - gc_iterator, &limit); + keyring_gc_select_iterator, &limit); up_write(&keyring->sem); - - kleave(""); + kleave(" [gc]"); } -- cgit v1.2.3