[PATCH v2 0/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE support

[PATCH v2 0/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE support

[David Windsor]

This series adds BPF_MAP_TYPE_CRED_STORAGE, enabling BPF programs to
associate data with credential structures (struct cred).

Like other local storage types (task, inode, sk), this provides automatic
lifecycle management and is useful for LSM programs tracking credential
state across LSM calls. Lifetime management is necessary for detecting
credential leaks and enforcing time-based security policies.

The implementation uses kfuncs (bpf_cred_storage_get/delete) that return
bpf_local_storage_data pointers, with map values accessible via the data
field.

v2:
- fix kernel ci build error

David Windsor (2):
  bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs
  selftests/bpf: Add cred local storage tests

 include/linux/bpf_lsm.h                       |  35 ++++
 include/linux/bpf_types.h                     |   1 +
 include/uapi/linux/bpf.h                      |   1 +
 kernel/bpf/Makefile                           |   1 +
 kernel/bpf/bpf_cred_storage.c                 | 175 ++++++++++++++++++
 kernel/bpf/syscall.c                          |  10 +-
 kernel/cred.c                                 |   7 +
 security/bpf/hooks.c                          |   1 +
 .../selftests/bpf/prog_tests/cred_storage.c   |  52 ++++++
 .../selftests/bpf/progs/cred_storage.c        |  87 +++++++++
 10 files changed, 367 insertions(+), 3 deletions(-)
 create mode 100644 kernel/bpf/bpf_cred_storage.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cred_storage.c
 create mode 100644 tools/testing/selftests/bpf/progs/cred_storage.c

-- 
2.43.0

[PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[David Windsor]

All other bpf local storage is obtained using helpers which benefit from
RET_PTR_TO_MAP_VALUE_OR_NULL, so can return void * pointers directly to
map values. kfuncs don't have that, so return struct
bpf_local_storage_data * and access map values through sdata->data.

Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 include/linux/bpf_lsm.h       |  35 +++++++
 include/linux/bpf_types.h     |   1 +
 include/uapi/linux/bpf.h      |   1 +
 kernel/bpf/Makefile           |   1 +
 kernel/bpf/bpf_cred_storage.c | 175 ++++++++++++++++++++++++++++++++++
 kernel/bpf/syscall.c          |  10 +-
 kernel/cred.c                 |   7 ++
 security/bpf/hooks.c          |   1 +
 8 files changed, 228 insertions(+), 3 deletions(-)
 create mode 100644 kernel/bpf/bpf_cred_storage.c

diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 643809cc78c3..b0e2e5f2a2b8 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -40,10 +40,27 @@ static inline struct bpf_storage_blob *bpf_inode(
 	return inode->i_security + bpf_lsm_blob_sizes.lbs_inode;
 }
 
+static inline struct bpf_storage_blob *bpf_cred(
+	const struct cred *cred)
+{
+	if (unlikely(!cred->security))
+		return NULL;
+
+	return cred->security + bpf_lsm_blob_sizes.lbs_cred;
+}
+
 extern const struct bpf_func_proto bpf_inode_storage_get_proto;
 extern const struct bpf_func_proto bpf_inode_storage_delete_proto;
 void bpf_inode_storage_free(struct inode *inode);
 
+void bpf_cred_storage_free(struct cred *cred);
+struct bpf_local_storage_data *bpf_cred_storage_get(struct bpf_map *map,
+						    struct cred *cred,
+						    void *init,
+						    int init__sz,
+						    u64 flags);
+int bpf_cred_storage_delete(struct bpf_map *map, struct cred *cred);
+
 void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func);
 
 int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
@@ -81,6 +98,24 @@ static inline void bpf_inode_storage_free(struct inode *inode)
 {
 }
 
+static inline void bpf_cred_storage_free(struct cred *cred)
+{
+}
+
+static inline struct bpf_local_storage_data *bpf_cred_storage_get(struct bpf_map *map,
+								  struct cred *cred,
+								  void *init,
+								  int init__sz,
+								  u64 flags)
+{
+	return NULL;
+}
+
+static inline int bpf_cred_storage_delete(struct bpf_map *map, struct cred *cred)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
 					   bpf_func_t *bpf_func)
 {
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index fa78f49d4a9a..b8349e837158 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -108,6 +108,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
 #ifdef CONFIG_BPF_LSM
 BPF_MAP_TYPE(BPF_MAP_TYPE_INODE_STORAGE, inode_storage_map_ops)
+BPF_MAP_TYPE(BPF_MAP_TYPE_CRED_STORAGE, cred_storage_map_ops)
 #endif
 BPF_MAP_TYPE(BPF_MAP_TYPE_TASK_STORAGE, task_storage_map_ops)
 #ifdef CONFIG_NET
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 233de8677382..8ce34453b907 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1026,6 +1026,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_USER_RINGBUF,
 	BPF_MAP_TYPE_CGRP_STORAGE,
 	BPF_MAP_TYPE_ARENA,
+	BPF_MAP_TYPE_CRED_STORAGE,
 	__MAX_BPF_MAP_TYPE
 };
 
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index f6cf8c2af5f7..7fd8746b2d51 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list
 obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o
 obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o
 obj-${CONFIG_BPF_LSM}	  += bpf_inode_storage.o
+obj-${CONFIG_BPF_LSM}	  += bpf_cred_storage.o
 obj-$(CONFIG_BPF_SYSCALL) += disasm.o mprog.o
 obj-$(CONFIG_BPF_JIT) += trampoline.o
 obj-$(CONFIG_BPF_SYSCALL) += btf.o memalloc.o rqspinlock.o stream.o
diff --git a/kernel/bpf/bpf_cred_storage.c b/kernel/bpf/bpf_cred_storage.c
new file mode 100644
index 000000000000..3202bb95830e
--- /dev/null
+++ b/kernel/bpf/bpf_cred_storage.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/rculist.h>
+#include <linux/list.h>
+#include <linux/hash.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/bpf.h>
+#include <linux/bpf_local_storage.h>
+#include <linux/bpf_lsm.h>
+#include <linux/cred.h>
+#include <linux/btf_ids.h>
+#include <linux/rcupdate_trace.h>
+
+DEFINE_BPF_STORAGE_CACHE(cred_cache);
+
+static struct bpf_local_storage __rcu **cred_storage_ptr(void *owner)
+{
+	struct cred *cred = owner;
+	struct bpf_storage_blob *bsb;
+
+	bsb = bpf_cred(cred);
+	if (!bsb)
+		return NULL;
+	return &bsb->storage;
+}
+
+static struct bpf_local_storage_data *cred_storage_lookup(struct cred *cred,
+							  struct bpf_map *map,
+							  bool cacheit_lockit)
+{
+	struct bpf_local_storage *cred_storage;
+	struct bpf_local_storage_map *smap;
+	struct bpf_storage_blob *bsb;
+
+	bsb = bpf_cred(cred);
+	if (!bsb)
+		return NULL;
+
+	cred_storage = rcu_dereference_check(bsb->storage, bpf_rcu_lock_held());
+	if (!cred_storage)
+		return NULL;
+
+	smap = (struct bpf_local_storage_map *)map;
+	return bpf_local_storage_lookup(cred_storage, smap, cacheit_lockit);
+}
+
+void bpf_cred_storage_free(struct cred *cred)
+{
+	struct bpf_local_storage *local_storage;
+	struct bpf_storage_blob *bsb;
+
+	bsb = bpf_cred(cred);
+	if (!bsb)
+		return;
+
+	migrate_disable();
+	rcu_read_lock();
+
+	local_storage = rcu_dereference(bsb->storage);
+	if (!local_storage)
+		goto out;
+
+	bpf_local_storage_destroy(local_storage);
+out:
+	rcu_read_unlock();
+	migrate_enable();
+}
+
+static int cred_storage_delete(struct cred *cred, struct bpf_map *map)
+{
+	struct bpf_local_storage_data *sdata;
+
+	sdata = cred_storage_lookup(cred, map, false);
+	if (!sdata)
+		return -ENOENT;
+
+	bpf_selem_unlink(SELEM(sdata), false);
+
+	return 0;
+}
+
+static struct bpf_map *cred_storage_map_alloc(union bpf_attr *attr)
+{
+	return bpf_local_storage_map_alloc(attr, &cred_cache, false);
+}
+
+static void cred_storage_map_free(struct bpf_map *map)
+{
+	bpf_local_storage_map_free(map, &cred_cache, NULL);
+}
+
+static int notsupp_get_next_key(struct bpf_map *map, void *key,
+				void *next_key)
+{
+	return -ENOTSUPP;
+}
+
+const struct bpf_map_ops cred_storage_map_ops = {
+	.map_meta_equal = bpf_map_meta_equal,
+	.map_alloc_check = bpf_local_storage_map_alloc_check,
+	.map_alloc = cred_storage_map_alloc,
+	.map_free = cred_storage_map_free,
+	.map_get_next_key = notsupp_get_next_key,
+	.map_check_btf = bpf_local_storage_map_check_btf,
+	.map_mem_usage = bpf_local_storage_map_mem_usage,
+	.map_btf_id = &bpf_local_storage_map_btf_id[0],
+	.map_owner_storage_ptr = cred_storage_ptr,
+};
+
+BTF_ID_LIST_SINGLE(bpf_cred_storage_btf_ids, struct, cred)
+
+__bpf_kfunc struct bpf_local_storage_data *bpf_cred_storage_get(struct bpf_map *map,
+								struct cred *cred,
+								void *init,
+								int init__sz,
+								u64 flags)
+{
+	struct bpf_local_storage_data *sdata;
+
+	WARN_ON_ONCE(!bpf_rcu_lock_held());
+	if (flags & ~(BPF_LOCAL_STORAGE_GET_F_CREATE))
+		return NULL;
+
+	if (!cred || !cred_storage_ptr(cred))
+		return NULL;
+
+	sdata = cred_storage_lookup(cred, map, true);
+	if (sdata)
+		return sdata;
+
+	/* This helper must only called from where the cred is guaranteed
+	 * to have a refcount and cannot be freed.
+	 */
+	if (flags & BPF_LOCAL_STORAGE_GET_F_CREATE) {
+		sdata = bpf_local_storage_update(
+			cred, (struct bpf_local_storage_map *)map, init,
+			BPF_NOEXIST, false, GFP_ATOMIC);
+		return IS_ERR(sdata) ? NULL : sdata;
+	}
+
+	return NULL;
+}
+
+__bpf_kfunc int bpf_cred_storage_delete(struct bpf_map *map, struct cred *cred)
+{
+	if (!cred)
+		return -EINVAL;
+
+	return cred_storage_delete(cred, map);
+}
+
+BTF_KFUNCS_START(bpf_cred_storage_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_cred_storage_delete, 0)
+BTF_ID_FLAGS(func, bpf_cred_storage_get, KF_RET_NULL)
+BTF_KFUNCS_END(bpf_cred_storage_kfunc_ids)
+
+static const struct btf_kfunc_id_set bpf_cred_storage_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set   = &bpf_cred_storage_kfunc_ids,
+};
+
+static int __init bpf_cred_storage_init(void)
+{
+	int err;
+	err = register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_cred_storage_kfunc_set);
+	if (err) {
+		pr_err("bpf_cred_storage: failed to register kfuncs: %d\n", err);
+		return err;
+	}
+
+	pr_info("bpf_cred_storage: kfuncs registered successfully\n");
+	return 0;
+}
+late_initcall(bpf_cred_storage_init);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 3f178a0f8eb1..f03811efe266 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1262,7 +1262,8 @@ static int map_check_btf(struct bpf_map *map, struct bpf_token *token,
 				    map->map_type != BPF_MAP_TYPE_SK_STORAGE &&
 				    map->map_type != BPF_MAP_TYPE_INODE_STORAGE &&
 				    map->map_type != BPF_MAP_TYPE_TASK_STORAGE &&
-				    map->map_type != BPF_MAP_TYPE_CGRP_STORAGE) {
+				    map->map_type != BPF_MAP_TYPE_CGRP_STORAGE &&
+				    map->map_type != BPF_MAP_TYPE_CRED_STORAGE) {
 					ret = -EOPNOTSUPP;
 					goto free_map_tab;
 				}
