Re: [RFC bpf-next] lsm: bpf: Remove lsm_prop_bpf

On Tue, Oct 28, 2025 at 4:54 AM Matt Bobrowski <[email protected]> wrote:
> On Mon, Oct 27, 2025 at 09:50:11PM -0400, Paul Moore wrote:
> > On Mon, Oct 27, 2025 at 6:45 PM Song Liu <[email protected]> wrote:
> > > On Mon, Oct 27, 2025 at 2:14 PM Paul Moore <[email protected]> wrote:
> > > > On Fri, Oct 24, 2025 at 8:10 PM Song Liu <[email protected]> wrote:
> > > > >
> > > > > lsm_prop_bpf is not used in any code. Remove it.
> > > > >
> > > > > Signed-off-by: Song Liu <[email protected]>
> > > > >
> > > > > ---
> > > > >
> > > > > Or did I miss any user of it?
> > > > > ---
> > > > >  include/linux/lsm/bpf.h  | 16 ----------------
> > > > >  include/linux/security.h |  2 --
> > > > >  2 files changed, 18 deletions(-)
> > > > >  delete mode 100644 include/linux/lsm/bpf.h
> > > >
> > > > You probably didn't miss any direct reference to lsm_prop_bpf, but the
> > > > data type you really should look for when deciding on this is
> > > > lsm_prop.  There are a number of LSM hooks that operate on a lsm_prop
> > > > struct instead of secid tokens, and without a lsm_prop_bpf
> > > > struct/field in the lsm_prop struct a BPF LSM will be limited compared
> > > > to other LSMs.  Perhaps that limitation is okay, but it is something
> > >
> > > I think audit is the only user of lsm_prop (via audit_names and
> > > audit_context). For BPF based LSM or audit, I don't think we need
> > > specific lsm_prop. If anything is needed, we can implement it with
> > > task local storage or inode local storage.
> > >
> > > CC audit@ and Eric Paris for more comments on audit side.
> >
> > You might not want to wait on a comment from Eric :)
> >
> > > > that should be discussed; I see you've added KP to the To/CC line, I
> > > > would want to see an ACK from him before I merge anything removing
> > > > lsm_prop_bpf.
> > >
> > > Matt Bobrowski is the co-maintainer of BPF LSM. I think we are OK
> > > with his Reviewed-by?
> >
> > Good to know, I wasn't aware that Matt was also listed as a maintainer
> > for the BPF LSM.  In that case as long as there is an ACK, not just a
> > reviewed tag, I think that should be sufficient.
>
> ACK.
>
> > > > I haven't checked to see if the LSM hooks associated with a lsm_prop
> > > > struct are currently allowed for a BPF LSM, but I would expect a patch
> > > > removing the lsm_prop_bpf struct/field to also disable those LSM hooks
> > > > for BPF LSM use.
> > >
> > > I don't think we need to disable anything here. When lsm_prop was
> > > first introduced in [1], nothing was added to handle BPF.
> >
> > If the BPF LSM isn't going to maintain any state in the lsm_prop
> > struct, I'd rather see the associated LSM interfaces disabled from
> > being used in a BPF LSM just so we don't run into odd expectations in
> > the future.  Maybe they are already disabled, I haven't checked.
>
> Well, it doesn't ATM, but nothing goes to say that this will change in
> the future. Until then though, I have no objections around removing
> lsm_prop_bpf from lsm_prop as there's currently no infrastructure in
> place allowing a BPF LSM to properly harness lsm_prop/lsm_prop_bpf. By
> harness, I mean literaly using lsm_prop/lsm_prop_bpf as some form of
> context storage mechanism.
>
> As for the disablement of the associated interfaces, I don't feel like
> this warranted at this point? Doing so might break some out-of-tree
> BPF LSM implementations, specifically those that might be using these
> associated LSM interfaces purely for instrumentation purposes at this
> point?

Okay, let's leave things as-is for right now.  The lsm_prop struct is
an important part of those APIs, and if there is a need for those APIs
in a BPF LSM then we should preserve all of the API, including the
lsm_prop component.

-- 
paul-moore.com