Re: [PATCH 6/7] keys: Provide a key_try_get() function and use it

Jarkko Sakkinen
2024-08-27T18:24:36Z

On Wed Aug 21, 2024 at 3:36 PM EEST, David Howells wrote:
> Add a key_try_get() function to try to get a ref on a key and switch code
> that's manipulating the key refcount directly to use it. This will allow a
> tracepoint to be emplaced there later.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jarkko Sakkinen <jarkko@kernel.org>
> cc: keyrings@vger.kernel.org
> cc: linux-security-module@vger.kernel.org
> ---
> include/linux/key.h | 1 +
> security/keys/key.c | 16 +++++++++++++++-
> security/keys/keyring.c | 2 +-
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/key.h b/include/linux/key.h
> index 80d736813b89..4e5baf3e7286 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -300,6 +300,7 @@ extern struct key *key_alloc(struct key_type *type,
> extern void key_revoke(struct key *key);
> extern void key_invalidate(struct key *key);
> struct key *key_get(struct key *key);
> +struct key *key_try_get(struct key *key);
> extern void key_put(struct key *key);
> extern bool key_put_tag(struct key_tag *tag);
> extern void key_remove_domain(struct key_tag *domain_tag);
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 14c7ee77ea15..59cffb6f9b94 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -649,6 +649,20 @@ struct key *key_get(struct key *key)
> }
> EXPORT_SYMBOL(key_get);
>
> +/**
> + * key_try_get - Get a ref on a key if its refcount is not non-zero.
> + * @key: The key to get a reference on.
> + *
> + * Get a reference on a key unless it has no references and return true if
> + * successful. @key must not be NULL.
> + */
> +struct key *key_try_get(struct key *key)
> +{
> + if (!refcount_inc_not_zero(&key->usage))
> + return NULL;
> + return key;
> +}
> +
> /**
> * key_put - Discard a reference to a key.
> * @key: The key to discard a reference from.
> @@ -709,7 +723,7 @@ struct key *key_lookup(key_serial_t id)
> /* A key is allowed to be looked up only if someone still owns a
> * reference to it - otherwise it's awaiting the gc.
> */
> - if (!refcount_inc_not_zero(&key->usage))
> + if (!key_try_get(key))
> goto not_found;
>
> error:
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index e77d927f1d4d..a09a4c2b1bcb 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -1174,7 +1174,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
> /* we've got a match but we might end up racing with
> * key_cleanup() if the keyring is currently 'dead'
> * (ie. it has a zero usage count) */
> - if (!refcount_inc_not_zero(&keyring->usage))
> + if (!key_try_get(keyring))
> continue;
> keyring->last_used_at = ktime_get_real_seconds();
> goto out;

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko