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.
--
Best
GUO Zihua
