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