Re: [PATCH v2 1/2] selinux: switch unnecessary GFP_ATOMIC allocs to GFP_KERNEL

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

 



On Wed, Aug 26, 2020 at 10:12 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> On Wed, Aug 26, 2020 at 3:51 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > On Wed, Aug 26, 2020 at 9:02 AM Stephen Smalley
> > <stephen.smalley.work@xxxxxxxxx> wrote:
> > > Generally the core SELinux permission checking code assumes it can be
> > > called from any context (including hardirq) and under any locking
> > > conditions, and hooks that are for permission checking (not security
> > > blob allocation/management) do the same.  This allows permission
> > > checks to be performed while locks are held by the caller. and from
> > > arbitrary contexts  I'd be inclined to just leave selinux_lockdown
> > > unchanged given that it might be called anywhere much like capable().
> >
> > Agreed.  The code paths relating to policy load, etc. are good
> > candidates for an ATOMIC->KERNEL conversion, assuming no other
> > constraints, but I'm somewhat nervous of converting stuff in hook.c
> > that doesn't have a hard sleep-ability defined.
>
> OK, I can drop the selinux_lockdown() hunk then. The other hooks.c
> hunks both have an existing GFP_KERNEL allocation in the same
> function, so I'd tend to keep them for consistency. Or do you want me
> to rather convert the GFP_KERNELs to GFP_ATOMICs there?

I was speaking generically in that I'm not comfortable converting
ATOMIC allocations to KERNEL allocations in the hooks.c layer without
some guarantee/constraint.  In the case of both _setxattr and
_setprocattr we are already doing KERNEL allocations in those
functions so switching the audit allocations to KERNEL isn't
necessarily changing the behavior of the SELinux hook, it can sleep
now regardless.

> > The other question about this patchset is motivation: were you seeing
> > problem reports relating to SELinux memory failures when the system
> > was under pressure, or is this preemptive?
>
> No, I just went over the GFP_* usage in security/selinux/ when scoping
> how many GFP_ATOMIC ones could be changed to GFP_KERNEL after the
> refcount patch and found these that could be switched right away.

Okay, thanks.

-- 
paul moore
www.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