On 5/30/19 9:27 AM, Janne Karhunen wrote:
On Thu, May 30, 2019 at 3:08 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
@@ -269,11 +269,23 @@ static void ima_lsm_update_rules(void)
Audit_equal,
entry->lsm[i].args_p,
&entry->lsm[i].rule);
- BUG_ON(!entry->lsm[i].rule);
+ if (result == -EINVAL)
+ pr_warn("ima: rule for LSM \'%d\' is invalid\n",
+ entry->lsm[i].type);
I could be wrong, but I think there is still a problem here in that you
are modifying entry->lsm[i].rule in-place, but it is protected under RCU
and therefore needs to be duplicated and then modified? Also you are
leaking the old rule?
Right. Bit too fast tapping the keyboard without thinking, will fix
and post in the proper form. But I guess the original point was to
verify if that 'notifier_block' is indeed the right way to get the
update notification?
Yes.
Both of those issues also exist prior to your
patch but you aren't fixing them here. And lastly, it looks like lsm
notifiers are atomic notifiers (not clear to me why) so you can't block
in the callback, thereby requiring scheduling the work as is done in
infiniband.
Great catch, thank you. That's an easy fix if no-one objects pushing
these through the system-wq for example.
I think you can switch the lsm notifier over to using blocking notifiers
instead; there seems to be no valid reason for making it atomic.
