On 2022/12/24 11:41, Guozihua (Scott) wrote:
> 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@xxxxxxxxxx> 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?
Correction: full RCU won't be enough as RCU won't work well without
notifier. My suggestion would be to backport the notifier mechanism.
--
Best
GUO Zihua
