Re: [PATCH net] can: bcm: switch timer to HRTIMER_MODE_SOFT and remove hrtimer_tasklet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.

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.

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.




[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux