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

[Linus Torvalds]

On Fri, 28 Mar 2025 at 06:23, Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> Yes, thank you. I think it would be easy enough to make that change to
> selinux_inode_permission() and to clear that inode flag on file
> relabels (e.g. in selinux_inode_post_setxattr() and
> inode_invalidate_secctx()).

So the thing that *really* made me go "I don't know how to do this in
the security layer" is not so much the relabeling - that should be
easy to handle by just clearing the bit, as you say.

And I wasn't even so much worried about policy changes that would
globally change meaning of existing labels:

> Not as sure about handling policy reloads
> / boolean changes at runtime without also caching the policy sequence
> number in the inode too so that can be compared.

Yeah, a sequence number seems like an obvious solution, even if it
might be a bit awkward to find a place to store it that doesn't
pollute the cache. The reason it would be _so_ nice to not call the
security hooks at all in this path is that I think we otherwise can do
all the inode security lookups by just looking at the very beginning
of the inode (that's why we do that IOP_FASTPERM thing - just to avoid
touching other cachelines). But if it avoids the
security_inode_permission() call, it would definitely be a win even
with a cache miss.

> Further, I'm unclear
> on how to implement this in a manner that works with the LSM stacking
> support,

So *this* was what made me go "I don't know how to to this AT ALL",
along with the fact that the rule for the bit would have to be that it
would be true for *all* execution contexts.

IOW, it's a very different thing from the usual security hook calls,
in that instead of saying "is this access ok for the current context",
the bit setting would have to say "this lookup is ok for _all_ calling
contexts for this inode for _all_ of the nested security things".

The sequence number approach should take care of any races, so that
part isn't a huge problem: just set the inode sequence number early,
before doing any of the checks. And then only set the bit at the end
if every stacked security layer says "yeah, this inode doesn't have
extra lookup rules as far as I'm concerned". So if any of the rules
changed in the meantime, the sequence number means that the bit won't
take effect. So that part should be fine.

But the "this inode doesn't have extra lookup rules" part is what
needs low-level knowledge about how all the security models work. And
it really would have to be true in all contexts - ie across different
namespaces etc.

(Note the "extra" part: the VFS layer deals with all the *normal* Unix
rules, including ACL, of course. So it's literally just about "are
there security hook rules that then limit things _beyond_ those
standard permission rules")

It might be worth noting that this call site is special for the VFS
anyway: it *looks* like a normal security hook, but "may_lookup()" is
literally *only* used for directories, and *only* used for "can I do
name lookup in this".

So if it helps the security layers, that case could be made into its
own specialized hook entirely, even if it would require basically
duplicating up "inode_permission()" that is currently used for both
the lookup case and for "generic" inode permission checking.

For example, I do know that SElinux turns the VFS layer permission
mask (it things like "MAY_EXEC") into its own masks that are different
for files and for directories (so MAY_EXEC becomes DIR__SEARCH for
SElinux for directories, but FILE__EXECUTE for when doing 'execve()'
on a file).

And so that's an example of something we could short-circuit here,
because *all* we care about is that DIR__SEARCH thing, and we know
that statically.

But I know selinux better than any other of the security models, so I
don't know what *any* of the others do (and honestly, the selinux code
I don't know that well - I've looked at it a lot, but I've really only
ever looked at it in the "I'm looking at profiles, is there anything
obvious I can do about this" sense).

                 Linus