Re: [PATCH] can: m_can: initialize spin lock on device probe

From: Vincent Mailhol
Date: Fri Apr 25 2025 - 03:27:34 EST

Next message: Shiming Cheng: "[PATCH] net: call inet_twsk_put() on TIMEWAIT sockets It is possible for a pointer of type struct inet_timewait_sock to be returned from the functions __inet_lookup_established() and __inet6_lookup_established(). This can cause a crash when the returned pointer is of type struct inet_timewait_sock and sock_put() is called on it. The following is a crash call stack that shows sk->sk_wmem_alloc being accessed in sk_free() during the call to sock_put() on a struct inet_timewait_sock pointer. To avoid this issue, use inet_twsk_put() instead of sock_put() when sk->sk_state is TCP_TIME_WAIT."
Previous message: Andrew Jones: "Re: [PATCH 2/3] riscv: Strengthen duplicate and inconsistent definition of RV_X()"
In reply to: Antonios Salios: "Re: [PATCH] can: m_can: initialize spin lock on device probe"
Next in thread: Antonios Salios: "Re: [PATCH] can: m_can: initialize spin lock on device probe"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fry. 25 Apr. 2025 at 15:34, Antonios Salios <antonios@xxxxxx> wrote:
> On Fri, 2025-04-25 at 00:23 +0900, Vincent Mailhol wrote:
> > Maybe you can briefly describe what kind of bug (NULL pointer
> > dereference).
>
> It's a spinlock bad magic bug that occurs when one tries to send a CAN
> frame using cansend. The frame gets transferred nonetheless.
> I'm testing the driver in an virtual RISC-V 64-bit environment with a
> recent mainline kernel. The M_CAN controller is io-mapped to the
> system.

I guess this is because your kernel has CONFIG_DEBUG_SPINLOCK:

https://elixir.bootlin.com/linux/v6.14.3/source/lib/Kconfig.debug#L1437

Without it, this would have been a more severe NULL pointer dereference.

> > Also, if you have the dmesg log of the error, this is something you
> > can add at
> > the end of the patch description.
>
> Will do, I'm just waiting for more feedback on the patch before sending
> a v3. In the meantime, the dmesg log looks like this:

Great!

My comments on the patch description are just nitpicks anyway. You can add my:

Reviewed-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>

to the v3.

Thanks a lot!

Yours sincerely,
Vincent Mailhol