On 5/22/19 8:20 AM, Stephen Smalley wrote:
On 5/22/19 7:49 AM, Janne Karhunen wrote:
Hi,
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?
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.
Another potentially worrisome aspect of the current
ima_lsm_update_rules() logic is that it does a BUG_ON() if the attempt
to update the rule fails, which could occur if e.g. one had an IMA
policy rule based on a given domain/type and that domain/type were
removed from policy (e.g. via policy module removal). Contrast with the
handling in audit_dupe_lsm_field(). The existing ima_lsm_update_rules()
logic could also yield a BUG_ON upon transient memory allocation failure.
