On 5/12/2021 9:44 AM, Ondrej Mosnacek wrote:

On Wed, May 12, 2021 at 6:18 PM Casey Schaufler [off-list ref] wrote:

quoted

On 5/12/2021 6:21 AM, Ondrej Mosnacek wrote:

quoted

On Sat, May 8, 2021 at 12:17 AM Casey Schaufler [off-list ref] wrote:

quoted

On 5/7/2021 4:40 AM, Ondrej Mosnacek wrote:

quoted

Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
lockdown") added an implementation of the locked_down LSM hook to
SELinux, with the aim to restrict which domains are allowed to perform
operations that would breach lockdown.

However, in several places the security_locked_down() hook is called in
situations where the current task isn't doing any action that would
directly breach lockdown, leading to SELinux checks that are basically
bogus.

Since in most of these situations converting the callers such that
security_locked_down() is called in a context where the current task
would be meaningful for SELinux is impossible or very non-trivial (and
could lead to TOCTOU issues for the classic Lockdown LSM
implementation), fix this by adding a separate hook
security_locked_down_globally()

This is a poor solution to the stated problem. Rather than adding
a new hook you should add the task as a parameter to the existing hook
and let the security modules do as they will based on its value.
If the caller does not have an appropriate task it should pass NULL.
The lockdown LSM can ignore the task value and SELinux can make its
own decision based on the task value passed.

The problem with that approach is that all callers would then need to
be updated and I intended to keep the patch small as I'd like it to go
to stable kernels as well.

But it does seem to be a better long-term solution - would it work for
you (and whichever maintainer would be taking the patch(es)) if I just
added another patch that refactors it to use the task parameter?

I can't figure out what you're suggesting. Are you saying that you
want to add a new hook *and* add the task parameter?

No, just to keep this patch as-is (and let it go to stable in this
form) and post another (non-stable) patch on top of it that undoes the
new hook and re-implements the fix using your suggestion. (Yeah, it'll
look weird, but I'm not sure how better to handle such situation - I'm
open to doing it whatever different way the maintainers prefer.)

James gets to make the call on this one. If it was my call I would
tell you to make the task parameter change and accept the backport
pain. I think that as a security developer community we spend way too
much time and effort trying to avoid being noticed in source trees.