@@ -1289,13 +1290,15 @@ static int map_check_btf(struct bpf_map *map, struct bpf_token *token,
 				    map->map_type != BPF_MAP_TYPE_SK_STORAGE &&
 				    map->map_type != BPF_MAP_TYPE_INODE_STORAGE &&
 				    map->map_type != BPF_MAP_TYPE_TASK_STORAGE &&
-				    map->map_type != BPF_MAP_TYPE_CGRP_STORAGE) {
+				    map->map_type != BPF_MAP_TYPE_CGRP_STORAGE &&
+				    map->map_type != BPF_MAP_TYPE_CRED_STORAGE) {
 					ret = -EOPNOTSUPP;
 					goto free_map_tab;
 				}
 				break;
 			case BPF_UPTR:
-				if (map->map_type != BPF_MAP_TYPE_TASK_STORAGE) {
+				if (map->map_type != BPF_MAP_TYPE_TASK_STORAGE &&
+			    map->map_type != BPF_MAP_TYPE_CRED_STORAGE) {
 					ret = -EOPNOTSUPP;
 					goto free_map_tab;
 				}
@@ -1449,6 +1452,7 @@ static int map_create(union bpf_attr *attr, bool kernel)
 	case BPF_MAP_TYPE_SK_STORAGE:
 	case BPF_MAP_TYPE_INODE_STORAGE:
 	case BPF_MAP_TYPE_TASK_STORAGE:
+	case BPF_MAP_TYPE_CRED_STORAGE:
 	case BPF_MAP_TYPE_CGRP_STORAGE:
 	case BPF_MAP_TYPE_BLOOM_FILTER:
 	case BPF_MAP_TYPE_LPM_TRIE:
diff --git a/kernel/cred.c b/kernel/cred.c
index 9676965c0981..a1be27fe5f4c 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -38,6 +38,10 @@ static struct kmem_cache *cred_jar;
 /* init to 2 - one for init_task, one to ensure it is never freed */
 static struct group_info init_groups = { .usage = REFCOUNT_INIT(2) };
 
+#ifdef CONFIG_BPF_LSM
+#include <linux/bpf_lsm.h>
+#endif
+
 /*
  * The initial credentials for the initial task
  */
@@ -76,6 +80,9 @@ static void put_cred_rcu(struct rcu_head *rcu)
 		      cred, atomic_long_read(&cred->usage));
 
 	security_cred_free(cred);
