From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "David Howells" <dhowells@redhat.com>
Cc: <keyrings@vger.kernel.org>,
<linux-security-module@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 7/7] keys: Add tracepoints for the keyrings facility
Date: Tue, 27 Aug 2024 21:27:47 +0300 [thread overview]
Message-ID: <D3QWN2NSC5KU.357N7GN0UU6ZW@kernel.org> (raw)
In-Reply-To: <20240821123616.60401-8-dhowells@redhat.com>
On Wed Aug 21, 2024 at 3:36 PM EEST, David Howells wrote:
> Add some tracepoints to aid in debuggin the keyrings facility and
> applications that use it. A number of events and operations are traceable,
> including:
>
> - Allocation
> - Refcounting
> - Instantiation and negative instantiation/rejection
> - Update
> - Detection of key being dead
> - Key quota changes
> - Key quota failure
> - Link, unlink and move
> - Keyring clearance
> - Revocation and invalidation
> - Garbage collection
>
> 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
I have immediate use for this when I take the time and refine [1] so
I guess that goes for existential reason :-)
> ---
> include/trace/events/key.h | 401 +++++++++++++++++++++++++++++++++++++
> security/keys/gc.c | 4 +
> security/keys/internal.h | 1 +
> security/keys/key.c | 50 ++++-
> security/keys/keyctl.c | 2 +
> security/keys/keyring.c | 27 ++-
> 6 files changed, 472 insertions(+), 13 deletions(-)
> create mode 100644 include/trace/events/key.h
>
> diff --git a/include/trace/events/key.h b/include/trace/events/key.h
> new file mode 100644
> index 000000000000..b3f8c39cc0e8
> --- /dev/null
> +++ b/include/trace/events/key.h
> @@ -0,0 +1,401 @@
> +/* Keyrings tracepoints
> + *
> + * Copyright (C) 2024 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM key
> +
> +#if !defined(_TRACE_KEY_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_KEY_H
> +
> +#include <linux/tracepoint.h>
> +
> +struct key;
> +
> +/*
> + * Declare tracing information enums and their string mappings for display.
> + */
> +#define key_ref_traces \
> + EM(key_trace_ref_alloc, "ALLOC") \
> + EM(key_trace_ref_free, "FREE ") \
> + EM(key_trace_ref_gc, "GC ") \
> + EM(key_trace_ref_get, "GET ") \
> + EM(key_trace_ref_put, "PUT ") \
> + E_(key_trace_ref_try_get, "GET ")
> +
> +#define key_dead_traces \
> + EM(key_trace_dead_type_removed, "TYPR") \
> + EM(key_trace_dead_domain_removed, "DOMR") \
> + EM(key_trace_dead_expired, "EXPD") \
> + E_(key_trace_dead_invalidated, "INVL")
> +
> +/*
> + * Generate enums for tracing information.
> + */
> +#ifndef __NETFS_DECLARE_TRACE_ENUMS_ONCE_ONLY
> +#define __NETFS_DECLARE_TRACE_ENUMS_ONCE_ONLY
> +
> +#undef EM
> +#undef E_
> +#define EM(a, b) a,
> +#define E_(a, b) a
> +
> +enum key_dead_trace { key_dead_traces } __mode(byte);
> +enum key_ref_trace { key_ref_traces } __mode(byte);
> +
> +#endif /* end __RXRPC_DECLARE_TRACE_ENUMS_ONCE_ONLY */
> +
> +/*
> + * Export enum symbols via userspace.
> + */
> +#undef EM
> +#undef E_
> +#define EM(a, b) TRACE_DEFINE_ENUM(a);
> +#define E_(a, b) TRACE_DEFINE_ENUM(a);
> +
> +key_dead_traces;
> +key_ref_traces;
> +
> +/*
> + * Now redefine the EM() and E_() macros to map the enums to the strings that
> + * will be printed in the output.
> + */
> +#undef EM
> +#undef E_
> +#define EM(a, b) { a, b },
> +#define E_(a, b) { a, b }
> +
> +TRACE_EVENT(key_alloc,
> + TP_PROTO(const struct key *key),
> +
> + TP_ARGS(key),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + __field(uid_t, uid)
> + __array(char, type, 8)
> + __array(char, desc, 24)
> + ),
> +
> + TP_fast_assign(
> + __entry->key = key->serial;
> + __entry->uid = from_kuid(&init_user_ns, key->uid);
> + strncpy(__entry->type, key->type->name, sizeof(__entry->type) - 1);
> + strncpy(__entry->desc, key->description ?: "", sizeof(__entry->desc) - 1);
> + __entry->type[sizeof(__entry->type) - 1] = 0;
> + __entry->desc[sizeof(__entry->desc) - 1] = 0;
> + ),
> +
> + TP_printk("key=%08x uid=%08x t=%s d=%s",
> + __entry->key,
> + __entry->uid,
> + __entry->type,
> + __entry->desc)
> + );
> +
> +TRACE_EVENT(key_ref,
> + TP_PROTO(key_serial_t key, unsigned int ref, enum key_ref_trace trace,
> + const void *where),
> +
> + TP_ARGS(key, ref, trace, where),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + __field(enum key_ref_trace, trace)
> + __field(unsigned int, ref)
> + __field(const void *, where)
> + ),
> +
> + TP_fast_assign(
> + __entry->key = key;
> + __entry->trace = trace;
> + __entry->ref = ref;
> + __entry->where = where;
> + ),
> +
> + TP_printk("key=%08x %s r=%d pc=%pSR",
> + __entry->key,
> + __print_symbolic(__entry->trace, key_ref_traces),
> + __entry->ref,
> + __entry->where)
> + );
> +
> +TRACE_EVENT(key_instantiate,
> + TP_PROTO(const struct key *key, const struct key_preparsed_payload *prep),
> +
> + TP_ARGS(key, prep),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + __field(unsigned int, datalen)
> + __field(unsigned int, quotalen)
> + ),
> +
> + TP_fast_assign(
> + __entry->key = key->serial;
> + __entry->datalen = prep->datalen;
> + __entry->quotalen = prep->quotalen;
> + ),
> +
> + TP_printk("key=%08x dlen=%u qlen=%u",
> + __entry->key,
> + __entry->datalen,
> + __entry->quotalen)
> + );
> +
> +TRACE_EVENT(key_reject,
> + TP_PROTO(const struct key *key, int error),
> +
> + TP_ARGS(key, error),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + __field(int, error)
> + ),
> +
> + TP_fast_assign(
> + __entry->key = key->serial;
> + __entry->error = error;
> + ),
> +
> + TP_printk("key=%08x err=%d",
> + __entry->key,
> + __entry->error)
> + );
> +
> +TRACE_EVENT(key_update,
> + TP_PROTO(const struct key *key, const struct key_preparsed_payload *prep),
> +
> + TP_ARGS(key, prep),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + __field(unsigned int, datalen)
> + __field(unsigned int, quotalen)
> + ),
> +
> + TP_fast_assign(
> + __entry->key = key->serial;
> + __entry->datalen = prep->datalen;
> + __entry->quotalen = prep->quotalen;
> + ),
> +
> + TP_printk("key=%08x dlen=%u qlen=%u",
> + __entry->key,
> + __entry->datalen,
> + __entry->quotalen)
> + );
> +
> +TRACE_EVENT(key_dead,
> + TP_PROTO(const struct key *key, enum key_dead_trace trace),
> +
> + TP_ARGS(key, trace),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + __field(enum key_dead_trace, trace)
> + ),
> +
> + TP_fast_assign(
> + __entry->key = key->serial;
> + __entry->trace = trace;
> + ),
> +
> + TP_printk("key=%08x %s",
> + __entry->key,
> + __print_symbolic(__entry->trace, key_dead_traces))
> + );
> +
> +TRACE_EVENT(key_quota,
> + TP_PROTO(const struct key_user *user, int change_keys, int change_bytes),
> +
> + TP_ARGS(user, change_keys, change_bytes),
> +
> + TP_STRUCT__entry(
> + __field(uid_t, uid)
> + __field(unsigned int, nkeys)
> + __field(unsigned int, nikeys)
> + __field(unsigned int, qnkeys)
> + __field(unsigned int, qnbytes)
> + __field(int, change_keys)
> + __field(int, change_bytes)
> + ),
> +
> + TP_fast_assign(
> + __entry->uid = from_kuid(&init_user_ns, user->uid);
> + __entry->nkeys = atomic_read(&user->nkeys);
> + __entry->nikeys = atomic_read(&user->nikeys);
> + __entry->qnkeys = user->qnkeys;
> + __entry->qnbytes = user->qnbytes;
> + __entry->change_keys = change_keys;
> + __entry->change_bytes = change_bytes;
> + ),
> +
> + TP_printk("uid=%d nkeys=%u/%u qkeys=%u qpay=%u ckeys=%d cpay=%d",
> + __entry->uid,
> + __entry->nikeys, __entry->nkeys,
> + __entry->qnkeys,
> + __entry->qnbytes,
> + __entry->change_keys, __entry->change_bytes)
> + );
> +
> +TRACE_EVENT(key_edquot,
> + TP_PROTO(const struct key_user *user, unsigned int maxkeys,
> + unsigned int maxbytes, unsigned int reqbytes),
> +
> + TP_ARGS(user, maxkeys, maxbytes, reqbytes),
> +
> + TP_STRUCT__entry(
> + __field(uid_t, uid)
> + __field(unsigned int, nkeys)
> + __field(unsigned int, nikeys)
> + __field(unsigned int, qnkeys)
> + __field(unsigned int, qnbytes)
> + __field(unsigned int, maxkeys)
> + __field(unsigned int, maxbytes)
> + __field(unsigned int, reqbytes)
> + ),
> +
> + TP_fast_assign(
> + __entry->uid = from_kuid(&init_user_ns, user->uid);
> + __entry->nkeys = atomic_read(&user->nkeys);
> + __entry->nikeys = atomic_read(&user->nikeys);
> + __entry->qnkeys = user->qnkeys;
> + __entry->qnbytes = user->qnbytes;
> + __entry->maxkeys = maxkeys;
> + __entry->maxbytes = maxbytes;
> + __entry->reqbytes = reqbytes;
> + ),
> +
> + TP_printk("u=%d nkeys=%u/%u qkeys=%u/%u qpay=%u/%u cpay=%u",
> + __entry->uid,
> + __entry->nikeys, __entry->nkeys,
> + __entry->qnkeys, __entry->maxkeys,
> + __entry->qnbytes, __entry->maxbytes,
> + __entry->reqbytes)
> + );
> +
> +TRACE_EVENT(key_link,
> + TP_PROTO(const struct key *keyring, const struct key *key),
> +
> + TP_ARGS(keyring, key),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, keyring)
> + __field(key_serial_t, key)
> + ),
> +
> + TP_fast_assign(
> + __entry->keyring = keyring->serial;
> + __entry->key = key->serial;
> + ),
> +
> + TP_printk("key=%08x to=%08x",
> + __entry->key, __entry->keyring)
> + );
> +
> +TRACE_EVENT(key_unlink,
> + TP_PROTO(const struct key *keyring, const struct key *key),
> +
> + TP_ARGS(keyring, key),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, keyring)
> + __field(key_serial_t, key)
> + ),
> +
> + TP_fast_assign(
> + __entry->keyring = keyring->serial;
> + __entry->key = key->serial;
> + ),
> +
> + TP_printk("key=%08x from=%08x",
> + __entry->key, __entry->keyring)
> + );
> +
> +TRACE_EVENT(key_move,
> + TP_PROTO(const struct key *from, const struct key *to,
> + const struct key *key),
> +
> + TP_ARGS(from, to, key),
> +
> + TP_STRUCT__entry(
> + __field(key_serial_t, from)
> + __field(key_serial_t, to)
> + __field(key_serial_t, key)
> + ),
> +
> + TP_fast_assign(
> + __entry->from = from->serial;
> + __entry->to = to->serial;
> + __entry->key = key->serial;
> + ),
> +
> + TP_printk("key=%08x from=%08x to=%08x",
> + __entry->key, __entry->from, __entry->to)
> + );
> +
> +TRACE_EVENT(key_clear,
> + TP_PROTO(const struct key *keyring),
> + TP_ARGS(keyring),
> + TP_STRUCT__entry(
> + __field(key_serial_t, keyring)
> + ),
> + TP_fast_assign(
> + __entry->keyring = keyring->serial;
> + ),
> + TP_printk("key=%08x",
> + __entry->keyring)
> + );
> +
> +TRACE_EVENT(key_revoke,
> + TP_PROTO(const struct key *key),
> + TP_ARGS(key),
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + ),
> + TP_fast_assign(
> + __entry->key = key->serial;
> + ),
> + TP_printk("key=%08x",
> + __entry->key)
> + );
> +
> +TRACE_EVENT(key_invalidate,
> + TP_PROTO(const struct key *key),
> + TP_ARGS(key),
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + ),
> + TP_fast_assign(
> + __entry->key = key->serial;
> + ),
> + TP_printk("key=%08x",
> + __entry->key)
> + );
> +
> +TRACE_EVENT(key_gc,
> + TP_PROTO(const struct key *key),
> + TP_ARGS(key),
> + TP_STRUCT__entry(
> + __field(key_serial_t, key)
> + ),
> + TP_fast_assign(
> + __entry->key = key->serial;
> + ),
> + TP_printk("key=%08x",
> + __entry->key)
> + );
> +
> +#undef EM
> +#undef E_
> +#endif /* _TRACE_KEY_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/security/keys/gc.c b/security/keys/gc.c
> index 7d687b0962b1..47f857653854 100644
> --- a/security/keys/gc.c
> +++ b/security/keys/gc.c
> @@ -141,6 +141,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
>
> list_del(&key->graveyard_link);
>
> + trace_key_gc(key);
> kdebug("- %u", key->serial);
> key_check(key);
>
> @@ -163,6 +164,8 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
> key_put_tag(key->domain_tag);
> kfree(key->description);
>
> + trace_key_ref(key->serial, refcount_read(&key->usage),
> + key_trace_ref_free, 0);
> memzero_explicit(key, sizeof(*key));
> kmem_cache_free(key_jar, key);
> }
> @@ -331,6 +334,7 @@ static void key_garbage_collector(struct work_struct *work)
> */
> found_unreferenced_key:
> kdebug("unrefd key %d", key->serial);
> + trace_key_ref(key->serial, refcount_read(&key->usage), key_trace_ref_gc, 0);
> rb_erase(&key->serial_node, &key_serial_tree);
> spin_unlock(&key_serial_lock);
>
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 33c929a6bb97..87600683a7f5 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -20,6 +20,7 @@
> #include <linux/mm.h>
> #include <linux/vmalloc.h>
> #include <keys/key_user.h>
> +#include <trace/events/key.h>
>
> struct iovec;
>
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 59cffb6f9b94..abb8d60a0dc2 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -14,6 +14,7 @@
> #include <linux/workqueue.h>
> #include <linux/random.h>
> #include <linux/err.h>
> +#define CREATE_TRACE_POINTS
> #include "internal.h"
>
> struct kmem_cache *key_jar;
> @@ -264,12 +265,15 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> if (!(flags & KEY_ALLOC_QUOTA_OVERRUN)) {
> if (user->qnkeys + 1 > maxkeys ||
> user->qnbytes + quotalen > maxbytes ||
> - user->qnbytes + quotalen < user->qnbytes)
> + user->qnbytes + quotalen < user->qnbytes) {
> + trace_key_edquot(user, maxkeys, maxbytes, quotalen);
> goto no_quota;
> + }
> }
>
> user->qnkeys++;
> user->qnbytes += quotalen;
> + trace_key_quota(user, +1, quotalen);
> spin_unlock_irqrestore(&user->lock, irqflags);
> }
>
> @@ -319,7 +323,10 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> /* publish the key by giving it a serial number */
> refcount_inc(&key->domain_tag->usage);
> atomic_inc(&user->nkeys);
> + trace_key_quota(user, 0, 0);
> key_alloc_serial(key);
> + trace_key_alloc(key);
> + trace_key_ref(key->serial, 1, key_trace_ref_alloc, __builtin_return_address(0));
>
> error:
> return key;
> @@ -331,6 +338,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> spin_lock_irqsave(&user->lock, irqflags);
> user->qnkeys--;
> user->qnbytes -= quotalen;
> + trace_key_quota(user, -1, -quotalen);
> spin_unlock_irqrestore(&user->lock, irqflags);
> }
> key_user_put(user);
> @@ -344,6 +352,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> spin_lock_irqsave(&user->lock, irqflags);
> user->qnkeys--;
> user->qnbytes -= quotalen;
> + trace_key_quota(user, -1, quotalen);
> spin_unlock_irqrestore(&user->lock, irqflags);
> }
> key_user_put(user);
> @@ -388,11 +397,13 @@ int key_payload_reserve(struct key *key, size_t datalen)
> if (delta > 0 &&
> (key->user->qnbytes + delta > maxbytes ||
> key->user->qnbytes + delta < key->user->qnbytes)) {
> + trace_key_edquot(key->user, 0, maxbytes, datalen);
> ret = -EDQUOT;
> }
> else {
> key->user->qnbytes += delta;
> key->quotalen += delta;
> + trace_key_quota(key->user, 0, delta);
> }
> spin_unlock_irqrestore(&key->user->lock, flags);
> }
> @@ -447,6 +458,7 @@ static int __key_instantiate_and_link(struct key *key,
> if (ret == 0) {
> /* mark the key as being instantiated */
> atomic_inc(&key->user->nikeys);
> + trace_key_instantiate(key, prep);
> mark_key_instantiated(key, 0);
> notify_key(key, NOTIFY_KEY_INSTANTIATED, 0);
>
> @@ -604,6 +616,7 @@ int key_reject_and_link(struct key *key,
> if (key->state == KEY_IS_UNINSTANTIATED) {
> /* mark the key as being negatively instantiated */
> atomic_inc(&key->user->nikeys);
> + trace_key_reject(key, -error);
> mark_key_instantiated(key, -error);
> notify_key(key, NOTIFY_KEY_INSTANTIATED, -error);
> key_set_expiry(key, ktime_get_real_seconds() + timeout);
> @@ -641,10 +654,15 @@ EXPORT_SYMBOL(key_reject_and_link);
> *
> * Get a reference on a key, if not NULL, and return the parameter.
> */
> -struct key *key_get(struct key *key)
> +noinline struct key *key_get(struct key *key)
> {
> - if (key)
> - refcount_inc(&key->usage);
> + if (key) {
> + int r;
> +
> + __refcount_inc(&key->usage, &r);
> + trace_key_ref(key->serial, r + 1, key_trace_ref_get,
> + __builtin_return_address(0));
> + }
> return key;
> }
> EXPORT_SYMBOL(key_get);
> @@ -656,10 +674,14 @@ EXPORT_SYMBOL(key_get);
> * 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)
> +noinline struct key *key_try_get(struct key *key)
> {
> - if (!refcount_inc_not_zero(&key->usage))
> + int r;
> +
> + if (!__refcount_inc_not_zero(&key->usage, &r))
> return NULL;
> + trace_key_ref(key->serial, r + 1, key_trace_ref_try_get,
> + __builtin_return_address(0));
> return key;
> }
>
> @@ -671,12 +693,19 @@ struct key *key_try_get(struct key *key)
> * schedule the cleanup task to come and pull it out of the tree in process
> * context at some later time.
> */
> -void key_put(struct key *key)
> +noinline void key_put(struct key *key)
> {
> if (key) {
> + key_serial_t serial = key->serial;
> + bool zero;
> + int r;
> +
> key_check(key);
>
> - if (refcount_dec_and_test(&key->usage)) {
> + zero = __refcount_dec_and_test(&key->usage, &r);
> + trace_key_ref(serial, r - 1, key_trace_ref_put,
> + __builtin_return_address(0));
> + if (zero) {
> unsigned long flags;
>
> /* deal with the user's key tracking and quota */
> @@ -684,6 +713,7 @@ void key_put(struct key *key)
> spin_lock_irqsave(&key->user->lock, flags);
> key->user->qnkeys--;
> key->user->qnbytes -= key->quotalen;
> + trace_key_quota(key->user, -1, -key->quotalen);
> spin_unlock_irqrestore(&key->user->lock, flags);
> }
> schedule_work(&key_gc_work);
> @@ -807,6 +837,7 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
> ret = key->type->update(key, prep);
> if (ret == 0) {
> /* Updating a negative key positively instantiates it */
> + trace_key_update(key, prep);
> mark_key_instantiated(key, 0);
> notify_key(key, NOTIFY_KEY_UPDATED, 0);
> }
> @@ -1131,6 +1162,7 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
> ret = key->type->update(key, &prep);
> if (ret == 0) {
> /* Updating a negative key positively instantiates it */
> + trace_key_update(key, &prep);
> mark_key_instantiated(key, 0);
> notify_key(key, NOTIFY_KEY_UPDATED, 0);
> }
> @@ -1166,6 +1198,7 @@ void key_revoke(struct key *key)
> */
> down_write_nested(&key->sem, 1);
> if (!test_and_set_bit(KEY_FLAG_REVOKED, &key->flags)) {
> + trace_key_revoke(key);
> notify_key(key, NOTIFY_KEY_REVOKED, 0);
> if (key->type->revoke)
> key->type->revoke(key);
> @@ -1198,6 +1231,7 @@ void key_invalidate(struct key *key)
> if (!test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
> down_write_nested(&key->sem, 1);
> if (!test_and_set_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
> + trace_key_invalidate(key);
> notify_key(key, NOTIFY_KEY_INVALIDATED, 0);
> key_schedule_gc_links();
> }
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index ab927a142f51..f2ef898209e6 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1020,11 +1020,13 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
>
> newowner->qnkeys++;
> newowner->qnbytes += key->quotalen;
> + trace_key_quota(newowner, +1, key->quotalen);
> spin_unlock_irqrestore(&newowner->lock, flags);
>
> spin_lock_irqsave(&key->user->lock, flags);
> key->user->qnkeys--;
> key->user->qnbytes -= key->quotalen;
> + trace_key_quota(newowner, -1, -key->quotalen);
> spin_unlock_irqrestore(&key->user->lock, flags);
> }
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index a09a4c2b1bcb..d67a60142318 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -1368,6 +1368,7 @@ void __key_link(struct key *keyring, struct key *key,
> struct assoc_array_edit **_edit)
> {
> key_get(key);
> + trace_key_link(keyring, key);
> assoc_array_insert_set_object(*_edit, keyring_key_to_ptr(key));
> assoc_array_apply_edit(*_edit);
> *_edit = NULL;
> @@ -1506,6 +1507,7 @@ static int __key_unlink_begin(struct key *keyring, struct key *key,
> static void __key_unlink(struct key *keyring, struct key *key,
> struct assoc_array_edit **_edit)
> {
> + trace_key_unlink(keyring, key);
> assoc_array_apply_edit(*_edit);
> notify_key(keyring, NOTIFY_KEY_UNLINKED, key_serial(key));
> *_edit = NULL;
> @@ -1625,6 +1627,7 @@ int key_move(struct key *key,
> if (ret < 0)
> goto error;
>
> + trace_key_move(from_keyring, to_keyring, key);
> __key_unlink(from_keyring, key, &from_edit);
> __key_link(to_keyring, key, &to_edit);
> error:
> @@ -1654,6 +1657,7 @@ int keyring_clear(struct key *keyring)
>
> down_write(&keyring->sem);
>
> + trace_key_clear(keyring);
> edit = assoc_array_clear(&keyring->keys, &keyring_assoc_array_ops);
> if (IS_ERR(edit)) {
> ret = PTR_ERR(edit);
> @@ -1697,14 +1701,27 @@ static bool key_is_dead(const struct key *key, time64_t limit)
> if (expiry != TIME64_MAX) {
> if (!(key->type->flags & KEY_TYPE_INSTANT_REAP))
> expiry += key_gc_delay;
> - if (expiry <= limit)
> + if (expiry <= limit) {
> + trace_key_dead(key, key_trace_dead_expired);
> return true;
> + }
> + }
> +
> + if (test_bit(KEY_FLAG_DEAD, &key->flags)) {
> + trace_key_dead(key, key_trace_dead_type_removed);
> + return true;
> + }
> +
> + if (test_bit(KEY_FLAG_INVALIDATED, &key->flags)) {
> + trace_key_dead(key, key_trace_dead_invalidated);
> + return true;
> }
>
> - return
> - key->flags & ((1 << KEY_FLAG_DEAD) |
> - (1 << KEY_FLAG_INVALIDATED)) ||
> - key->domain_tag->removed;
> + if (key->domain_tag->removed) {
> + trace_key_dead(key, key_trace_dead_domain_removed);
> + return true;
> + }
> + return false;
> }
>
> static bool keyring_gc_select_iterator(void *object, void *iterator_data)
Splits matter because you can "set and forget" stuff now there is not
much to review in this main patch because the change is so dead
obvious and at sight:
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
I also gave quick test:
Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
[1] TPM2 asymmetric keys:
https://lore.kernel.org/linux-integrity/20240528210823.28798-1-jarkko@kernel.org/
BR, Jarkko
next prev parent reply other threads:[~2024-08-27 18:27 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-21 12:36 [PATCH 0/7] keys: Add tracepoints David Howells
2024-08-21 12:36 ` [PATCH 1/7] keys: Out of line key_is_dead() so it can have tracepoints added in David Howells
2024-08-27 18:22 ` Jarkko Sakkinen
2024-08-21 12:36 ` [PATCH 2/7] keys: Extract struct key_user to its own header for tracing purposes David Howells
2024-08-27 18:23 ` Jarkko Sakkinen
2024-08-21 12:36 ` [PATCH 3/7] keys: Move key_get() out of line so a tracepoint can be added David Howells
2024-08-27 18:23 ` Jarkko Sakkinen
2024-08-21 12:36 ` [PATCH 4/7] keys: Add a key_ref_get() wrapper David Howells
2024-08-27 18:23 ` Jarkko Sakkinen
2024-08-21 12:36 ` [PATCH 5/7] keys: Use key_get() instead of __key_get() David Howells
2024-08-27 18:24 ` Jarkko Sakkinen
2024-08-21 12:36 ` [PATCH 6/7] keys: Provide a key_try_get() function and use it David Howells
2024-08-27 18:24 ` Jarkko Sakkinen
2024-08-21 12:36 ` [PATCH 7/7] keys: Add tracepoints for the keyrings facility David Howells
2024-08-27 18:27 ` Jarkko Sakkinen [this message]
2024-09-28 2:03 ` Justin Stitt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D3QWN2NSC5KU.357N7GN0UU6ZW@kernel.org \
--to=jarkko@kernel.org \
--cc=dhowells@redhat.com \
--cc=keyrings@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.