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] [day] [month] [year] [list]
Message-ID: <20250716-godlike-organic-pudu-53deda-mkl@pengutronix.de>
Date: Wed, 16 Jul 2025 07:55:32 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Andrei Lalaev <andrey.lalaev@...il.com>
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 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   |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