+#ifdef CONFIG_BPF_LSM
+	bpf_cred_storage_free(cred);
+#endif
 	key_put(cred->session_keyring);
 	key_put(cred->process_keyring);
 	key_put(cred->thread_keyring);
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index db759025abe1..d42badc18eb6 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -30,6 +30,7 @@ static int __init bpf_lsm_init(void)
 
 struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = {
 	.lbs_inode = sizeof(struct bpf_storage_blob),
+	.lbs_cred = sizeof(struct bpf_storage_blob),
 };
 
 DEFINE_LSM(bpf) = {
-- 
2.43.0

[PATCH 2/2] selftests/bpf: Add cred local storage tests

[David Windsor]

Add test coverage for the new BPF_MAP_TYPE_CRED_STORAGE map type.
The test verifies that credential storage can be created, accessed,
and persists across credential lifecycle events.

Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 .../selftests/bpf/prog_tests/cred_storage.c   | 52 +++++++++++
 .../selftests/bpf/progs/cred_storage.c        | 87 +++++++++++++++++++
 2 files changed, 139 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cred_storage.c
 create mode 100644 tools/testing/selftests/bpf/progs/cred_storage.c

diff --git a/tools/testing/selftests/bpf/prog_tests/cred_storage.c b/tools/testing/selftests/bpf/prog_tests/cred_storage.c
new file mode 100644
index 000000000000..1a99f6453a0f
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cred_storage.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+#include "cred_storage.skel.h"
+
+static void test_cred_lifecycle(void)
+{
+	struct cred_storage *skel;
+	pid_t child;
+	int status, err;
+
+	skel = cred_storage__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_load"))
+		return;
+
+	err = cred_storage__attach(skel);
+	if (!ASSERT_OK(err, "attach"))
+		goto cleanup;
+
+	skel->data->cred_storage_result = -1;
+
+	skel->bss->monitored_pid = getpid();
+
+	child = fork();
+	if (child == 0) {
+		/* forces cred_prepare with new credentials */
+		exit(0);
+	} else if (child > 0) {
+		waitpid(child, &status, 0);
+
+		/* give time for cred_free hook to run */
+		usleep(10000);
+
+		/* verify that the dummy value was stored and persisted */
+		ASSERT_EQ(skel->data->cred_storage_result, 0,
+			  "cred_storage_dummy_value");
+	} else {
+		ASSERT_TRUE(false, "fork failed");
+	}
+
+cleanup:
+	cred_storage__destroy(skel);
+}
+
+void test_cred_storage(void)
+{
+	if (test__start_subtest("lifecycle"))
+		test_cred_lifecycle();
+}
diff --git a/tools/testing/selftests/bpf/progs/cred_storage.c b/tools/testing/selftests/bpf/progs/cred_storage.c
new file mode 100644
index 000000000000..ae66d3b00d2e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cred_storage.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2025 David Windsor.
+ */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+#define DUMMY_STORAGE_VALUE 0xdeadbeef
+
+extern struct bpf_local_storage_data *bpf_cred_storage_get(struct bpf_map *map,
+							   struct cred *cred,
+							   void *init, int init__sz, __u64 flags) __ksym;
+
+__u32 monitored_pid = 0;
+int cred_storage_result = -1;
+
+struct cred_storage {
+	__u32 value;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_CRED_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct cred_storage);
+} cred_storage_map SEC(".maps");
+
+SEC("lsm/cred_prepare")
+int BPF_PROG(cred_prepare, struct cred *new, const struct cred *old, gfp_t gfp)
+{
+	__u32 pid = bpf_get_current_pid_tgid() >> 32;
+	struct cred_storage init_storage = {
+		.value = DUMMY_STORAGE_VALUE,
+	};
+	struct bpf_local_storage_data *sdata;
+	struct cred_storage *storage;
+
+	if (pid != monitored_pid)
+		return 0;
+
+	sdata = bpf_cred_storage_get((struct bpf_map *)&cred_storage_map, new, &init_storage,
+				     sizeof(init_storage), BPF_LOCAL_STORAGE_GET_F_CREATE);
+	if (!sdata)
+		return 0;
+
+	storage = (struct cred_storage *)sdata->data;
+	if (!storage)
+		return 0;
+
+	/* Verify the storage was initialized correctly */
+	if (storage->value == DUMMY_STORAGE_VALUE)
+		cred_storage_result = 0;
+
+	return 0;
+}
+
+SEC("lsm/cred_free")
+int BPF_PROG(cred_free, struct cred *cred)
+{
+	__u32 pid = bpf_get_current_pid_tgid() >> 32;
+	struct bpf_local_storage_data *sdata;
+	struct cred_storage *storage;
+
+	if (pid != monitored_pid)
+		return 0;
+
+	/* Try to retrieve the storage that should have been created in prepare */
+	sdata = bpf_cred_storage_get((struct bpf_map *)&cred_storage_map, cred,
+				     NULL, 0, 0);
+	if (!sdata)
+		return 0;
+
+	storage = (struct cred_storage *)sdata->data;
+	if (!storage)
+		return 0;
+
+	/* Verify the dummy value is still there during free */
+	if (storage->value == DUMMY_STORAGE_VALUE)
+		cred_storage_result = 0;
+
+	return 0;
+}
-- 
2.43.0

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[Song Liu]

