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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEjxPJ6XnBmbzH44YVQxxv8WOyPN7N81fpj7OYonEOTB=rn6wg@mail.gmail.com>
Date: Thu, 27 Mar 2025 12:55:27 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Jeffrey Vander Stoep <jeffv@...gle.com>, Thiébaud Weksteen <tweek@...gle.com>, 
	Paul Moore <paul@...l-moore.com>, "Cameron K. Williams" <ckwilliams.work@...il.com>, 
	"Kipp N. Davis" <kippndavis.work@....com>, selinux@...r.kernel.org, 
	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Nick Kralevich <nnk@...gle.com>, Kees Cook <kees@...nel.org>
Subject: Re: [GIT PULL] selinux/selinux-pr-20250323

On Thu, Mar 27, 2025 at 11:50 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Thu, 27 Mar 2025 at 01:59, Jeffrey Vander Stoep <jeffv@...gle.com> 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