[PATCH 0/3] bpf: tidy up internals of bpf key handling

[PATCH 0/3] bpf: tidy up internals of bpf key handling

[James Bottomley]

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.

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 | 37 +++++++++----------------------------
 2 files changed, 10 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH 1/3] bpf: make bpf_key an opaque type

[James Bottomley]

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

[PATCH 2/3] bpf: remove bpf_key reference

[James Bottomley]

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>
---
 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..9575d018ed0f 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((u64)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((u64)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

Re: [PATCH 2/3] bpf: remove bpf_key reference

[kernel test robot]

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-rc7 next-20250725]
[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/20250724-224304
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250724143428.4416-3-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH 2/3] bpf: remove bpf_key reference
config: i386-randconfig-063-20250725 (https://download.01.org/0day-ci/archive/20250726/202507261944.7ub6moae-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/20250726/202507261944.7ub6moae-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/202507261944.7ub6moae-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:3695:52: sparse: sparse: cast removes address space '__user' of expression
   kernel/trace/bpf_trace.c:3709:56: sparse: sparse: cast removes address space '__user' of expression
   kernel/trace/bpf_trace.c:3723:52: sparse: sparse: cast removes address space '__user' of expression
   kernel/trace/bpf_trace.c:3730:56: sparse: sparse: cast removes address space '__user' of expression
   kernel/trace/bpf_trace.c:3738:52: sparse: sparse: cast removes address space '__user' of expression
   kernel/trace/bpf_trace.c:3746:56: sparse: sparse: cast removes address space '__user' of expression
>> kernel/trace/bpf_trace.c:1349:42: sparse: sparse: non size-preserving pointer to integer cast
   kernel/trace/bpf_trace.c:1377:42: sparse: sparse: non size-preserving pointer to integer cast
   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 +1349 kernel/trace/bpf_trace.c

  1339	
  1340	/**
  1341	 * bpf_key_put - decrement key reference count if key is valid and free bpf_key
  1342	 * @bkey: bpf_key structure
  1343	 *
  1344	 * Decrement the reference count of the key inside *bkey*, if the pointer
  1345	 * is valid, and free *bkey*.
  1346	 */
  1347	__bpf_kfunc void bpf_key_put(struct bpf_key *bkey)
  1348	{
> 1349		if (system_keyring_id_check((u64)bkey->key) < 0)
  1350			key_put(bkey->key);
  1351	
  1352		kfree(bkey);
  1353	}
  1354	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/3] bpf: remove bpf_key reference

[kernel test robot]

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-rc7 next-20250725]
[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/20250724-224304
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250724143428.4416-3-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH 2/3] bpf: remove bpf_key reference
config: i386-randconfig-002-20250725 (https://download.01.org/0day-ci/archive/20250726/202507262040.o1RZHQvf-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250726/202507262040.o1RZHQvf-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/202507262040.o1RZHQvf-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/trace/bpf_trace.c: In function '____bpf_trace_printk':
   kernel/trace/bpf_trace.c:378:9: warning: function '____bpf_trace_printk' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
     378 |         ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
         |         ^~~
   kernel/trace/bpf_trace.c: In function '____bpf_trace_vprintk':
   kernel/trace/bpf_trace.c:434:9: warning: function '____bpf_trace_vprintk' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
     434 |         ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
         |         ^~~
   kernel/trace/bpf_trace.c: In function '____bpf_seq_printf':
   kernel/trace/bpf_trace.c:476:9: warning: function '____bpf_seq_printf' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
     476 |         seq_bprintf(m, fmt, data.bin_args);
         |         ^~~~~~~~~~~
   kernel/trace/bpf_trace.c: In function 'bpf_key_put':
>> kernel/trace/bpf_trace.c:1349:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    1349 |         if (system_keyring_id_check((u64)bkey->key) < 0)
         |                                     ^


vim +1349 kernel/trace/bpf_trace.c

  1339	
  1340	/**
  1341	 * bpf_key_put - decrement key reference count if key is valid and free bpf_key
  1342	 * @bkey: bpf_key structure
  1343	 *
  1344	 * Decrement the reference count of the key inside *bkey*, if the pointer
  1345	 * is valid, and free *bkey*.
  1346	 */
  1347	__bpf_kfunc void bpf_key_put(struct bpf_key *bkey)
  1348	{
> 1349		if (system_keyring_id_check((u64)bkey->key) < 0)
  1350			key_put(bkey->key);
  1351	
  1352		kfree(bkey);
  1353	}
  1354	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 3/3] bpf: eliminate the allocation of an intermediate struct bpf_key

[James Bottomley]

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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 kernel/trace/bpf_trace.c | 39 +++++++++------------------------------
 1 file changed, 9 insertions(+), 30 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 9575d018ed0f..287b69438fac 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1242,10 +1242,6 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = {
 };
 
 #ifdef CONFIG_KEYS
-struct bpf_key {
-	struct key *key;
-};
-
 __bpf_kfunc_start_defs();
 
 /**
@@ -1276,7 +1272,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 +1284,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 +1310,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;
 }
 
 /**
@@ -1346,10 +1325,10 @@ __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((u64)bkey->key) < 0)
-		key_put(bkey->key);
+	struct key *key = (struct key *)bkey;
 
-	kfree(bkey);
+	if (system_keyring_id_check((u64)key) < 0)
+		key_put(key);
 }
 
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
@@ -1370,11 +1349,12 @@ __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((u64)trusted_keyring->key) < 0) {
+	if (system_keyring_id_check((u64)key) < 0) {
 		/*
 		 * Do the permission check deferred in bpf_lookup_user_key().
 		 * See bpf_lookup_user_key() for more details.
@@ -1383,7 +1363,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 +1373,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

Re: [PATCH 0/3] bpf: tidy up internals of bpf key handling

[James Bottomley]

On Thu, 2025-07-24 at 10:34 -0400, James Bottomley wrote:
> 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.

This last bit turns out to be slightly untrue because I've changed the
bpf_lookup_system_key API to overload the NULL pointer: the
builtin_trusted keyring is identified by a key id of 0 which is also a
NULL pointer.  I could fix this by giving a special return (like -1) to
the builtin_trusted keyring and swizzling it back in
bpf_verify_pkcs7_signature(), or I could alter bpf_lookup_system_key to
return ERR_PTR, which would be an API change.  The former is easier and
maintains the API compatibilitys, it's just a bit icky.

Regards,

James