KEYS: Fix the keyring hash function
The keyring hash function (used by the associative array) is supposed to clear the bottommost nibble of the index key (where the hash value resides) for keyrings and make sure it is non-zero for non-keyrings. This is done to make keyrings cluster together on one branch of the tree separately to other keys. Unfortunately, the wrong mask is used, so only the bottom two bits are examined and cleared and not the whole bottom nibble. This means that keys and keyrings can still be successfully searched for under most circumstances as the hash is consistent in its miscalculation, but if a keyring's associative array bottom node gets filled up then approx 75% of the keyrings will not be put into the 0 branch. The consequence of this is that a key in a keyring linked to by another keyring, ie. keyring A -> keyring B -> key may not be found if the search starts at keyring A and then descends into keyring B because search_nested_keyrings() only searches up the 0 branch (as it "knows" all keyrings must be there and not elsewhere in the tree). The fix is to use the right mask. This can be tested with: r=`keyctl newring sandbox @s` for ((i=0; i<=16; i++)); do keyctl newring ring$i $r; done for ((i=0; i<=16; i++)); do keyctl add user a$i a %:ring$i; done for ((i=0; i<=16; i++)); do keyctl search $r user a$i; done This creates a sandbox keyring, then creates 17 keyrings therein (labelled ring0..ring16). This causes the root node of the sandbox's associative array to overflow and for the tree to have extra nodes inserted. Each keyring then is given a user key (labelled aN for ringN) for us to search for. We then search for the user keys we added, starting from the sandbox. If working correctly, it should return the same ordered list of key IDs as for...keyctl add... did. Without this patch, it reports ENOKEY "Required key not available" for some of the keys. Just which keys get this depends as the kernel pointer to the key type forms part of the hash function. Reported-by: Nalin Dahyabhai <nalin@redhat.com> Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Stephen Gallagher <sgallagh@redhat.com>
This commit is contained in:
parent
2480f57fb3
commit
d54e58b7f0
1 changed files with 4 additions and 4 deletions
|
@ -160,7 +160,7 @@ static u64 mult_64x32_and_fold(u64 x, u32 y)
|
||||||
static unsigned long hash_key_type_and_desc(const struct keyring_index_key *index_key)
|
static unsigned long hash_key_type_and_desc(const struct keyring_index_key *index_key)
|
||||||
{
|
{
|
||||||
const unsigned level_shift = ASSOC_ARRAY_LEVEL_STEP;
|
const unsigned level_shift = ASSOC_ARRAY_LEVEL_STEP;
|
||||||
const unsigned long level_mask = ASSOC_ARRAY_LEVEL_STEP_MASK;
|
const unsigned long fan_mask = ASSOC_ARRAY_FAN_MASK;
|
||||||
const char *description = index_key->description;
|
const char *description = index_key->description;
|
||||||
unsigned long hash, type;
|
unsigned long hash, type;
|
||||||
u32 piece;
|
u32 piece;
|
||||||
|
@ -194,10 +194,10 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
|
||||||
* ordinary keys by making sure the lowest level segment in the hash is
|
* ordinary keys by making sure the lowest level segment in the hash is
|
||||||
* zero for keyrings and non-zero otherwise.
|
* zero for keyrings and non-zero otherwise.
|
||||||
*/
|
*/
|
||||||
if (index_key->type != &key_type_keyring && (hash & level_mask) == 0)
|
if (index_key->type != &key_type_keyring && (hash & fan_mask) == 0)
|
||||||
return hash | (hash >> (ASSOC_ARRAY_KEY_CHUNK_SIZE - level_shift)) | 1;
|
return hash | (hash >> (ASSOC_ARRAY_KEY_CHUNK_SIZE - level_shift)) | 1;
|
||||||
if (index_key->type == &key_type_keyring && (hash & level_mask) != 0)
|
if (index_key->type == &key_type_keyring && (hash & fan_mask) != 0)
|
||||||
return (hash + (hash << level_shift)) & ~level_mask;
|
return (hash + (hash << level_shift)) & ~fan_mask;
|
||||||
return hash;
|
return hash;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue