sleep in selinux_audit_rule_init

[Janne Karhunen]

On Wed, May 22, 2019 at 3:20 PM Stephen Smalley <sds at tycho.nsa.gov> wrote:

> > I managed to hit a following BUG, looks like ima can call
> > selinux_audit_rule_init that can sleep in rcu critical section in
> > ima_match_policy():
> >
> > __might_sleep
> > kmem_cache_alloc_trace
> > selinux_audit_rule_init <<< kzalloc (.. GFP_KERNEL)
> > security_audit_rule_init
> > ima_match_policy <<< list_for_each_entry_rcu
> > ima_get_action
> > process_measurement
> > ima_file_check
> > path_openat
> > do_filp_open
> > ..
> >
> > I guess this is the ima_match_rules() calling ima_lsm_update_rules()
> > when it concludes that the selinux policy may have been reloaded.
> >
> > The easy way for me to fix my own butt in this regard is to change the
> > selinux allocation not to wait, but Paul would you be OK with such
> > change? The alternative looks like a pretty big change in the ima?
>
> This is perhaps a sign of a deeper bug in IMA; if they are in the middle
> of matching against their policy rules, then they shouldn't be
> updating/modifying those rules in the middle of match processing?  How
> is that safe under RCU?

Heh indeed...


> If you look at how the audit subsystem deals with the same problem, they
> have a callback (audit_update_lsm_rules) that is called upon an AVC
> reset (hence upon a policy reload) and can update all of their rules at
> that time, not lazily during matching.  Since that time, a more general
> notifier mechanism was added, register_lsm_notifier(), and is used by
> infiniband to update its state upon policy changes.

I guess the same approach could work here. I'll see how that would
look like exactly..


--
Janne