Re: [RFC] IMA LSM based rule race condition issue on 4.19 LTS

["Guozihua (Scott)" <guozihua@huawei.com>]

On 2022/12/23 16:04, Guozihua (Scott) wrote:
> On 2022/12/21 18:51, Guozihua (Scott) wrote:
>> On 2022/12/20 9:11, Guozihua (Scott) wrote:
>>> On 2022/12/19 21:11, Mimi Zohar wrote:
>>>> On Mon, 2022-12-19 at 15:10 +0800, Guozihua (Scott) wrote:
>>>>> On 2022/12/16 11:04, Paul Moore wrote:
>>>>>> On Thu, Dec 15, 2022 at 9:36 PM Guozihua (Scott) <guozihua@huawei.com> wrote:
>>>>>>> On 2022/12/16 5:04, Paul Moore wrote:
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>>> How bad is the backport really?  Perhaps it is worth doing it to see
>>>>>>>> what it looks like?
>>>>>>>>
>>>>>>> It might not be that bad, I'll try to post a version next Monday.
>>>>>>
>>>>>> Thanks for giving it a shot.
>>>>>>
>>>>> When I am trying a partial backport of b16942455193 ("ima: use the lsm
>>>>> policy update notifier"), I took a closer look into it and if we rip off
>>>>> the RCU and the notifier part, there would be a potential UAF issue when
>>>>> multiple processes are calling ima_lsm_update_rule() and
>>>>> ima_match_rules() at the same time. ima_lsm_update_rule() would free the
>>>>> old rule if the new rule is successfully copied and initialized, leading
>>>>> to ima_match_rules() accessing a freed rule.
>>>>>
>>>>> To reserve the mainline solution, we would have to either introduce RCU
>>>>> for rule access, which would work better with notifier mechanism or the
>>>>> same rule would be updated multiple times, or we would have to introduce
>>>>> a lock for LSM based rule update.
>>>>
>>>> Even with the RCU changes, the rules will be updated multiple times. 
>>>> With your "ima: Handle -ESTALE returned by ima_filter_rule_match()"
>>>> patch, upstream makes a single local copy of the rule to avoid updating
>>>> it multiple times.  Without the notifier, it's updating all the rules.
>>> That's true. However, in the mainline solution, we are only making a
>>> local copy of the rule. In 4.19, because of the lazy update mechanism,
>>> we are replacing the rule on the rule list multiple times and is trying
>>> to free the original rule.
>>>>
>>>> Perhaps an atomic variable to detect if the rules are already being
>>>> updated would suffice.  If the atomic variable is set, make a single
>>>> local copy of the rule.
>>> That should do it. I'll send a patch set soon.
>>>
>> Including Huaxin Lu in the loop. Sorry for forgotten about it for quite
>> some time.
>>
>> I tried the backported solution, it seems that it's causing RCU stall.
>> It seems on 4.19.y IMA is already accessing rules through RCU. Still
>> debugging it.
> It seems that after the backport, a NULL pointer deference pops out.
> I'll have to look into it.
> 
It seems that any other means except from a full RCU or locking won't be
able to prevent race condition between policy update and rule match. Any
other suggestions?

-- 
Best
GUO Zihua