On Wed, Jul 30, 2025 at 10:32 AM James Bottomley
<[email protected]> wrote:
>
> bpf_key.has_ref is used to distinguish between real key pointers and
> the fake key pointers that are used for system keyrings (to ensure the
> actual pointers to system keyrings are never visible outside
> certs/system_keyring.c). The keyrings subsystem has an exported
> function to do this, so use that in the bpf keyring code eliminating
> the need to store has_ref.
>
> Signed-off-by: James Bottomley <[email protected]>
>
> ---
> v2: use unsigned long for pointer to int conversion
> ---
> kernel/trace/bpf_trace.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index e7bf00d1cd05..c0ccd55a4d91 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1244,7 +1244,6 @@ static const struct bpf_func_proto
> bpf_get_func_arg_cnt_proto = {
> #ifdef CONFIG_KEYS
> struct bpf_key {
> struct key *key;
> - bool has_ref;
> };
>
> __bpf_kfunc_start_defs();
> @@ -1297,7 +1296,6 @@ __bpf_kfunc struct bpf_key *bpf_lookup_user_key(s32
> serial, u64 flags)
> }
>
> bkey->key = key_ref_to_ptr(key_ref);
> - bkey->has_ref = true;
>
> return bkey;
> }
> @@ -1335,7 +1333,6 @@ __bpf_kfunc struct bpf_key *bpf_lookup_system_key(u64
> id)
> return NULL;
>
> bkey->key = (struct key *)(unsigned long)id;
> - bkey->has_ref = false;
>
> return bkey;
> }
> @@ -1349,7 +1346,7 @@ __bpf_kfunc struct bpf_key *bpf_lookup_system_key(u64
> id)
> */
> __bpf_kfunc void bpf_key_put(struct bpf_key *bkey)
> {
> - if (bkey->has_ref)
> + if (system_keyring_id_check((unsigned long)bkey->key) < 0)
> key_put(bkey->key);
Should be (u64) to avoid truncation ?
But is it really the case that id==1 and id==2 are exposed to UAPI already?
As far as I can see lookup_user_key() does:
default:
key_ref = ERR_PTR(-EINVAL);
if (id < 1)
goto error;
key = key_lookup(id);
so only id==0 is invalid, but id=1 can be a valid user key, no?
