On Fri, Sep 12, 2025 at 3:25 PM David Windsor <dwindsor@xxxxxxxxx> 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@xxxxxxxxx> 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); [...]
