* [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
* [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 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
* 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).