On Fri, Sep 12, 2025 at 3:25 PM David Windsor <dwindsor@gmail.com> wrote:
>
> All other bpf local storage is obtained using helpers which benefit from
> RET_PTR_TO_MAP_VALUE_OR_NULL, so can return void * pointers directly to
> map values. kfuncs don't have that, so return struct
> bpf_local_storage_data * and access map values through sdata->data.
>
> Signed-off-by: David Windsor <dwindsor@gmail.com>

Maybe I missed something, but I think you haven't addressed Alexei's
question in v1: why this is needed and why hash map is not sufficient.

Other local storage types (task, inode, sk storage) may get a large
number of entries in a system, and thus would benefit from object
local storage. I don't think we expect too many creds in a system.
hash map of a smallish size should be good in most cases, and be
faster than cred local storage.

Did I get this right?

Thanks,
Song

The following are some quick feedbacks of the patch, but let's
first address the question above.

This is hitting KASAN BUG in CI:

https://github.com/kernel-patches/bpf/actions/runs/17687566710/job/50275683479

(You may need to log in GitHub to see details).

[...]

> +
> +__bpf_kfunc int bpf_cred_storage_delete(struct bpf_map *map, struct cred *cred)
> +{
> +       if (!cred)
> +               return -EINVAL;
> +
> +       return cred_storage_delete(cred, map);
> +}
> +
> +BTF_KFUNCS_START(bpf_cred_storage_kfunc_ids)
> +BTF_ID_FLAGS(func, bpf_cred_storage_delete, 0)
> +BTF_ID_FLAGS(func, bpf_cred_storage_get, KF_RET_NULL)
> +BTF_KFUNCS_END(bpf_cred_storage_kfunc_ids)
> +
> +static const struct btf_kfunc_id_set bpf_cred_storage_kfunc_set = {
> +       .owner = THIS_MODULE,
> +       .set   = &bpf_cred_storage_kfunc_ids,
> +};
> +
> +static int __init bpf_cred_storage_init(void)
> +{
> +       int err;

We need an empty line after the declaration.
scripts/checkpatch.pl should warn this. Please fix other warnings
from checkpatch.pl.

> +       err = register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_cred_storage_kfunc_set);
> +       if (err) {

[...]

> diff --git a/kernel/cred.c b/kernel/cred.c
> index 9676965c0981..a1be27fe5f4c 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -38,6 +38,10 @@ static struct kmem_cache *cred_jar;
>  /* init to 2 - one for init_task, one to ensure it is never freed */
>  static struct group_info init_groups = { .usage = REFCOUNT_INIT(2) };
>
> +#ifdef CONFIG_BPF_LSM
> +#include <linux/bpf_lsm.h>
> +#endif

We defined a dummy version of bpf_cred_storage_free
in bpf_lsm.h, so the ifdef here is not needed.

> +
>  /*
>   * The initial credentials for the initial task
>   */
> @@ -76,6 +80,9 @@ static void put_cred_rcu(struct rcu_head *rcu)
>                       cred, atomic_long_read(&cred->usage));
>
>         security_cred_free(cred);
> +#ifdef CONFIG_BPF_LSM
> +       bpf_cred_storage_free(cred);
> +#endif

Ditto.

>         key_put(cred->session_keyring);
>         key_put(cred->process_keyring);
>         key_put(cred->thread_keyring);
[...]

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[David Windsor]

