Re: [PATCH] keys: Fix linking a duplicate key to a keyring's assoc_array

From: David Howells
Date: Thu Jun 08 2023 - 10:13:50 EST


Apologies for missing this patch.

Petr Pavlu <petr.pavlu@xxxxxxxx> wrote:

> * Back on the first task, function construct_alloc_key() first runs
> __key_link_begin() to determine an assoc_array_edit operation to
> insert a new key. Index keys in the array are compared exactly as-is,
> using keyring_compare_object(). The operation finds that "abcdef" is
> not yet present in the destination keyring.

Good catch, but I think it's probably the wrong solution.

keyring_compare_object() needs to use the ->cmp() function from the key type.

It's not just request_key() that might have a problem, but also key_link().

There are also asymmetric keys which match against multiple criteria - though
I'm fine with just matching the main description there (the important bit
being to maintain the integrity of the tree inside assoc_array).

I wonder, should I scrap the assoc_array approach and go to each keyring being
a linked-list? It's slower, but a lot easier to manage - and more forgiving
of problems.

struct key_ptr {
struct list_head link;
struct key *key;
unsigned long key_hash;
};

I'm also wondering if I should remove the key type from the matching criteria
- i.e. there can only be one key with any particular description in a ring at
once, regardless of type. Unfortunately, this is may be a UAPI breaker
somewhere.

Any thoughts?

David