Thread (31 messages) 31 messages, 4 authors, 2024-07-09

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

From: KP Singh <kpsingh@kernel.org>
Date: 2024-07-08 13:52:13
Also in: bpf

On Mon, Jul 8, 2024 at 2:52 PM Paul Moore [off-list ref] wrote:
On Mon, Jul 8, 2024 at 6:04 AM KP Singh [off-list ref] wrote:
quoted
On Sat, Jul 6, 2024 at 6:40 AM Paul Moore [off-list ref] wrote:
quoted
On Fri, Jul 5, 2024 at 3:34 PM KP Singh [off-list ref] wrote:
quoted
On Fri, Jul 5, 2024 at 8:07 PM Paul Moore [off-list ref] wrote:
quoted
On Wed, Jul 3, 2024 at 7:08 PM KP Singh [off-list ref] wrote:
[...]
quoted
quoted
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.
quoted
quoted
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/ (local)
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help