On Fri, Sep 12, 2025 at 5:27 PM David Windsor <dwindsor@xxxxxxxxx> 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
