linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: liqiong <liqiong@nfschina.com>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: THOBY Simon <Simon.THOBY@viveris.fr>,
	"dmitry.kasatkin@gmail.com" <dmitry.kasatkin@gmail.com>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"serge@hallyn.com" <serge@hallyn.com>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ima: fix deadlock within RCU list of ima_rules
Date: Wed, 25 Aug 2021 19:45:33 +0800	[thread overview]
Message-ID: <5a032a1b-f763-a0e4-8ea2-803872bd7174@nfschina.com> (raw)
In-Reply-To: <d502623a-7a49-04f8-1672-6521ceef260b@nfschina.com>

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
>>
>>
>


  reply	other threads:[~2021-08-25 11:46 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 10:15 [PATCH] ima: fix infinite loop within "ima_match_policy" function liqiong
2021-08-19 12:58 ` THOBY Simon
2021-08-19 13:47   ` Mimi Zohar
2021-08-19 19:31     ` Mimi Zohar
2021-08-20 10:15   ` 李力琼
2021-08-20 13:23     ` THOBY Simon
2021-08-20 15:48       ` Mimi Zohar
2021-08-23  3:04         ` 李力琼
2021-08-23  7:51           ` 李力琼
2021-08-23  8:06           ` liqiong
2021-08-23  8:14             ` THOBY Simon
2021-08-23 11:57               ` Mimi Zohar
2021-08-23 12:02                 ` THOBY Simon
2021-08-23 12:09                   ` Mimi Zohar
2021-08-23 12:56               ` liqiong
2021-08-23 11:22           ` Mimi Zohar
2021-08-20 17:53       ` liqiong
2021-08-23  7:13         ` THOBY Simon
2021-08-24  8:57 ` [PATCH] ima: fix deadlock " liqiong
2021-08-24  9:50   ` THOBY Simon
2021-08-24 12:09     ` liqiong
2021-08-24 12:38       ` Mimi Zohar
2021-08-25  7:05         ` [PATCH] ima: fix deadlock within RCU list of ima_rules liqiong
2021-08-25 11:45           ` liqiong [this message]
2021-08-25 12:03             ` THOBY Simon
2021-08-26  8:15               ` liqiong
2021-08-26  9:01                 ` THOBY Simon
2021-08-27  6:41                   ` liqiong
2021-08-27  7:30                     ` THOBY Simon
2021-08-27  9:10                       ` liqiong
2021-08-27  9:20                         ` THOBY Simon
2021-08-27 10:35   ` [PATCH] ima: fix deadlock when traversing "ima_default_rules" liqiong
2021-08-27 16:16     ` Mimi Zohar
2021-09-18  3:11     ` liqiong
2021-09-30 19:46       ` Mimi Zohar
2021-10-09 10:38       ` liqiong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5a032a1b-f763-a0e4-8ea2-803872bd7174@nfschina.com \
    --to=liqiong@nfschina.com \
    --cc=Simon.THOBY@viveris.fr \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=jmorris@namei.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).