In gs_usb_probe(), gs_make_candev() is called to allocate and set up the CAN channel, as well as to call register_candev(). However, gs_usb_probe() defers the assignment of dev->parent and parent->hf_size_rx until after gs_make_candev() returns. This opens a race window for the user space to bring up the CAN interface, while it's not completely setup. To close the race window, initialize all CAN interfaces in a first loop and then register them in a second loop: Don't call register_candev() from gs_make_candev() but explicitly from gs_usb_probe(). Adjust gs_destroy_candev() likewise, move unregister_candev() to gs_usb_disconnect() and the error paths in gs_usb_probe(), to keep it inverse to symmetric to gs_make_candev(). Closes: https://sashiko.dev/#/bug/linux-be54de7d-0b75-408c-b3ee-dd90c9756857 Closes: https://sashiko.dev/#/bug/linux-71076d59-0843-4749-a5ab-fdd98a08582f Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices") Cc: stable@vger.kernel.org Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 65 +++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 15f28833f2de..31c25cda9f43 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -1490,18 +1490,8 @@ static struct gs_can *gs_make_candev(unsigned int channel, can_rx_offload_add_manual(netdev, &dev->offload, GS_NAPI_WEIGHT); SET_NETDEV_DEV(netdev, &intf->dev); - rc = register_candev(dev->netdev); - if (rc) { - dev_err(&intf->dev, - "Couldn't register candev for channel %d (%pe)\n", - channel, ERR_PTR(rc)); - goto out_can_rx_offload_del; - } - return dev; -out_can_rx_offload_del: - can_rx_offload_del(&dev->offload); out_free_candev: free_candev(dev->netdev); return ERR_PTR(rc); @@ -1509,7 +1499,6 @@ static struct gs_can *gs_make_candev(unsigned int channel, static void gs_destroy_candev(struct gs_can *dev) { - unregister_candev(dev->netdev); can_rx_offload_del(&dev->offload); free_candev(dev->netdev); } @@ -1587,21 +1576,16 @@ static int gs_usb_probe(struct usb_interface *intf, for (i = 0; i < icount; i++) { unsigned int hf_size_rx = 0; + struct gs_can *dev; - parent->canch[i] = gs_make_candev(i, intf, &dconf); - if (IS_ERR_OR_NULL(parent->canch[i])) { + dev = gs_make_candev(i, intf, &dconf); + if (IS_ERR_OR_NULL(dev)) { /* save error code to return later */ - rc = PTR_ERR(parent->canch[i]); + rc = PTR_ERR(dev); - /* on failure destroy previously created candevs */ - icount = i; - for (i = 0; i < icount; i++) - gs_destroy_candev(parent->canch[i]); - - usb_kill_anchored_urbs(&parent->rx_submitted); - kfree(parent); - return rc; + goto out_gs_destroy_candev; } + parent->canch[i] = dev; parent->canch[i]->parent = parent; /* set RX packet size based on FD and if hardware @@ -1621,7 +1605,37 @@ static int gs_usb_probe(struct usb_interface *intf, parent->hf_size_rx = max(parent->hf_size_rx, hf_size_rx); } + for (i = 0; i < icount; i++) { + struct gs_can *dev = parent->canch[i]; + + rc = register_candev(dev->netdev); + if (rc) { + const unsigned int cur = i; + + dev_err(&intf->dev, + "Couldn't register candev for channel %d (%pe)\n", + i, ERR_PTR(rc)); + + for (i = 0; i < cur; i++) + unregister_candev(parent->canch[i]->netdev); + + goto out_gs_destroy_candev; + } + } + return 0; + +out_gs_destroy_candev: + for (i = 0; i < parent->channel_cnt; i++) { + if (!parent->canch[i]) + continue; + + gs_destroy_candev(parent->canch[i]); + } + + kfree(parent); + + return rc; } static void gs_usb_disconnect(struct usb_interface *intf) @@ -1636,9 +1650,10 @@ static void gs_usb_disconnect(struct usb_interface *intf) return; } - for (i = 0; i < parent->channel_cnt; i++) - if (parent->canch[i]) - gs_destroy_candev(parent->canch[i]); + for (i = 0; i < parent->channel_cnt; i++) { + unregister_candev(parent->canch[i]->netdev); + gs_destroy_candev(parent->canch[i]); + } kfree(parent); } -- 2.53.0