On Fri, Sep 12, 2025 at 7:12 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Sep 12, 2025 at 3:25 PM David Windsor <dwindsor@gmail.com> wrote:
> >
> > All other bpf local storage is obtained using helpers which benefit from
> > RET_PTR_TO_MAP_VALUE_OR_NULL, so can return void * pointers directly to
> > map values. kfuncs don't have that, so return struct
> > bpf_local_storage_data * and access map values through sdata->data.
> >
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>
> Maybe I missed something, but I think you haven't addressed Alexei's
> question in v1: why this is needed and why hash map is not sufficient.
>
> Other local storage types (task, inode, sk storage) may get a large
> number of entries in a system, and thus would benefit from object
> local storage. I don't think we expect too many creds in a system.
> hash map of a smallish size should be good in most cases, and be
> faster than cred local storage.
>
> Did I get this right?
>
> Thanks,
> Song
>

Yes I think I addressed in the cover letter of -v2:

"Like other local storage types (task, inode, sk), this provides automatic
lifecycle management and is useful for LSM programs tracking credential
state across LSM calls. Lifetime management is necessary for detecting
credential leaks and enforcing time-based security policies."

You're right it's faster and there aren't many creds, but I feel like
in this case, it'll be a nightmare to manual cleanup with hashmaps. I
think the correctness we get with lifetime management is worth it in
this case, but could be convinced otherwise. Many cred usage patterns
are short lived and a hash map could quickly become stale...






> The following are some quick feedbacks of the patch, but let's
> first address the question above.
>
> This is hitting KASAN BUG in CI:
>
> https://github.com/kernel-patches/bpf/actions/runs/17687566710/job/50275683479
>
> (You may need to log in GitHub to see details).
>
> [...]
>
> > +
> > +__bpf_kfunc int bpf_cred_storage_delete(struct bpf_map *map, struct cred *cred)
> > +{
> > +       if (!cred)
> > +               return -EINVAL;
> > +
> > +       return cred_storage_delete(cred, map);
> > +}
> > +
> > +BTF_KFUNCS_START(bpf_cred_storage_kfunc_ids)
> > +BTF_ID_FLAGS(func, bpf_cred_storage_delete, 0)
> > +BTF_ID_FLAGS(func, bpf_cred_storage_get, KF_RET_NULL)
> > +BTF_KFUNCS_END(bpf_cred_storage_kfunc_ids)
> > +
> > +static const struct btf_kfunc_id_set bpf_cred_storage_kfunc_set = {
> > +       .owner = THIS_MODULE,
> > +       .set   = &bpf_cred_storage_kfunc_ids,
> > +};
> > +
> > +static int __init bpf_cred_storage_init(void)
> > +{
> > +       int err;
>
> We need an empty line after the declaration.
> scripts/checkpatch.pl should warn this. Please fix other warnings
> from checkpatch.pl.
>
> > +       err = register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_cred_storage_kfunc_set);
> > +       if (err) {
>
> [...]
>
> > diff --git a/kernel/cred.c b/kernel/cred.c
> > index 9676965c0981..a1be27fe5f4c 100644
> > --- a/kernel/cred.c
> > +++ b/kernel/cred.c
> > @@ -38,6 +38,10 @@ static struct kmem_cache *cred_jar;
> >  /* init to 2 - one for init_task, one to ensure it is never freed */
> >  static struct group_info init_groups = { .usage = REFCOUNT_INIT(2) };
> >
> > +#ifdef CONFIG_BPF_LSM
> > +#include <linux/bpf_lsm.h>
> > +#endif
>
> We defined a dummy version of bpf_cred_storage_free
> in bpf_lsm.h, so the ifdef here is not needed.
>
> > +
> >  /*
> >   * The initial credentials for the initial task
> >   */
> > @@ -76,6 +80,9 @@ static void put_cred_rcu(struct rcu_head *rcu)
> >                       cred, atomic_long_read(&cred->usage));
> >
> >         security_cred_free(cred);
> > +#ifdef CONFIG_BPF_LSM
> > +       bpf_cred_storage_free(cred);
> > +#endif
>
> Ditto.
>
> >         key_put(cred->session_keyring);
> >         key_put(cred->process_keyring);
> >         key_put(cred->thread_keyring);
> [...]

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[Song Liu]

On Fri, Sep 12, 2025 at 5:27 PM David Windsor <dwindsor@gmail.com> wrote:
[...]
> >
> > Maybe I missed something, but I think you haven't addressed Alexei's
> > question in v1: why this is needed and why hash map is not sufficient.
> >
> > Other local storage types (task, inode, sk storage) may get a large
> > number of entries in a system, and thus would benefit from object
> > local storage. I don't think we expect too many creds in a system.
> > hash map of a smallish size should be good in most cases, and be
> > faster than cred local storage.
> >
> > Did I get this right?
> >
> > Thanks,
> > Song
> >
>
> Yes I think I addressed in the cover letter of -v2:
>
> "Like other local storage types (task, inode, sk), this provides automatic
> lifecycle management and is useful for LSM programs tracking credential
> state across LSM calls. Lifetime management is necessary for detecting
> credential leaks and enforcing time-based security policies."
>
> You're right it's faster and there aren't many creds, but I feel like
> in this case, it'll be a nightmare to manual cleanup with hashmaps. I
> think the correctness we get with lifetime management is worth it in
> this case, but could be convinced otherwise. Many cred usage patterns
> are short lived and a hash map could quickly become stale...

We can clean up the hashmap in hook cred_free, no? The following
check in security_cred_free() seems problematic:

        if (unlikely(cred->security == NULL))
                return;

But as far as I can tell, it is not really useful, and can be removed.
With this removed, hash map will work just as well. Did I miss
something?

Thanks,
Song

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[Alexei Starovoitov]

