> On 11.01.22 03:02, Ziyang Xuan (William) wrote:
>>> On Mon, Jan 10, 2022 at 09:23:22PM +0800, Ziyang Xuan wrote:
>>>> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>>>>
>>>> [ commit bf74aa86e111aa3b2fbb25db37e3a3fab71b5b68 upstream ]
>>>>
>>>> Stop tx/rx cycle rely on the active state of tasklet and hrtimer
>>>> sequentially in bcm_remove_op(), the op object will be freed if they
>>>> are all unactive. Assume the hrtimer timeout is short, the hrtimer
>>>> cb has been excuted after tasklet conditional judgment which must be
>>>> false after last round tasklet_kill() and before condition
>>>> hrtimer_active(), it is false when execute to hrtimer_active(). Bug
>>>> is triggerd, because the stopping action is end and the op object
>>>> will be freed, but the tasklet is scheduled. The resources of the op
>>>> object will occur UAF bug.
>>>
>>> That is not the changelog text of this commit. Why modify it?
>>
>> Above statement is the reason why I want to backport the patch to
>> stable tree. Maybe I could give an extra cover-letter to explain
>> the details of the problem, but modify the original changelog. Is it?
>>
>
> If you backport the bcm HRTIMER_MODE_SOFT implementation to the 4.19 stable tree the problem is not fixed for 4.14, 4.4, etc.
This backport patch does not fit lts 4.4 and 4.9. In addition,
I backport patch to lts for the first time. So I am going to
backport the patch one by one.
>
> HRTIMER_MODE_SOFT has been introduced in 4.16
>
> The issue of a race condition at bcm op removal has already been addressed before in commit a06393ed03167 ("can: bcm: fix hrtimer/tasklet termination in bcm op removal").
>
> - hrtimer_cancel(&op->timer);
> - hrtimer_cancel(&op->thrtimer);
> -
> - if (op->tsklet.func)
> - tasklet_kill(&op->tsklet);
> + if (op->tsklet.func) {
> + while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
> + test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
> + hrtimer_active(&op->timer)) {
> + hrtimer_cancel(&op->timer);
> + tasklet_kill(&op->tsklet);
> + }
> + }
>
> IMO we should better try to improve this fix and enable it for older stable trees than fixing only the 4.19.
The commit bf74aa86e111 can solve the op UAF nicely and enter mainline from v5.4.
I prefer to backport the patch to lower lts, but other sewing and mending,
for example move hrtimer_active() forward.
>
> Best regards,
> Oliver
>
>
>
>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>
>>>> This patch switches the timer to HRTIMER_MODE_SOFT, which executed the
>>>> timer callback in softirq context and removes the hrtimer_tasklet.
>>>>
>>>> Reported-by: syzbot+652023d5376450cc8516@xxxxxxxxxxxxxxxxxxxxxxxxx
>>
>> This is the public problem reporter. Do I need to move it to cover-letter
>> but here?
>>
>>>> Cc: stable@xxxxxxxxxxxxxxx # 4.19
>>
>> I want to backport the patch to linux-4.19.y stable tree. How do I need to
>> modify?
>>
>>>> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>>>> Signed-off-by: Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
>>>> Acked-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
>>>> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Ziyang Xuan <william.xuanziyang@xxxxxxxxxx>
>>>> ---
>>>> net/can/bcm.c | 156 +++++++++++++++++---------------------------------
>>>> 1 file changed, 52 insertions(+), 104 deletions(-)
>>>
>>> What stable kernel tree(s) are you wanting this backported to?
>>>
>>> thanks,
>>>
>>> greg k-h
>>> .
>>>
>>
>> Thank you for your patient guidance.
>>
> .
