[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-integrity
Subject:    Re: [PATCH] ima: Handle -ESTALE returned by ima_filter_rule_match()
From:       "Guozihua (Scott)" <guozihua () huawei ! com>
Date:       2022-08-30 12:13:59
Message-ID: d63be10a-b650-49a4-4a5d-809681fb8581 () huawei ! com
[Download RAW message or body]

On 2022/8/30 20:03, Mimi Zohar wrote:
> On Tue, 2022-08-30 at 16:41 +0800, Guozihua (Scott) wrote:
>> On 2022/8/30 9:20, Mimi Zohar wrote:
>>> On Sat, 2022-08-27 at 17:57 +0800, Guozihua (Scott) wrote:
>>>> On 2022/8/25 21:02, Mimi Zohar wrote:
>>>>> On Wed, 2022-08-24 at 09:56 +0800, Guozihua (Scott) wrote:
>>>>>> On 2022/8/24 9:26, Mimi Zohar wrote:
>>>>>>> On Tue, 2022-08-23 at 21:28 +0800, Guozihua (Scott) wrote:
>>>>>>>> On 2022/8/23 21:21, Mimi Zohar wrote:
>>>>>>>>> On Tue, 2022-08-23 at 16:12 +0800, Guozihua (Scott) wrote:
>>>>>>>>>>> The question is whether we're waiting for the SELinux policy to change
>>>>>>>>>>> from ESTALE or whether it is the number of SELinux based IMA policy
>>>>>>>>>>> rules or some combination of the two.  Retrying three times seems to be
>>>>>>>>>>> random.  If SELinux waited for ESTALE to change, then it would only be
>>>>>>>>>>> dependent on the time it took to update the SELinux based IMA policy
>>>>>>>>>>> rules.
>>>>>>>>>>
>>>>>>>>>> We are waiting for ima_lsm_update_rules() to finish re-initializing all
>>>>>>>>>> the LSM based rules.
>>>>>>>>>
>>>>>>>>> Fine.  Hopefully retrying a maximum of 3 times is sufficient.
>>>>>>>>>
>>>>>>>> Well, at least this should greatly reduce the chance of this issue from
>>>>>>>> happening.
>>>>>>>
>>>>>>> Agreed
>>>>>>>
>>>>>>>> This would be the best we I can think of without locking and
>>>>>>>> busy waiting. Maybe we can also add delays before we retry. Maybe you
>>>>>>>> got any other thought in mind?
>>>>>>>
>>>>>>> Another option would be to re-introduce the equivalent of the "lazy"
>>>>>>> LSM update on -ESTALE, but without updating the policy rule, as the
>>>>>>> notifier callback will eventually get to it.
>>>>>>>
>>>>>>
>>>>>> For this to happen we would need a way to tell when we are able to
>>>>>> continue with the retry though.
>>>>>
>>>>> Previously with the lazy update, on failure security_filter_rule_init()
>>>>> was called before the retry.  To avoid locking or detecting when to
>>>>> continue, another option would be to call to
>>>>> security_filter_rule_init() with a local copy of the rule.  The retry
>>>>> would be based on a local copy of the rule.
>>>>>
>>>>> Eventually the registered callback will complete, so we don't need to
>>>>> be concerned about updating the actual rules.
>>>>
>>>> Is it possible to cause race condition though? With this, the notifier
>>>> path seems to be unnecessary.
>>>
>>> I don't see how there would be a race condition.  The notifier callback
>>> is the normal method of updating the policy rules.  Hopefully -ESTALE
>>> isn't something that happens frequently.
>>
>> The notifier callback uses RCU to update rules, I think we should mimic
>> that behavior if we are to update individual rules in the matching logic.
> 
> If the callback update hasn't completed causing an -ESTALE, the
> fallback is to directly query the LSM for a single IMA policy rule.
> Please keep it simple.
> 

Got it, I'll send a new patch.

-- 
Best
GUO Zihua
[prev in list] [next in list] [prev in thread] [next in thread]