On Sat, Sep 13, 2025 at 3:27 PM David Windsor <dwindsor@gmail.com> wrote:
>
>
>
> On Sat, Sep 13, 2025 at 5:58 PM Song Liu <song@kernel.org> wrote:
>>
>> On Fri, Sep 12, 2025 at 5:27 PM David Windsor <dwindsor@gmail.com> wrote:
>> [...]
>> > >
>> > > Maybe I missed something, but I think you haven't addressed Alexei's
>> > > question in v1: why this is needed and why hash map is not sufficient.
>> > >
>> > > Other local storage types (task, inode, sk storage) may get a large
>> > > number of entries in a system, and thus would benefit from object
>> > > local storage. I don't think we expect too many creds in a system.
>> > > hash map of a smallish size should be good in most cases, and be
>> > > faster than cred local storage.
>> > >
>> > > Did I get this right?
>> > >
>> > > Thanks,
>> > > Song
>> > >
>> >
>> > Yes I think I addressed in the cover letter of -v2:
>> >
>> > "Like other local storage types (task, inode, sk), this provides automatic
>> > lifecycle management and is useful for LSM programs tracking credential
>> > state across LSM calls. Lifetime management is necessary for detecting
>> > credential leaks and enforcing time-based security policies."
>> >
>> > You're right it's faster and there aren't many creds, but I feel like
>> > in this case, it'll be a nightmare to manual cleanup with hashmaps. I
>> > think the correctness we get with lifetime management is worth it in
>> > this case, but could be convinced otherwise. Many cred usage patterns
>> > are short lived and a hash map could quickly become stale...
>>
>> We can clean up the hashmap in hook cred_free, no? The following
>> check in security_cred_free() seems problematic:
>>
>>         if (unlikely(cred->security == NULL))
>>                 return;
>>
>> But as far as I can tell, it is not really useful, and can be removed.
>> With this removed, hash map will work just as well. Did I miss
>> something?
>
>
> No I think actually this is easier.
>
> I will prepare a patch for the race in cleanup I stumbled on earlier which is still there and could affect other users.
>
> That said, is there any use case for local storage for these structs:
>
> - struct file
> - struct msg_msg
> - struct ipc
>
> I can off the top of my head think of some security use cases for these but not sure if hashmaps are needed, perhaps struct file

Sorry, no. This is not a copy paste territory.
The existing local storage maps were added because
performance was critical for those use cases,
but we made a few mistakes. There is a performance
cliff that has to be fixed before we adopt it to
other kernel objects.
Please use hash map and consider wrapping rhashtable
as a new bpf map type if fixed max_entries is problematic.

pw-bot: cr

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[David Windsor]

On Sun, Sep 14, 2025 at 9:10 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Sep 13, 2025 at 3:27 PM David Windsor <dwindsor@gmail.com> wrote:
> >
> >
> >
> > On Sat, Sep 13, 2025 at 5:58 PM Song Liu <song@kernel.org> wrote:
> >>
> >> On Fri, Sep 12, 2025 at 5:27 PM David Windsor <dwindsor@gmail.com> wrote:
> >> [...]
> >> > >
> >> > > Maybe I missed something, but I think you haven't addressed Alexei's
> >> > > question in v1: why this is needed and why hash map is not sufficient.
> >> > >
> >> > > Other local storage types (task, inode, sk storage) may get a large
> >> > > number of entries in a system, and thus would benefit from object
> >> > > local storage. I don't think we expect too many creds in a system.
> >> > > hash map of a smallish size should be good in most cases, and be
> >> > > faster than cred local storage.
> >> > >
> >> > > Did I get this right?
> >> > >
> >> > > Thanks,
> >> > > Song
> >> > >
> >> >
> >> > Yes I think I addressed in the cover letter of -v2:
> >> >
> >> > "Like other local storage types (task, inode, sk), this provides automatic
> >> > lifecycle management and is useful for LSM programs tracking credential
> >> > state across LSM calls. Lifetime management is necessary for detecting
> >> > credential leaks and enforcing time-based security policies."
> >> >
> >> > You're right it's faster and there aren't many creds, but I feel like
> >> > in this case, it'll be a nightmare to manual cleanup with hashmaps. I
> >> > think the correctness we get with lifetime management is worth it in
> >> > this case, but could be convinced otherwise. Many cred usage patterns
> >> > are short lived and a hash map could quickly become stale...
> >>
> >> We can clean up the hashmap in hook cred_free, no? The following
> >> check in security_cred_free() seems problematic:
> >>
> >>         if (unlikely(cred->security == NULL))
> >>                 return;
> >>
> >> But as far as I can tell, it is not really useful, and can be removed.
> >> With this removed, hash map will work just as well. Did I miss
> >> something?
> >
> >
> > No I think actually this is easier.
> >
> > I will prepare a patch for the race in cleanup I stumbled on earlier which is still there and could affect other users.
> >
> > That said, is there any use case for local storage for these structs:
> >
> > - struct file
> > - struct msg_msg
> > - struct ipc
> >
> > I can off the top of my head think of some security use cases for these but not sure if hashmaps are needed, perhaps struct file
>
> Sorry, no. This is not a copy paste territory.

no i get it's not copy/paste but I have the series for struct file
ready for submission, with selftests. this is also a performance
critical use case and there will be numerous struct file on edge
servers.

> The existing local storage maps were added because
> performance was critical for those use cases,
> but we made a few mistakes. There is a performance
> cliff that has to be fixed before we adopt it to
> other kernel objects.

ahh wasn't aware of this.

> Please use hash map and consider wrapping rhashtable
> as a new bpf map type if fixed max_entries is problematic.
>

makes sense thanks

> pw-bot: cr

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[David Windsor]

