Re: [PATCH 3/3] can: gs_usb: gs_can_open(): initialize time counter before starting device

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

On 21.09.2022 07:07:42, john@jbrengineering.co.uk wrote:
> > On busy networks the CAN controller might receive CAN frames directly
> > after starting it but before the timecounter is setup. This will lead
> > to NULL pointer deref while converting the converting the CAN frame's
> > timestamp with the timecounter.
> > 
> > Close the race window by setting up the timecounter before starting
> > the CAN controller.
> 
> My logic of starting the timer after the USB request to start was due
> to the function returning before if the USB start request returns an
> error. With this change, the timer will be started and poll the USB
> timestamp request even if the device is not started - I tested and
> confirmed this is the case.
> 
> I agree with the issue this patch solves and flaw previously but
> believe it requires a check of the GS_CAN_FEATURE_HW_TIMESTAMP and
> gs_usb_timestamp_stop(dev) in the rc < 0 check.
> 
> if (rc < 0) {
>   netdev_err(netdev, "Couldn't start device (err=%d)\n", rc);
>   kfree(dm);
>   /* stop polling timestamp */
>   if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
>     gs_usb_timestamp_stop(dev);
>   return rc;
> }

Right! Will fix that.

Thanks,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]