Re: [RFC PATCH] selinux: uninline unlikely parts of avc_has_perm_noaudit()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 7, 2023 at 3:45 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> This is based on earlier patch posted to the list by Linus, his
> commit description read:
>
>  "avc_has_perm_noaudit()is one of those hot functions that end up
>   being used by almost all filesystem operations (through
>   "avc_has_perm()") and it's intended to be cheap enough to inline.
>
>   However, it turns out that the unlikely parts of it (where it
>   doesn't find an existing avc node) need a fair amount of stack
>   space for the automatic replacement node, so if it were to be
>   inlined (at least clang does not) it would just use stack space
>   unnecessarily.
>
>   So split the unlikely part out of it, and mark that part noinline.
>   That improves the actual likely part."
>
> The basic idea behind the patch was reasonable, but there were minor
> nits (double indenting, etc.) and the RCU read lock unlock/re-lock in
> avc_compute_av() began to look even more ugly.  This patch builds on
> Linus' first effort by cleaning things up a bit and removing the RCU
> unlock/lock dance in avc_compute_av().
>
> Removing the RCU lock dance in avc_compute_av() is safe as there are
> currently two callers of avc_compute_av(): avc_has_perm_noaudit() and
> avc_has_extended_perms().  The first caller in avc_has_perm_noaudit()
> does not require a RCU lock as there is no avc_node to protect so the
> RCU lock can be dropped before calling avc_compute_av().  The second
> caller, avc_has_extended_perms(), is similar in that there is no
> avc_node that requires RCU protection, but the code is simplified by
> holding the RCU look around the avc_compute_av() call, and given that
> we enter a RCU critical section in security_compute_av() (called from
> av_compute_av()) the impact will likely be unnoticeable.  It is also
> worth noting that avc_has_extended_perms() is only called from the
> SELinux ioctl() access control hook at the moment.
>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> ---
>  security/selinux/avc.c | 85 ++++++++++++++++++++++++++++--------------
>  1 file changed, 57 insertions(+), 28 deletions(-)

FYI, marked as a RFC as I haven't tested this yet but I felt this was
worth posting early as Linus' original patch was sent as an attachment
and some of you may have missed it as a result.

https://lore.kernel.org/selinux/CAHk-=whR4KSGqfEodXMwOMdQBt+V2HHMyz6+CobiydnZE+Vq9Q@xxxxxxxxxxxxxx/

-- 
paul-moore.com




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux