Re: [GIT PULL] selinux/selinux-pr-20250323

[Stephen Smalley]

On Thu, Mar 27, 2025 at 11:50 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, 27 Mar 2025 at 01:59, Jeffrey Vander Stoep <jeffv@xxxxxxxxxx> wrote:
> >
> > The value here isn't so much about checking the source context
> > "kernel", but rather about checking the target context and enforcing
> > that firmware can only come from trusted filesystems. So even a
> > compromised privileged process that sets firmware_class.path cannot
> > cause the kernel to load firmware from an arbitrary source.
>
> Yes, and that's literally why I earlier in the thread pointed out the
> new code in selinux_kernel_load_data()
>
>   "I'm looking at selinux_kernel_load_data() in particular, where you
>    don't even pass it a file at all, so it's not like it could check for
>    "is this file integrity-protected" or anything like that"
>
> because I understand that you might want to verify the *file* the
> firmware comes from, but I think verifying the context in which the
> firmware is loaded is absolutely insane and incorrect.

So the only use case I could see for that particular check would be if
we wanted to block loading firmware directly from memory/blobs rather
than from files. If that's not a valid use case, then we can get rid
of that particular check if desired; it just seemed inconsistent
between the two hooks otherwise. What's the purpose of having the
LOADING_FIRMWARE enum or hook call on that code path at all then?

> And that is literally *all* that the new case in
> selinux_kernel_load_data() does. There is no excuse for that craziness
> that I can come up with.
>
> And yes, I'm harping on this, because I really *hate* how the security
> layer comes up in my performance profiles so much. It's truly
> disgusting. So when I see new hooks that don't make sense to me, I
> react *very* strongly.

If you have constructive suggestions (or patches!) to improve
performance of LSM and/or SELinux, we'd be glad to take them. Or even
helpful hints on how to best measure and see the same overheads you
are seeing and where.

>
> Do I believe this insanity matters for performance? No.
>
> But do I believe that the security code needs to *think* about the
> random hooks it adds more? Yes. YES!
>
> Which is why I really hate seeing new random hooks where I then go
> "that is complete and utter nonsense".
>
> [ This whole patch triggered me for another reason too - firmware
> loading in particular has a history of user space actively and
> maliciously screwing the kernel up.
>
>   The reason we load firmware directly from the kernel is because user
> space "policy" decisions actively broke our original "let user space
> do it" model.
>
>   So if somebody thinks I'm overreacting, they are probably right, but
> dammit, this triggers two of my big red flags for "this is horribly
> wrong" ]
>
>                 Linus