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

[Casey Schaufler <casey@schaufler-ca.com>]

On 3/28/2025 9:36 AM, Linus Torvalds wrote:
> On Fri, 28 Mar 2025 at 06:23, Stephen Smalley
> <stephen.smalley.work@gmail.com> 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".

That is correct. But the LSMs we currently have don't do frequent
attribute changes on objects and generally constrain process attribute
changes to exec() and administrative actions. Invalidating the lookup ok
flag when any LSM changes attributes is most likely to occur at a time
when all LSMs could be expected to change attributes.

> 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.

It would be nice if changing the rules for one LSM (e.g. adding a
Smack access rule: # echo Snap Pop rw > /sys/fs/smackfs/load2) didn't
affect the lookup ok flag for other LSMs, but I don't see how the
overhead would be justified. I'm also concerned about how detecting
and clearing the flag on a single LSM case on policy change is going
to be anything but brutal.