linux-kernel - Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAPhsuW5dEqMBPxvh03dc=K6az0Z6TP-aXCpiowLoTxDHxCvTsw@mail.gmail.com>
Date: Tue, 16 Sep 2025 12:36:51 -0700
From: Song Liu <song@...nel.org>
To: David Windsor <dwindsor@...il.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>, bpf <bpf@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Eduard <eddyz87@...il.com>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>
Subject: Re: [PATCH 1/2] bpf: Add BPF_MAP_TYPE_CRED_STORAGE map type and kfuncs

On Tue, Sep 16, 2025 at 11:49 AM David Windsor <dwindsor@...il.com> wrote:
>
>
>
> On Tue, Sep 16, 2025 at 1:47 PM Song Liu <song@...nel.org> wrote:
> >
> > On Tue, Sep 16, 2025 at 9:36 AM David Windsor <dwindsor@...il.com> wrote:
> > >
> > > On Tue, Sep 16, 2025 at 12:16 PM Song Liu <song@...nel.org> wrote:
> > > >
> > > > On Tue, Sep 16, 2025 at 8:25 AM David Windsor <dwindsor@...il.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