On Sun, Sep 14, 2025 at 10:10 PM David Windsor <dwindsor@gmail.com> wrote:
>
> On Sun, Sep 14, 2025 at 9:10 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Sat, Sep 13, 2025 at 3:27 PM David Windsor <dwindsor@gmail.com> wrote:
> > >
> > >
> > >
> > > On Sat, Sep 13, 2025 at 5:58 PM Song Liu <song@kernel.org> wrote:
> > >>
> > >> On Fri, Sep 12, 2025 at 5:27 PM David Windsor <dwindsor@gmail.com> wrote:
> > >> [...]
> > >> > >
> > >> > > Maybe I missed something, but I think you haven't addressed Alexei's
> > >> > > question in v1: why this is needed and why hash map is not sufficient.
> > >> > >
> > >> > > Other local storage types (task, inode, sk storage) may get a large
> > >> > > number of entries in a system, and thus would benefit from object
> > >> > > local storage. I don't think we expect too many creds in a system.
> > >> > > hash map of a smallish size should be good in most cases, and be
> > >> > > faster than cred local storage.
> > >> > >
> > >> > > Did I get this right?
> > >> > >
> > >> > > Thanks,
> > >> > > Song
> > >> > >
> > >> >
> > >> > Yes I think I addressed in the cover letter of -v2:
> > >> >
> > >> > "Like other local storage types (task, inode, sk), this provides automatic
> > >> > lifecycle management and is useful for LSM programs tracking credential
> > >> > state across LSM calls. Lifetime management is necessary for detecting
> > >> > credential leaks and enforcing time-based security policies."
> > >> >
> > >> > You're right it's faster and there aren't many creds, but I feel like
> > >> > in this case, it'll be a nightmare to manual cleanup with hashmaps. I
> > >> > think the correctness we get with lifetime management is worth it in
> > >> > this case, but could be convinced otherwise. Many cred usage patterns
> > >> > are short lived and a hash map could quickly become stale...
> > >>
> > >> We can clean up the hashmap in hook cred_free, no? The following
> > >> check in security_cred_free() seems problematic:
> > >>
> > >>         if (unlikely(cred->security == NULL))
> > >>                 return;
> > >>
> > >> But as far as I can tell, it is not really useful, and can be removed.
> > >> With this removed, hash map will work just as well. Did I miss
> > >> something?
> > >
> > >
> > > No I think actually this is easier.
> > >
> > > I will prepare a patch for the race in cleanup I stumbled on earlier which is still there and could affect other users.
> > >
> > > That said, is there any use case for local storage for these structs:
> > >
> > > - struct file
> > > - struct msg_msg
> > > - struct ipc
> > >
> > > I can off the top of my head think of some security use cases for these but not sure if hashmaps are needed, perhaps struct file
> >
> > Sorry, no. This is not a copy paste territory.
>
> no i get it's not copy/paste but I have the series for struct file
> ready for submission, with selftests. this is also a performance
> critical use case and there will be numerous struct file on edge
> servers.
>
> > The existing local storage maps were added because
> > performance was critical for those use cases,
> > but we made a few mistakes. There is a performance
> > cliff that has to be fixed before we adopt it to
> > other kernel objects.
>
> ahh wasn't aware of this.
>
> > Please use hash map and consider wrapping rhashtable
> > as a new bpf map type if fixed max_entries is problematic.
> >
>
> makes sense thanks
>

Hi,

Thinking about this more, hashmaps are still problematic for this case.

Meaning, placing a hook on security_cred_free alone for garbage
collection / end-of-life processing isn't enough - we still have to
deal with prepare/commit_creds. This flow works by having
prepare_creds clone an existing cred object, then commit_creds works
by swapping old creds with new one atomically, then later freeing the
original cred. If we are not very careful there will be a period of
time during which both cred objects could be valid, and I think this
is worth the feature alone.

Also, the main reason we want local storage for these structs is that
LSMs use them. Every classic LSM (SELinux, Smack, AppArmor, TOMOYO,
Landlock, Yama) has a cred blob, all of them have file blobs, ipc
blobs, superblock blobs, etc. struct cred is the basis for subject
identities in at least SELinux, probably others I'm sure.

With respect to performance issues, correct thing to do is still build
out these local storage types for in-kernel LSMs but then fix the
performance issue.

kp do you have any thoughts on this?

> > pw-bot: cr

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[Song Liu]

On Tue, Sep 16, 2025 at 8:25 AM David Windsor <dwindsor@gmail.com> wrote:
[...]
> >
> > makes sense thanks
> >
>
> Hi,
>
> Thinking about this more, hashmaps are still problematic for this case.
>
> Meaning, placing a hook on security_cred_free alone for garbage
> collection / end-of-life processing isn't enough - we still have to
> deal with prepare/commit_creds. This flow works by having
> prepare_creds clone an existing cred object, then commit_creds works
> by swapping old creds with new one atomically, then later freeing the
> original cred. If we are not very careful there will be a period of
> time during which both cred objects could be valid, and I think this
> is worth the feature alone.

With cred local storage, we still need to deal with prepare/commit creds,
right? cred local storage only makes sure the storage is allocated and
freed. The BPF LSM programs still need to initiate the data properly
based on the policy. IOW, whether we have cred local storage or not,
it is necessary to handle all the paths that alloc/free the cred. Did I miss
something here?

Thanks,
Song

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[David Windsor]

On Tue, Sep 16, 2025 at 12:16 PM Song Liu <song@kernel.org> wrote:
>
> On Tue, Sep 16, 2025 at 8:25 AM David Windsor <dwindsor@gmail.com> wrote:
> [...]
> > >
> > > makes sense thanks
> > >
> >
> > Hi,
> >
> > Thinking about this more, hashmaps are still problematic for this case.
> >
> > Meaning, placing a hook on security_cred_free alone for garbage
> > collection / end-of-life processing isn't enough - we still have to
> > deal with prepare/commit_creds. This flow works by having
> > prepare_creds clone an existing cred object, then commit_creds works
> > by swapping old creds with new one atomically, then later freeing the
> > original cred. If we are not very careful there will be a period of
> > time during which both cred objects could be valid, and I think this
> > is worth the feature alone.
>
> With cred local storage, we still need to deal with prepare/commit creds,
> right? cred local storage only makes sure the storage is allocated and
> freed. The BPF LSM programs still need to initiate the data properly
> based on the policy. IOW, whether we have cred local storage or not,
> it is necessary to handle all the paths that alloc/free the cred. Did I miss
> something here?
>

