* [RFC PATCH] can: gs_usb: fix kernel oops during restart
@ 2025-07-14 17:55 Andrei Lalaev
2025-07-15 9:37 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Lalaev @ 2025-07-14 17:55 UTC (permalink / raw)
To: mkl, mailhol.vincent; +Cc: andrey.lalaev, linux-kernel, linux-can
When CAN adapter in BUS_OFF state and "can_restart" is called,
it causes the following kernel oops:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
Internal error: Oops: 0000000086000005 [#1] PREEMPT SMP
CPU: 0 UID: 0 PID: 725 Comm: ip Not tainted 6.12.37-v8-16k+ #2
Hardware name: Raspberry Pi 5 Model B Rev 1.0 (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : 0x0
lr : can_restart+0x80/0xf8 [can_dev]
sp : ffffc000844f3700
x29: ffffc000844f3710 x28: ffffd06fcf3389f8 x27: 0000000000000000
x26: ffff800080ba0000 x25: 0000000000000000 x24: ffffd06f58730268
x23: 0000000000000000 x22: 0000000000000001 x21: ffff8001001ef210
x20: ffffc000844f3a10 x19: ffff800080ba0000 x18: 0000000000000000
x17: 0000000000000002 x16: 000000005b38ca14 x15: 0000000000000400
x14: 0000000000000800 x13: 000000005b482df7 x12: ffff800002d84280
x11: 000000005b482df7 x10: ffff800002d84290 x9 : ffffd06fcf01f6ec
x8 : 0000000000000000 x7 : 0000000000000000 x6 : 000000000000003f
x5 : ffffd06fcef8ab30 x4 : 0000000000000008 x3 : 016b3b57a19d7300
x2 : 0000000000000088 x1 : 0000000000000001 x0 : ffff800080ba0000
Call trace:
0x0
can_restart_now+0x4c/0x70 [can_dev]
can_changelink+0x258/0x458 [can_dev]
rtnl_newlink+0x52c/0xa38
rtnetlink_rcv_msg+0x238/0x338
netlink_rcv_skb+0x128/0x148
rtnetlink_rcv+0x24/0x38
netlink_unicast+0x24c/0x408
netlink_sendmsg+0x288/0x378
____sys_sendmsg+0x1bc/0x2a0
__sys_sendmsg+0x144/0x1a0
__arm64_sys_sendmsg+0x30/0x48
invoke_syscall+0x4c/0x110
el0_svc_common+0x8c/0xf0
do_el0_svc+0x28/0x40
el0_svc+0x34/0xa0
el0t_64_sync_handler+0x84/0x100
el0t_64_sync+0x190/0x198
Provide a "do_set_mode" callback to overcome the issue.
Signed-off-by: Andrei Lalaev <andrey.lalaev@gmail.com>
---
The issue can be easily reproduced:
ip link set can0 type can bitrate 100000
ip link set up can0
cangen can0
Then I force "BUS_OFF" by connecting CAN_HIGH to CAN_LOW.
And restart the interface:
ip link set can0 type can restart
My knowledge about CAN is pretty limited, so I am not sure
if it is a correct or complete solution.
Could someone with more experience in CAN or the gs_usb driver confirm
whether this should be addressed by implementing the do_set_mode in gs_usb,
or if there's a better approach?
Thanks in advance!
---
drivers/net/can/usb/gs_usb.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index bb6335278e46..0d66d843c1e3 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -748,6 +748,18 @@ static int gs_usb_set_data_bittiming(struct net_device *netdev)
GFP_KERNEL);
}
+static int gs_usb_do_set_mode(struct net_device *netdev, enum can_mode mode)
+{
+ switch (mode) {
+ case CAN_MODE_START:
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
static void gs_usb_xmit_callback(struct urb *urb)
{
struct gs_tx_context *txc = urb->context;
@@ -1278,6 +1290,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
dev->can.clock.freq = le32_to_cpu(bt_const.fclk_can);
dev->can.bittiming_const = &dev->bt_const;
dev->can.do_set_bittiming = gs_usb_set_bittiming;
+ dev->can.do_set_mode = gs_usb_do_set_mode;
dev->can.ctrlmode_supported = CAN_CTRLMODE_CC_LEN8_DLC;
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart
2025-07-14 17:55 [RFC PATCH] can: gs_usb: fix kernel oops during restart Andrei Lalaev
@ 2025-07-15 9:37 ` Marc Kleine-Budde
2025-07-15 14:24 ` Andrei Lalaev
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-07-15 9:37 UTC (permalink / raw)
To: Andrei Lalaev; +Cc: mailhol.vincent, linux-kernel, linux-can
[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]
On 14.07.2025 19:55:02, Andrei Lalaev wrote:
> When CAN adapter in BUS_OFF state and "can_restart" is called,
> it causes the following kernel oops:
Doh!
I wonder why no one stumbled over this before. That's a systematic
problem for all CAN drivers that don't implement this callback.
What about this fix?
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 13826e8a707b..94603c9eb4aa 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
}
if (data[IFLA_CAN_RESTART_MS]) {
+ if (!priv->do_set_mode) {
+ NL_SET_ERR_MSG(extack,
+ "device doesn't support restart from Bus Off");
+ return -EOPNOTSUPP;
+ }
+
/* Do not allow changing restart delay while running */
if (dev->flags & IFF_UP)
return -EBUSY;
@@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
}
if (data[IFLA_CAN_RESTART]) {
+ if (!priv->do_set_mode) {
+ NL_SET_ERR_MSG(extack,
+ "device doesn't support restart from Bus Off");
+ return -EOPNOTSUPP;
+ }
+
/* Do not allow a restart while not running */
if (!(dev->flags & IFF_UP))
return -EINVAL;
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart
2025-07-15 9:37 ` Marc Kleine-Budde
@ 2025-07-15 14:24 ` Andrei Lalaev
2025-07-15 14:29 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Lalaev @ 2025-07-15 14:24 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: mailhol.vincent, linux-kernel, linux-can
On 15.07.2025 11:37, Marc Kleine-Budde wrote:
> On 14.07.2025 19:55:02, Andrei Lalaev wrote:
>> When CAN adapter in BUS_OFF state and "can_restart" is called,
>> it causes the following kernel oops:
>
> Doh!
>
> I wonder why no one stumbled over this before. That's a systematic
> problem for all CAN drivers that don't implement this callback.
Hi Mark,
I was also surprised because this callback isn't marked as mandatory
and that there are no additional checks.
>
> What about this fix?
>
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 13826e8a707b..94603c9eb4aa 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> }
>
> if (data[IFLA_CAN_RESTART_MS]) {
> + if (!priv->do_set_mode) {
> + NL_SET_ERR_MSG(extack,
> + "device doesn't support restart from Bus Off");
> + return -EOPNOTSUPP;
> + }
> +
> /* Do not allow changing restart delay while running */
> if (dev->flags & IFF_UP)
> return -EBUSY;
> @@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> }
>
> if (data[IFLA_CAN_RESTART]) {
> + if (!priv->do_set_mode) {
> + NL_SET_ERR_MSG(extack,
> + "device doesn't support restart from Bus Off");
> + return -EOPNOTSUPP;
> + }
> +
> /* Do not allow a restart while not running */
> if (!(dev->flags & IFF_UP))
> return -EINVAL;
>
> regards,
> Marc
>
Thanks for the patch. As expected, it fixes the kernel OOPS,
but the interface never leaves the BUS_OFF state.
--
Best regards,
Andrei Lalaev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart
2025-07-15 14:24 ` Andrei Lalaev
@ 2025-07-15 14:29 ` Marc Kleine-Budde
2025-07-16 5:45 ` Andrei Lalaev
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-07-15 14:29 UTC (permalink / raw)
To: Andrei Lalaev; +Cc: mailhol.vincent, linux-kernel, linux-can
[-- Attachment #1: Type: text/plain, Size: 2303 bytes --]
On 15.07.2025 16:24:22, Andrei Lalaev wrote:
> I was also surprised because this callback isn't marked as mandatory
> and that there are no additional checks.
>
> >
> > What about this fix?
> >
> > diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> > index 13826e8a707b..94603c9eb4aa 100644
> > --- a/drivers/net/can/dev/netlink.c
> > +++ b/drivers/net/can/dev/netlink.c
> > @@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> > }
> >
> > if (data[IFLA_CAN_RESTART_MS]) {
> > + if (!priv->do_set_mode) {
> > + NL_SET_ERR_MSG(extack,
> > + "device doesn't support restart from Bus Off");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > /* Do not allow changing restart delay while running */
> > if (dev->flags & IFF_UP)
> > return -EBUSY;
> > @@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> > }
> >
> > if (data[IFLA_CAN_RESTART]) {
> > + if (!priv->do_set_mode) {
> > + NL_SET_ERR_MSG(extack,
> > + "device doesn't support restart from Bus Off");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > /* Do not allow a restart while not running */
> > if (!(dev->flags & IFF_UP))
> > return -EINVAL;
>
> Thanks for the patch. As expected, it fixes the kernel OOPS,
> but the interface never leaves the BUS_OFF state.
Which device and which firmware are you using?
The gs_usb/candlelight interface is un(der)defined when it comes to
bus-off handling.
I think the original candlelight with the stm32f072 does auto bus-off
recovery. Not sure about the candlelight on stm32g0b1.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart
2025-07-15 14:29 ` Marc Kleine-Budde
@ 2025-07-16 5:45 ` Andrei Lalaev
2025-07-16 5:55 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Lalaev @ 2025-07-16 5:45 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: mailhol.vincent, linux-kernel, linux-can
On 15.07.2025 16:29, Marc Kleine-Budde wrote:
> On 15.07.2025 16:24:22, Andrei Lalaev wrote:
>> I was also surprised because this callback isn't marked as mandatory
>> and that there are no additional checks.
>>
>>>
>>> What about this fix?
>>>
>>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>>> index 13826e8a707b..94603c9eb4aa 100644
>>> --- a/drivers/net/can/dev/netlink.c
>>> +++ b/drivers/net/can/dev/netlink.c
>>> @@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>>> }
>>>
>>> if (data[IFLA_CAN_RESTART_MS]) {
>>> + if (!priv->do_set_mode) {
>>> + NL_SET_ERR_MSG(extack,
>>> + "device doesn't support restart from Bus Off");
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> /* Do not allow changing restart delay while running */
>>> if (dev->flags & IFF_UP)
>>> return -EBUSY;
>>> @@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>>> }
>>>
>>> if (data[IFLA_CAN_RESTART]) {
>>> + if (!priv->do_set_mode) {
>>> + NL_SET_ERR_MSG(extack,
>>> + "device doesn't support restart from Bus Off");
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> /* Do not allow a restart while not running */
>>> if (!(dev->flags & IFF_UP))
>>> return -EINVAL;
>>
>> Thanks for the patch. As expected, it fixes the kernel OOPS,
>> but the interface never leaves the BUS_OFF state.
>
> Which device and which firmware are you using?
>
> The gs_usb/candlelight interface is un(der)defined when it comes to
> bus-off handling.
>
> I think the original candlelight with the stm32f072 does auto bus-off
> recovery. Not sure about the candlelight on stm32g0b1.
>
> regards,
> Marc
>
Sorry, my bad for not mentioning it earlier. I have several USB-CAN adapters:
- two are based on STM32F072 (not original CandleLight, but using the same FW)
- one is a original CandleLightFD on STM32G0B1, that I used for testing
And all of them behave exactly as you described:
- both STM32F072-based automatically recover from BUS_OFF and I see
it in `ip -details link show can0`
- STM32G0B1-based doesn't recover and I have to down/up interface to restore it
Since this is expected behavior and no kernel OOPS occurs,
I will switch to your patch.
Thanks a lot for the patch and your help!
--
Best regards,
Andrei Lalaev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart
2025-07-16 5:45 ` Andrei Lalaev
@ 2025-07-16 5:55 ` Marc Kleine-Budde
0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-07-16 5:55 UTC (permalink / raw)
To: Andrei Lalaev; +Cc: mailhol.vincent, linux-kernel, linux-can
[-- Attachment #1: Type: text/plain, Size: 3442 bytes --]
On 16.07.2025 07:45:08, Andrei Lalaev wrote:
> On 15.07.2025 16:29, Marc Kleine-Budde wrote:
> > On 15.07.2025 16:24:22, Andrei Lalaev wrote:
> >> I was also surprised because this callback isn't marked as mandatory
> >> and that there are no additional checks.
> >>
> >>>
> >>> What about this fix?
> >>>
> >>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> >>> index 13826e8a707b..94603c9eb4aa 100644
> >>> --- a/drivers/net/can/dev/netlink.c
> >>> +++ b/drivers/net/can/dev/netlink.c
> >>> @@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> >>> }
> >>>
> >>> if (data[IFLA_CAN_RESTART_MS]) {
> >>> + if (!priv->do_set_mode) {
> >>> + NL_SET_ERR_MSG(extack,
> >>> + "device doesn't support restart from Bus Off");
> >>> + return -EOPNOTSUPP;
> >>> + }
> >>> +
> >>> /* Do not allow changing restart delay while running */
> >>> if (dev->flags & IFF_UP)
> >>> return -EBUSY;
> >>> @@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> >>> }
> >>>
> >>> if (data[IFLA_CAN_RESTART]) {
> >>> + if (!priv->do_set_mode) {
> >>> + NL_SET_ERR_MSG(extack,
> >>> + "device doesn't support restart from Bus Off");
> >>> + return -EOPNOTSUPP;
> >>> + }
> >>> +
> >>> /* Do not allow a restart while not running */
> >>> if (!(dev->flags & IFF_UP))
> >>> return -EINVAL;
> >>
> >> Thanks for the patch. As expected, it fixes the kernel OOPS,
> >> but the interface never leaves the BUS_OFF state.
> >
> > Which device and which firmware are you using?
> >
> > The gs_usb/candlelight interface is un(der)defined when it comes to
> > bus-off handling.
> >
> > I think the original candlelight with the stm32f072 does auto bus-off
> > recovery. Not sure about the candlelight on stm32g0b1.
>
> Sorry, my bad for not mentioning it earlier. I have several USB-CAN adapters:
> - two are based on STM32F072 (not original CandleLight, but using the same FW)
> - one is a original CandleLightFD on STM32G0B1, that I used for testing
>
> And all of them behave exactly as you described:
> - both STM32F072-based automatically recover from BUS_OFF and I see
> it in `ip -details link show can0`
> - STM32G0B1-based doesn't recover and I have to down/up interface to restore it
>
> Since this is expected behavior and no kernel OOPS occurs,
> I will switch to your patch.
At least the behavior can be explained, it's not expected, though :) I
think we have to fix the stm32g0b1 firmware to auto recover from bus
off...and in the long term, extend the candlelight firmware, the USB
protocol and the Linux driver to support proper Bus-Off handling.
> Thanks a lot for the patch and your help!
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-16 5:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 17:55 [RFC PATCH] can: gs_usb: fix kernel oops during restart Andrei Lalaev
2025-07-15 9:37 ` Marc Kleine-Budde
2025-07-15 14:24 ` Andrei Lalaev
2025-07-15 14:29 ` Marc Kleine-Budde
2025-07-16 5:45 ` Andrei Lalaev
2025-07-16 5:55 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).