linux-kernel - Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <988d9355-2243-4187-b4ab-78652a1fb008@gmail.com>
Date: Wed, 16 Jul 2025 07:45:08 +0200
From: Andrei Lalaev <andrey.lalaev@...il.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: mailhol.vincent@...adoo.fr, linux-kernel@...r.kernel.org,
 linux-can@...r.kernel.org
Subject: Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart

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