aboutsummaryrefslogtreecommitdiffstats
path: root/security/keys
diff options
context:
space:
mode:
authorDavid Howells <dhowells@redhat.com>2013-12-02 06:24:18 -0500
committerDavid Howells <dhowells@redhat.com>2013-12-02 06:24:18 -0500
commit23fd78d76415729b338ff1802a0066b4a62f7fb8 (patch)
treeb832cd635e3e3fe8cca93de1a2e5bb165a968538 /security/keys
parentd54e58b7f01552b0eb7d445f4b58de4499ad5ea6 (diff)
KEYS: Fix multiple key add into associative array
If sufficient keys (or keyrings) are added into a keyring such that a node in the associative array's tree overflows (each node has a capacity N, currently 16) and such that all N+1 keys have the same index key segment for that level of the tree (the level'th nibble of the index key), then assoc_array_insert() calls ops->diff_objects() to indicate at which bit position the two index keys vary. However, __key_link_begin() passes a NULL object to assoc_array_insert() with the intention of supplying the correct pointer later before we commit the change. This means that keyring_diff_objects() is given a NULL pointer as one of its arguments which it does not expect. This results in an oops like the attached. With the previous patch to fix the keyring hash function, this can be forced much more easily by creating a keyring and only adding keyrings to it. Add any other sort of key and a different insertion path is taken - all 16+1 objects must want to cluster in the same node slot. This can be tested by: r=`keyctl newring sandbox @s` for ((i=0; i<=16; i++)); do keyctl newring ring$i $r; done This should work fine, but oopses when the 17th keyring is added. Since ops->diff_objects() is always called with the first pointer pointing to the object to be inserted (ie. the NULL pointer), we can fix the problem by changing the to-be-inserted object pointer to point to the index key passed into assoc_array_insert() instead. Whilst we're at it, we also switch the arguments so that they are the same as for ->compare_object(). BUG: unable to handle kernel NULL pointer dereference at 0000000000000088 IP: [<ffffffff81191ee4>] hash_key_type_and_desc+0x18/0xb0 ... RIP: 0010:[<ffffffff81191ee4>] hash_key_type_and_desc+0x18/0xb0 ... Call Trace: [<ffffffff81191f9d>] keyring_diff_objects+0x21/0xd2 [<ffffffff811f09ef>] assoc_array_insert+0x3b6/0x908 [<ffffffff811929a7>] __key_link_begin+0x78/0xe5 [<ffffffff81191a2e>] key_create_or_update+0x17d/0x36a [<ffffffff81192e0a>] SyS_add_key+0x123/0x183 [<ffffffff81400ddb>] tracesys+0xdd/0xe2 Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Stephen Gallagher <sgallagh@redhat.com>
Diffstat (limited to 'security/keys')
-rw-r--r--security/keys/keyring.c7
1 files changed, 3 insertions, 4 deletions
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 0adbc77a59b9..3dd8445cd489 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -279,12 +279,11 @@ static bool keyring_compare_object(const void *object, const void *data)
279 * Compare the index keys of a pair of objects and determine the bit position 279 * Compare the index keys of a pair of objects and determine the bit position
280 * at which they differ - if they differ. 280 * at which they differ - if they differ.
281 */ 281 */
282static int keyring_diff_objects(const void *_a, const void *_b) 282static int keyring_diff_objects(const void *object, const void *data)
283{ 283{
284 const struct key *key_a = keyring_ptr_to_key(_a); 284 const struct key *key_a = keyring_ptr_to_key(object);
285 const struct key *key_b = keyring_ptr_to_key(_b);
286 const struct keyring_index_key *a = &key_a->index_key; 285 const struct keyring_index_key *a = &key_a->index_key;
287 const struct keyring_index_key *b = &key_b->index_key; 286 const struct keyring_index_key *b = data;
288 unsigned long seg_a, seg_b; 287 unsigned long seg_a, seg_b;
289 int level, i; 288 int level, i;
290 289