[PATCH v13 3/5] security: Replace indirect LSM hook calls with static calls

[KP Singh]

On Mon, Jul 8, 2024 at 2:52 PM Paul Moore <paul at paul-moore.com> wrote:
>
> On Mon, Jul 8, 2024 at 6:04 AM KP Singh <kpsingh at kernel.org> wrote:
> > On Sat, Jul 6, 2024 at 6:40 AM Paul Moore <paul at paul-moore.com> wrote:
> > > On Fri, Jul 5, 2024 at 3:34 PM KP Singh <kpsingh at kernel.org> wrote:
> > > > On Fri, Jul 5, 2024 at 8:07 PM Paul Moore <paul at paul-moore.com> wrote:
> > > > > On Wed, Jul 3, 2024 at 7:08 PM KP Singh <kpsingh at kernel.org> wrote:
> >
> > [...]
> >
> > > >
> > > > Paul, I am talking about eliminating a class of bugs, but you don't
> > > > seem to get the point and you are fixated on the very instance of this
> > > > bug class.
> > >
> > > I do understand that you are trying to eliminate a class of bugs, the
> > > point I'm trying to make is that I believe we have addressed that
> > > already with the patches I've previously cited.
> >
> > The class I am referring to is useless hooks returning a default value
> > and imposing a denial / enforcement when they are not supposed to.
>
> If a LSM hook's default value were to result in an undesirable
> behavior within the kernel that would be an issue regardless of what
> LSMs were involved and we would either need to modify the hook and/or
> the default value.  I am not convinced that we can adequately solve
> this entire class of problems simply by allowing one LSM, or even
> arbitrary combinations of LSMs, to disable or otherwise disconnect
> themselves from the framework.
>
> > > As the BPF maintainer you are always free to do whatever you like
> > > within the scope of the LSM you maintain so long as it does not touch
> > > or otherwise impact any of the other LSMs or the LSM framework.  If
> > > you do affect the other LSMs, or the LSM framework, you need to get an
> > > ACK from the associated maintainer.  That's pretty much how Linux
> > > kernel development works.
> >
> > Okay, then let's not make an LSM API, I will handle it within the BPF LSM.
> >
> > The patch I proposed should not affect any other LSMs and is self
> > contained within BPF LSM:
> >
> > https://lore.kernel.org/bpf/CACYkzJ6jADoGNuPP3-1wkk-kV7NOQh+eFkU5KEDEZgq9qNNEfg@mail.gmail.com/
>
> The code changes may be self-contained within the BPF LSM, but it does
> appear that the bpf_lsm_toggle_hook() function directly manipulates
> the LSM framework hook state which is not something we want to allow -
> none of the individual LSMs should be directly manipulating the LSM
> hook state/configuration.  Although perhaps I'm missing or not
> factoring in some context around the patch linked above and that isn't
> the case?

I think you are ignoring my point that BPF does not want to add
extraneous function calls which at the least result in extra overhead.

 You have ignored the fact that BPF LSM never wanted these empty
callbacks and you still continue to ignore it. Sigh, I will drop it
now and will propose it as a separate patch so that we can at least
unblock the static call series.

- KP

> While I had issues with Kees' comments, at a high level his suggestion
> of dropping the last patch and moving forward is something you should
> consider as I don't see this a good path forward with all of the
> approaches that have been discussed thus far.
>
> --
> paul-moore.com