Re: [PATCH] ima: fix deadlock within RCU list of ima_rules

liqiong
2021-08-25T11:46:02Z

Hi Mimi,

This copy may be better.

subject: ima: fix deadlock when iterating over the init "ima_rules" list.

When traversing back to head, the init "ima_rules" list can't exit
iterating if "ima_rules" has been updated to "ima_policy_rules".
It causes soft lockup and RCU stalls. So we can introduce a duplicate
of "ima_rules" for each "ima_rules" list loop.

Signed-off-by: liqiong <liqiong@nfschina.com>
---
This problem can happen in practice: updating the IMA policy
in the boot process while systemd-services are being checked.

security/integrity/ima/ima_policy.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fd5d46e511f1..e92b197bfd3c 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

Regards,

liqiong

在 2021年08月25日 15:05, liqiong 写道:
> Hi Mimi,
>
> Thanks for the advice，maybe i should trim the message,
> here is a new copy:
>
>
> subject: ima: fix deadlock when iterating over the init "ima_rules" list.
>
> The init "ima_rules" list can't traverse back to head, if "ima_rules"
> is being updated to "ima_policy_rules". It causes soft lockup and RCU stalls.
> So we can introduce a duplicate of "ima_rules" for each "ima_rules" list loop.
>
> Signed-off-by: liqiong <liqiong@nfschina.com>
> ---
> This problem can happen in practice: updating the IMA policy
> in the boot process while systemd-services are being checked.
>
> security/integrity/ima/ima_policy.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index fd5d46e511f1..e92b197bfd3c 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
>
>
> Regards,
>
> liqiong
>
> 在 2021年08月24日 20:38, Mimi Zohar 写道:
>> On Tue, 2021-08-24 at 20:09 +0800, liqiong wrote:
>>> Hi Simon :
>>>
>>> ima: fix deadlock within RCU list of ima_rules.
>>>
>> Before the following paragraph, an introductory sentence is needed.
>> Try adding a sentence to the affect that "ima_rules" initially points
>> to the "ima_default_rules", but after loading a custom policy points to
>> the "ima_policy_rules". Then describe the bug at a high level,
>> something like - transitioning to the "ima_policy_rules" isn't being
>> done safely.
>>
>> Followed by the details.
>>
>>> ima_match_policy() is looping on the policy ruleset while
>>> ima_update_policy() updates the variable "ima_rules". This can
>>> lead to a situation where ima_match_policy() can't exit the
>>> 'list_for_each_entry_rcu' loop, causing RCU stalls
>>> ("rcu_sched detected stall on CPU ...").
>>>
>>> This problem can happen in practice: updating the IMA policy
>>> in the boot process while systemd-services are being checked.
>>>
>>> In addition to ima_match_policy(), other function with
>>> "list_for_each_entry_rcu" should happen too. Fix locking by
>>> introducing a duplicate of "ima_rules" for each
>>> "list_for_each_entry_rcu".
>>>
>>>
>>> How about this commit message ?
>>>
>>> I have tested this patch in lab, we can reproduced this error case,
>>> have done reboot test many times. This patch should work.
>> The above comment doesn't belong in the commit message, but is a
>> message to the reviewers/maintainers and goes after the patch
>> descriptions three dashes line.
>>
>> thanks,
>>
>> Mimi
>>
>>
>