Yes each LSM will have to do whatever it feels it should. Some will
initialize their blob's data with one type of data, some another,
depends on the LSM's use case. We're just here to provide the storage
- bpf cannot use the "classic" LSM storage blob.

I was referring to the fact that if we use a hashmap to track state on
a per-cred basis there may be a period of time when it could be come
stale during the state change from commit -> prepare_creds.

Thanks,
David

> Thanks,
> Song

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[Song Liu]

On Tue, Sep 16, 2025 at 9:36 AM David Windsor <dwindsor@gmail.com> wrote:
>
> On Tue, Sep 16, 2025 at 12:16 PM Song Liu <song@kernel.org> wrote:
> >
> > On Tue, Sep 16, 2025 at 8:25 AM David Windsor <dwindsor@gmail.com> wrote:
> > [...]
> > > >
> > > > makes sense thanks
> > > >
> > >
> > > Hi,
> > >
> > > Thinking about this more, hashmaps are still problematic for this case.
> > >
> > > Meaning, placing a hook on security_cred_free alone for garbage
> > > collection / end-of-life processing isn't enough - we still have to
> > > deal with prepare/commit_creds. This flow works by having
> > > prepare_creds clone an existing cred object, then commit_creds works
> > > by swapping old creds with new one atomically, then later freeing the
> > > original cred. If we are not very careful there will be a period of
> > > time during which both cred objects could be valid, and I think this
> > > is worth the feature alone.
> >
> > With cred local storage, we still need to deal with prepare/commit creds,
> > right? cred local storage only makes sure the storage is allocated and
> > freed. The BPF LSM programs still need to initiate the data properly
> > based on the policy. IOW, whether we have cred local storage or not,
> > it is necessary to handle all the paths that alloc/free the cred. Did I miss
> > something here?
> >
>
> Yes each LSM will have to do whatever it feels it should. Some will
> initialize their blob's data with one type of data, some another,
> depends on the LSM's use case. We're just here to provide the storage
> - bpf cannot use the "classic" LSM storage blob.
>
> I was referring to the fact that if we use a hashmap to track state on
> a per-cred basis there may be a period of time when it could be come
> stale during the state change from commit -> prepare_creds.

I still don't see how cred local storage will make a difference here. If the
cred is stale, the data attached to it is also stale. As long as we free the
attached data together with the cred, it should just work, no?

Song

Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

[Song Liu]

On Tue, Sep 16, 2025 at 11:49 AM David Windsor <dwindsor@gmail.com> wrote:
>
>
>
> On Tue, Sep 16, 2025 at 1:47 PM Song Liu <song@kernel.org> wrote:
> >
> > On Tue, Sep 16, 2025 at 9:36 AM David Windsor <dwindsor@gmail.com> wrote:
> > >
> > > On Tue, Sep 16, 2025 at 12:16 PM Song Liu <song@kernel.org> wrote:
> > > >
> > > > On Tue, Sep 16, 2025 at 8:25 AM David Windsor <dwindsor@gmail.com> wrote:
> > > > [...]
> > > > > >
> > > > > > makes sense thanks
> > > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > Thinking about this more, hashmaps are still problematic for this case.
> > > > >
> > > > > Meaning, placing a hook on security_cred_free alone for garbage
> > > > > collection / end-of-life processing isn't enough - we still have to
> > > > > deal with prepare/commit_creds. This flow works by having
> > > > > prepare_creds clone an existing cred object, then commit_creds works
> > > > > by swapping old creds with new one atomically, then later freeing the
> > > > > original cred. If we are not very careful there will be a period of
> > > > > time during which both cred objects could be valid, and I think this
> > > > > is worth the feature alone.
> > > >
> > > > With cred local storage, we still need to deal with prepare/commit creds,
> > > > right? cred local storage only makes sure the storage is allocated and
> > > > freed. The BPF LSM programs still need to initiate the data properly
> > > > based on the policy. IOW, whether we have cred local storage or not,
> > > > it is necessary to handle all the paths that alloc/free the cred. Did I miss
> > > > something here?
> > > >
> > >
> > > Yes each LSM will have to do whatever it feels it should. Some will
> > > initialize their blob's data with one type of data, some another,
> > > depends on the LSM's use case. We're just here to provide the storage
> > > - bpf cannot use the "classic" LSM storage blob.
> > >
> > > I was referring to the fact that if we use a hashmap to track state on
> > > a per-cred basis there may be a period of time when it could be come
> > > stale during the state change from commit -> prepare_creds.
> >
> > I still don't see how cred local storage will make a difference here. If the
> > cred is stale, the data attached to it is also stale. As long as we free the
> > attached data together with the cred, it should just work, no?
> >
>
> If we use local storage, the cred being stale will still be possible but its attached object will at least be consistent.
>
> In this case, a cred object has been replaced with a new instance, but under RCU readers may still legally see the old pointer for some time.
>
> If we're tracking state in a side map keyed by the pointer and you’ve already removed the old entry (after copying state to the new one), those readers will get a miss even though the old object is still valid:
>
> CPU0 (commit_creds)        CPU1 (BPF hook)         Hashmap
> -------------------------  ----------------------  ----------------------------
> task->cred = old_pointer                           entry[old_pointer] = blob_old
>
> rcu_assign_pointer(new_pointer)
>
>                            still sees old_pointer
>                            lookup(old_pointer) -> blob_old
>
> map_update(new_pointer, blob_old)                  entry[new_pointer] = blob_old
> map_delete(old_pointer)                            del entry[old_pointer]
>
>                            still sees old_pointer
>                            lookup(old_pointer) -> NULL (this is a failure)
>

This is a problem because we are calling map_delete(old_pointer)
at commit_creds time. If we instead calls map_delete(old_pointer)
at security_cred_free, it will be the same as cred local storage.

Does this make sense?

Thanks,
Song