[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAPhsuW44HznMHFZdaxCcdsVrYuYhJOQAPEjETxhm-j_fk18QUw@mail.gmail.com>
Date: Sat, 13 Sep 2025 14:58:39 -0700
From: Song Liu <song@...nel.org>
To: David Windsor <dwindsor@...il.com>
Cc: bpf@...r.kernel.org, linux-kernel@...r.kernel.org, martin.lau@...ux.dev,
ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, eddyz87@...il.com,
yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org
Subject: Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs
On Fri, Sep 12, 2025 at 5:27 PM David Windsor <dwindsor@...il.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
Powered by blists - more mailing lists