Skip to content

Commit 2603be9

Browse files
can: gs_usb: gs_can_open(): improve error handling
The gs_usb driver handles USB devices with more than 1 CAN channel. The RX path for all channels share the same bulk endpoint (the transmitted bulk data encodes the channel number). These per-device resources are allocated and submitted by the first opened channel. During this allocation, the resources are either released immediately in case of a failure or the URBs are anchored. All anchored URBs are finally killed with gs_usb_disconnect(). Currently, gs_can_open() returns with an error if the allocation of a URB or a buffer fails. However, if usb_submit_urb() fails, the driver continues with the URBs submitted so far, even if no URBs were successfully submitted. Treat every error as fatal and free all allocated resources immediately. Switch to goto-style error handling, to prepare the driver for more per-device resource allocation. Cc: [email protected] Cc: John Whittington <[email protected]> Link: https://lore.kernel.org/all/20230716-gs_usb-fix-time-stamp-counter-v1-1-9017cefcd9d5@pengutronix.de Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 0dd1805 commit 2603be9

File tree

1 file changed

+22
-9
lines changed

1 file changed

+22
-9
lines changed

drivers/net/can/usb/gs_usb.c

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,7 @@ static int gs_can_open(struct net_device *netdev)
833833
.mode = cpu_to_le32(GS_CAN_MODE_START),
834834
};
835835
struct gs_host_frame *hf;
836+
struct urb *urb = NULL;
836837
u32 ctrlmode;
837838
u32 flags = 0;
838839
int rc, i;
@@ -856,22 +857,23 @@ static int gs_can_open(struct net_device *netdev)
856857

857858
if (!parent->active_channels) {
858859
for (i = 0; i < GS_MAX_RX_URBS; i++) {
859-
struct urb *urb;
860860
u8 *buf;
861861

862862
/* alloc rx urb */
863863
urb = usb_alloc_urb(0, GFP_KERNEL);
864-
if (!urb)
865-
return -ENOMEM;
864+
if (!urb) {
865+
rc = -ENOMEM;
866+
goto out_usb_kill_anchored_urbs;
867+
}
866868

867869
/* alloc rx buffer */
868870
buf = kmalloc(dev->parent->hf_size_rx,
869871
GFP_KERNEL);
870872
if (!buf) {
871873
netdev_err(netdev,
872874
"No memory left for USB buffer\n");
873-
usb_free_urb(urb);
874-
return -ENOMEM;
875+
rc = -ENOMEM;
876+
goto out_usb_free_urb;
875877
}
876878

877879
/* fill, anchor, and submit rx urb */
@@ -894,9 +896,7 @@ static int gs_can_open(struct net_device *netdev)
894896
netdev_err(netdev,
895897
"usb_submit failed (err=%d)\n", rc);
896898

897-
usb_unanchor_urb(urb);
898-
usb_free_urb(urb);
899-
break;
899+
goto out_usb_unanchor_urb;
900900
}
901901

902902
/* Drop reference,
@@ -945,14 +945,27 @@ static int gs_can_open(struct net_device *netdev)
945945
if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
946946
gs_usb_timestamp_stop(dev);
947947
dev->can.state = CAN_STATE_STOPPED;
948-
return rc;
948+
949+
goto out_usb_kill_anchored_urbs;
949950
}
950951

951952
parent->active_channels++;
952953
if (!(dev->can.ctrlmode & CAN_CTRLMODE_LISTENONLY))
953954
netif_start_queue(netdev);
954955

955956
return 0;
957+
958+
out_usb_unanchor_urb:
959+
usb_unanchor_urb(urb);
960+
out_usb_free_urb:
961+
usb_free_urb(urb);
962+
out_usb_kill_anchored_urbs:
963+
if (!parent->active_channels)
964+
usb_kill_anchored_urbs(&dev->tx_submitted);
965+
966+
close_candev(netdev);
967+
968+
return rc;
956969
}
957970

958971
static int gs_usb_get_state(const struct net_device *netdev,

0 commit comments

Comments
 (0)