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

[Paul Moore]

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?

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