* [PATCH v2 0/3] bpf: tidy up internals of bpf key handling @ 2025-07-30 17:27 James Bottomley 2025-07-30 17:27 ` [PATCH v2 1/3] bpf: make bpf_key an opaque type James Bottomley ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: James Bottomley @ 2025-07-30 17:27 UTC (permalink / raw) To: bpf, linux-trace-kernel; +Cc: Roberto Sassu This patch series reduces the size of the implementing code and eliminates allocations on the bpf_key_lookup paths. There is no externally visible change to the BPF API. v2 fixes the test failures by keeping an empty bpf_key structure and differentiating between failure and builtin key returns. Regards, James James Bottomley (3): bpf: make bpf_key an opaque type bpf: remove bpf_key reference bpf: eliminate the allocation of an intermediate struct bpf_key include/linux/bpf.h | 5 +---- kernel/trace/bpf_trace.c | 47 ++++++++++++++++------------------------ 2 files changed, 20 insertions(+), 32 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/3] bpf: make bpf_key an opaque type 2025-07-30 17:27 [PATCH v2 0/3] bpf: tidy up internals of bpf key handling James Bottomley @ 2025-07-30 17:27 ` James Bottomley 2025-07-30 17:27 ` [PATCH v2 2/3] bpf: remove bpf_key reference James Bottomley 2025-07-30 17:27 ` [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key James Bottomley 2 siblings, 0 replies; 11+ messages in thread From: James Bottomley @ 2025-07-30 17:27 UTC (permalink / raw) To: bpf, linux-trace-kernel; +Cc: Roberto Sassu Since the only consumers of struct bpf_key are bpf scripts which call the bpf kfuncs which take struct bpf_key, only the implementing functions in bpf_trace.c should be reaching inside this structure. Enforce this by making the structure opaque in the header with a body that's only defined inside bpf_trace.c Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- include/linux/bpf.h | 5 +---- kernel/trace/bpf_trace.c | 5 +++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index f9cd2164ed23..34b2df7aaf3e 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -3656,10 +3656,7 @@ static inline void bpf_cgroup_atype_put(int cgroup_atype) {} struct key; #ifdef CONFIG_KEYS -struct bpf_key { - struct key *key; - bool has_ref; -}; +struct bpf_key; #endif /* CONFIG_KEYS */ static inline bool type_is_alloc(u32 type) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 3ae52978cae6..e7bf00d1cd05 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1242,6 +1242,11 @@ 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(); /** -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] bpf: remove bpf_key reference 2025-07-30 17:27 [PATCH v2 0/3] bpf: tidy up internals of bpf key handling James Bottomley 2025-07-30 17:27 ` [PATCH v2 1/3] bpf: make bpf_key an opaque type James Bottomley @ 2025-07-30 17:27 ` James Bottomley 2025-07-31 17:03 ` Alexei Starovoitov 2025-07-30 17:27 ` [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key James Bottomley 2 siblings, 1 reply; 11+ messages in thread From: James Bottomley @ 2025-07-30 17:27 UTC (permalink / raw) To: bpf, linux-trace-kernel; +Cc: Roberto Sassu 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 <James.Bottomley@HansenPartnership.com> --- 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); kfree(bkey); @@ -1377,7 +1374,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p, u32 data_len, sig_len; int ret; - if (trusted_keyring->has_ref) { + if (system_keyring_id_check((unsigned long)trusted_keyring->key) < 0) { /* * Do the permission check deferred in bpf_lookup_user_key(). * See bpf_lookup_user_key() for more details. -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] bpf: remove bpf_key reference 2025-07-30 17:27 ` [PATCH v2 2/3] bpf: remove bpf_key reference James Bottomley @ 2025-07-31 17:03 ` Alexei Starovoitov 2025-07-31 17:27 ` James Bottomley 0 siblings, 1 reply; 11+ messages in thread From: Alexei Starovoitov @ 2025-07-31 17:03 UTC (permalink / raw) To: James Bottomley; +Cc: bpf, linux-trace-kernel, Roberto Sassu On Wed, Jul 30, 2025 at 10:32 AM James Bottomley <James.Bottomley@hansenpartnership.com> 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 <James.Bottomley@HansenPartnership.com> > > --- > 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] bpf: remove bpf_key reference 2025-07-31 17:03 ` Alexei Starovoitov @ 2025-07-31 17:27 ` James Bottomley 2025-07-31 18:04 ` Alexei Starovoitov 0 siblings, 1 reply; 11+ messages in thread From: James Bottomley @ 2025-07-31 17:27 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: bpf, linux-trace-kernel, Roberto Sassu On Thu, 2025-07-31 at 10:03 -0700, Alexei Starovoitov wrote: > On Wed, Jul 30, 2025 at 10:32 AM James Bottomley > <James.Bottomley@hansenpartnership.com> 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 > > <James.Bottomley@HansenPartnership.com> > > > > --- > > 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) > > + q > > key_put(bkey->key); > > Should be (u64) to avoid truncation ? It can't be: gcc only allows pointer to unsigned long conversion, so the statement if (system_keyring_id_check((u64)bkey->key) < 0) produces a pointer to int conversion error. Since the function prototype is u64 the conversion from unsigned long to u64 happens automatically. > 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? Well, remember the id as pointer trick is only used for the system_key lookup. What you get back from user_key lookup is a real pointer to the key (regardless of what serial id you pass in) so there's no chance of getting 1 or 2 back for a user key. However, if you were thinking of overloading key look up, it is currently the case, in spite of the check in lookup above, that user key serial numbers begin at three thanks to this code in key.c:key_alloc_serial() do { get_random_bytes(&key->serial, sizeof(key->serial)); key->serial >>= 1; /* negative numbers are not permitted */ } while (key->serial < 3); David said he would prefer, if we to allow system keyring lookup here, to use negative ids (like keyrings) for them. Regards, James ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] bpf: remove bpf_key reference 2025-07-31 17:27 ` James Bottomley @ 2025-07-31 18:04 ` Alexei Starovoitov 2025-07-31 18:53 ` James Bottomley 0 siblings, 1 reply; 11+ messages in thread From: Alexei Starovoitov @ 2025-07-31 18:04 UTC (permalink / raw) To: James Bottomley; +Cc: bpf, linux-trace-kernel, Roberto Sassu On Thu, Jul 31, 2025 at 10:27 AM James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > On Thu, 2025-07-31 at 10:03 -0700, Alexei Starovoitov wrote: > > On Wed, Jul 30, 2025 at 10:32 AM James Bottomley > > <James.Bottomley@hansenpartnership.com> 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 > > > <James.Bottomley@HansenPartnership.com> > > > > > > --- > > > 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) > > > + q > > > key_put(bkey->key); > > > > Should be (u64) to avoid truncation ? > > It can't be: gcc only allows pointer to unsigned long conversion, so > the statement > > if (system_keyring_id_check((u64)bkey->key) < 0) > > produces a pointer to int conversion error. Since the function > prototype is u64 the conversion from unsigned long to u64 happens > automatically. > > > > 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? > > Well, remember the id as pointer trick is only used for the system_key > lookup. What you get back from user_key lookup is a real pointer to > the key (regardless of what serial id you pass in) so there's no chance > of getting 1 or 2 back for a user key. > > However, if you were thinking of overloading key look up, it is > currently the case, in spite of the check in lookup above, that user > key serial numbers begin at three thanks to this code in > key.c:key_alloc_serial() > > do { > get_random_bytes(&key->serial, sizeof(key->serial)); > > key->serial >>= 1; /* negative numbers are not permitted */ > } while (key->serial < 3); I see. That's what I was missing. > David said he would prefer, if we to allow system keyring lookup here, > to use negative ids (like keyrings) for them. Makes sense to me as well. Do you want to do a follow up or respin this set ? Would be great if he can ack this set too. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] bpf: remove bpf_key reference 2025-07-31 18:04 ` Alexei Starovoitov @ 2025-07-31 18:53 ` James Bottomley 0 siblings, 0 replies; 11+ messages in thread From: James Bottomley @ 2025-07-31 18:53 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: bpf, linux-trace-kernel, Roberto Sassu On Thu, 2025-07-31 at 11:04 -0700, Alexei Starovoitov wrote: > On Thu, Jul 31, 2025 at 10:27 AM James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: [...] > > David said he would prefer, if we to allow system keyring lookup > > here, to use negative ids (like keyrings) for them. > > Makes sense to me as well. > Do you want to do a follow up or respin this set ? No, I think until David decides what he'd like to do for the keyring code it's good as is: keeping the separate system and user key lookups. > Would be great if he can ack this set too. I can ask him, but he's not always responsive. Regards, James ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key 2025-07-30 17:27 [PATCH v2 0/3] bpf: tidy up internals of bpf key handling James Bottomley 2025-07-30 17:27 ` [PATCH v2 1/3] bpf: make bpf_key an opaque type James Bottomley 2025-07-30 17:27 ` [PATCH v2 2/3] bpf: remove bpf_key reference James Bottomley @ 2025-07-30 17:27 ` James Bottomley 2025-07-31 22:28 ` kernel test robot ` (2 more replies) 2 siblings, 3 replies; 11+ messages in thread From: James Bottomley @ 2025-07-30 17:27 UTC (permalink / raw) To: bpf, linux-trace-kernel; +Cc: Roberto Sassu Now that struct bpf_key is an opaque structure only containing a pointer to the key, make it an alias for the key itself and thus eliminate the need to allocate and free the container. Because the return value of bpf_lookup_system_key() is now overloaded with 0 being a legitimate built in key identifier being the same value as NULL indicating failure, key id 0 is swizzled to -1 to distinguish it again and swizzled back in bpf_key_put() and bpf_verify_pkcs7_signature() to ensure correctness. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- v2: keep empty struct bpf_key to avoid BTF problems and swizzle 0 key id. --- kernel/trace/bpf_trace.c | 43 +++++++++++++++------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index c0ccd55a4d91..7242167fd4b6 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1242,9 +1242,11 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = { }; #ifdef CONFIG_KEYS +/* BTF requires this even if it serves no purpose */ struct bpf_key { - struct key *key; }; +/* conventional value to replace zero return which would become NULL */ +const u64 BUILTIN_KEY = -1LL; __bpf_kfunc_start_defs(); @@ -1276,7 +1278,6 @@ __bpf_kfunc_start_defs(); __bpf_kfunc struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags) { key_ref_t key_ref; - struct bpf_key *bkey; if (flags & ~KEY_LOOKUP_ALL) return NULL; @@ -1289,15 +1290,7 @@ __bpf_kfunc struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags) if (IS_ERR(key_ref)) return NULL; - bkey = kmalloc(sizeof(*bkey), GFP_KERNEL); - if (!bkey) { - key_put(key_ref_to_ptr(key_ref)); - return NULL; - } - - bkey->key = key_ref_to_ptr(key_ref); - - return bkey; + return (struct bpf_key *)key_ref_to_ptr(key_ref); } /** @@ -1323,18 +1316,10 @@ __bpf_kfunc struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags) */ __bpf_kfunc struct bpf_key *bpf_lookup_system_key(u64 id) { - struct bpf_key *bkey; - if (system_keyring_id_check(id) < 0) return NULL; - bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC); - if (!bkey) - return NULL; - - bkey->key = (struct key *)(unsigned long)id; - - return bkey; + return (struct bpf_key *)(unsigned long)(id ? id : BUILTIN_KEY); } /** @@ -1346,10 +1331,11 @@ __bpf_kfunc struct bpf_key *bpf_lookup_system_key(u64 id) */ __bpf_kfunc void bpf_key_put(struct bpf_key *bkey) { - if (system_keyring_id_check((unsigned long)bkey->key) < 0) - key_put(bkey->key); + struct key *key = (struct key *)bkey; - kfree(bkey); + if (system_keyring_id_check((unsigned long)key) < 0 && + (unsigned long)key != BUILTIN_KEY) + key_put(key); } #ifdef CONFIG_SYSTEM_DATA_VERIFICATION @@ -1370,11 +1356,15 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p, { struct bpf_dynptr_kern *data_ptr = (struct bpf_dynptr_kern *)data_p; struct bpf_dynptr_kern *sig_ptr = (struct bpf_dynptr_kern *)sig_p; + struct key *key = (struct key *)trusted_keyring; const void *data, *sig; u32 data_len, sig_len; int ret; - if (system_keyring_id_check((unsigned long)trusted_keyring->key) < 0) { + if ((unsigned long)key == BUILTIN_KEY) + key = NULL; + + if (system_keyring_id_check((unsigned long)key) < 0) { /* * Do the permission check deferred in bpf_lookup_user_key(). * See bpf_lookup_user_key() for more details. @@ -1383,7 +1373,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p, * it is already done by keyring_search() called by * find_asymmetric_key(). */ - ret = key_validate(trusted_keyring->key); + ret = key_validate(key); if (ret < 0) return ret; } @@ -1393,8 +1383,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p, sig_len = __bpf_dynptr_size(sig_ptr); sig = __bpf_dynptr_data(sig_ptr, sig_len); - return verify_pkcs7_signature(data, data_len, sig, sig_len, - trusted_keyring->key, + return verify_pkcs7_signature(data, data_len, sig, sig_len, key, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key 2025-07-30 17:27 ` [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key James Bottomley @ 2025-07-31 22:28 ` kernel test robot 2025-08-01 1:59 ` kernel test robot 2025-08-04 6:21 ` Dan Carpenter 2 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2025-07-31 22:28 UTC (permalink / raw) To: James Bottomley, bpf, linux-trace-kernel; +Cc: oe-kbuild-all, Roberto Sassu Hi James, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] [also build test WARNING on bpf/master linus/master v6.16 next-20250731] [cannot apply to bpf-next/net] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/bpf-make-bpf_key-an-opaque-type/20250731-013040 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20250730172745.8480-4-James.Bottomley%40HansenPartnership.com patch subject: [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key config: i386-randconfig-141-20250731 (https://download.01.org/0day-ci/archive/20250801/202508010803.nlVVIZ7G-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202508010803.nlVVIZ7G-lkp@intel.com/ smatch warnings: kernel/trace/bpf_trace.c:1337 bpf_key_put() warn: always true condition '(key != BUILTIN_KEY) => (0-u32max != u64max)' vim +1337 kernel/trace/bpf_trace.c 1324 1325 /** 1326 * bpf_key_put - decrement key reference count if key is valid and free bpf_key 1327 * @bkey: bpf_key structure 1328 * 1329 * Decrement the reference count of the key inside *bkey*, if the pointer 1330 * is valid, and free *bkey*. 1331 */ 1332 __bpf_kfunc void bpf_key_put(struct bpf_key *bkey) 1333 { 1334 struct key *key = (struct key *)bkey; 1335 1336 if (system_keyring_id_check((unsigned long)key) < 0 && > 1337 (unsigned long)key != BUILTIN_KEY) 1338 key_put(key); 1339 } 1340 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key 2025-07-30 17:27 ` [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key James Bottomley 2025-07-31 22:28 ` kernel test robot @ 2025-08-01 1:59 ` kernel test robot 2025-08-04 6:21 ` Dan Carpenter 2 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2025-08-01 1:59 UTC (permalink / raw) To: James Bottomley, bpf, linux-trace-kernel; +Cc: oe-kbuild-all, Roberto Sassu Hi James, kernel test robot noticed the following build warnings: [auto build test WARNING on bpf-next/master] [also build test WARNING on bpf/master linus/master v6.16 next-20250731] [cannot apply to bpf-next/net] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/bpf-make-bpf_key-an-opaque-type/20250731-013040 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20250730172745.8480-4-James.Bottomley%40HansenPartnership.com patch subject: [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key config: i386-randconfig-r133-20250801 (https://download.01.org/0day-ci/archive/20250801/202508010906.eulQ24IN-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250801/202508010906.eulQ24IN-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202508010906.eulQ24IN-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) kernel/trace/bpf_trace.c:834:41: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void [noderef] __user *[addressable] [assigned] [usertype] sival_ptr @@ got void * @@ kernel/trace/bpf_trace.c:834:41: sparse: expected void [noderef] __user *[addressable] [assigned] [usertype] sival_ptr kernel/trace/bpf_trace.c:834:41: sparse: got void * >> kernel/trace/bpf_trace.c:1249:11: sparse: sparse: symbol 'BUILTIN_KEY' was not declared. Should it be static? kernel/trace/bpf_trace.c:3684:52: sparse: sparse: cast removes address space '__user' of expression kernel/trace/bpf_trace.c:3698:56: sparse: sparse: cast removes address space '__user' of expression kernel/trace/bpf_trace.c:3712:52: sparse: sparse: cast removes address space '__user' of expression kernel/trace/bpf_trace.c:3719:56: sparse: sparse: cast removes address space '__user' of expression kernel/trace/bpf_trace.c:3727:52: sparse: sparse: cast removes address space '__user' of expression kernel/trace/bpf_trace.c:3735:56: sparse: sparse: cast removes address space '__user' of expression kernel/trace/bpf_trace.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, include/linux/mmzone.h, ...): include/linux/rcupdate.h:871:25: sparse: sparse: context imbalance in 'uprobe_prog_run' - unexpected unlock vim +/BUILTIN_KEY +1249 kernel/trace/bpf_trace.c 1243 1244 #ifdef CONFIG_KEYS 1245 /* BTF requires this even if it serves no purpose */ 1246 struct bpf_key { 1247 }; 1248 /* conventional value to replace zero return which would become NULL */ > 1249 const u64 BUILTIN_KEY = -1LL; 1250 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key 2025-07-30 17:27 ` [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key James Bottomley 2025-07-31 22:28 ` kernel test robot 2025-08-01 1:59 ` kernel test robot @ 2025-08-04 6:21 ` Dan Carpenter 2 siblings, 0 replies; 11+ messages in thread From: Dan Carpenter @ 2025-08-04 6:21 UTC (permalink / raw) To: oe-kbuild, James Bottomley, bpf, linux-trace-kernel Cc: lkp, oe-kbuild-all, Roberto Sassu Hi James, kernel test robot noticed the following build warnings: https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/bpf-make-bpf_key-an-opaque-type/20250731-013040 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master patch link: https://lore.kernel.org/r/20250730172745.8480-4-James.Bottomley%40HansenPartnership.com patch subject: [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key config: i386-randconfig-141-20250803 (https://download.01.org/0day-ci/archive/20250804/202508040803.nwExqJWe-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202508040803.nwExqJWe-lkp@intel.com/ smatch warnings: kernel/trace/bpf_trace.c:1364 bpf_verify_pkcs7_signature() warn: impossible condition '(key == BUILTIN_KEY) => (0-u32max == u64max)' vim +1364 kernel/trace/bpf_trace.c cce4c40b960673 Daniel Xu 2024-06-12 1353 __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p, cce4c40b960673 Daniel Xu 2024-06-12 1354 struct bpf_dynptr *sig_p, 865b0566d8f1a0 Roberto Sassu 2022-09-20 1355 struct bpf_key *trusted_keyring) 865b0566d8f1a0 Roberto Sassu 2022-09-20 1356 { cce4c40b960673 Daniel Xu 2024-06-12 1357 struct bpf_dynptr_kern *data_ptr = (struct bpf_dynptr_kern *)data_p; cce4c40b960673 Daniel Xu 2024-06-12 1358 struct bpf_dynptr_kern *sig_ptr = (struct bpf_dynptr_kern *)sig_p; 9cc2aa8d6b5c93 James Bottomley 2025-07-30 1359 struct key *key = (struct key *)trusted_keyring; 74523c06ae20b8 Song Liu 2023-11-06 1360 const void *data, *sig; 74523c06ae20b8 Song Liu 2023-11-06 1361 u32 data_len, sig_len; 865b0566d8f1a0 Roberto Sassu 2022-09-20 1362 int ret; 865b0566d8f1a0 Roberto Sassu 2022-09-20 1363 9cc2aa8d6b5c93 James Bottomley 2025-07-30 @1364 if ((unsigned long)key == BUILTIN_KEY) BUILTIN_KEY should be changed to -1L so that this works on 32bit systems. 9cc2aa8d6b5c93 James Bottomley 2025-07-30 1365 key = NULL; 9cc2aa8d6b5c93 James Bottomley 2025-07-30 1366 9cc2aa8d6b5c93 James Bottomley 2025-07-30 1367 if (system_keyring_id_check((unsigned long)key) < 0) { 865b0566d8f1a0 Roberto Sassu 2022-09-20 1368 /* 865b0566d8f1a0 Roberto Sassu 2022-09-20 1369 * Do the permission check deferred in bpf_lookup_user_key(). 865b0566d8f1a0 Roberto Sassu 2022-09-20 1370 * See bpf_lookup_user_key() for more details. 865b0566d8f1a0 Roberto Sassu 2022-09-20 1371 * 865b0566d8f1a0 Roberto Sassu 2022-09-20 1372 * A call to key_task_permission() here would be redundant, as 865b0566d8f1a0 Roberto Sassu 2022-09-20 1373 * it is already done by keyring_search() called by 865b0566d8f1a0 Roberto Sassu 2022-09-20 1374 * find_asymmetric_key(). 865b0566d8f1a0 Roberto Sassu 2022-09-20 1375 */ 9cc2aa8d6b5c93 James Bottomley 2025-07-30 1376 ret = key_validate(key); 865b0566d8f1a0 Roberto Sassu 2022-09-20 1377 if (ret < 0) 865b0566d8f1a0 Roberto Sassu 2022-09-20 1378 return ret; 865b0566d8f1a0 Roberto Sassu 2022-09-20 1379 } 865b0566d8f1a0 Roberto Sassu 2022-09-20 1380 74523c06ae20b8 Song Liu 2023-11-06 1381 data_len = __bpf_dynptr_size(data_ptr); 74523c06ae20b8 Song Liu 2023-11-06 1382 data = __bpf_dynptr_data(data_ptr, data_len); 74523c06ae20b8 Song Liu 2023-11-06 1383 sig_len = __bpf_dynptr_size(sig_ptr); 74523c06ae20b8 Song Liu 2023-11-06 1384 sig = __bpf_dynptr_data(sig_ptr, sig_len); 74523c06ae20b8 Song Liu 2023-11-06 1385 9cc2aa8d6b5c93 James Bottomley 2025-07-30 1386 return verify_pkcs7_signature(data, data_len, sig, sig_len, key, 865b0566d8f1a0 Roberto Sassu 2022-09-20 1387 VERIFYING_UNSPECIFIED_SIGNATURE, NULL, 865b0566d8f1a0 Roberto Sassu 2022-09-20 1388 NULL); 865b0566d8f1a0 Roberto Sassu 2022-09-20 1389 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-08-04 6:22 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-30 17:27 [PATCH v2 0/3] bpf: tidy up internals of bpf key handling James Bottomley 2025-07-30 17:27 ` [PATCH v2 1/3] bpf: make bpf_key an opaque type James Bottomley 2025-07-30 17:27 ` [PATCH v2 2/3] bpf: remove bpf_key reference James Bottomley 2025-07-31 17:03 ` Alexei Starovoitov 2025-07-31 17:27 ` James Bottomley 2025-07-31 18:04 ` Alexei Starovoitov 2025-07-31 18:53 ` James Bottomley 2025-07-30 17:27 ` [PATCH v2 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key James Bottomley 2025-07-31 22:28 ` kernel test robot 2025-08-01 1:59 ` kernel test robot 2025-08-04 6:21 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).