Commit 2603be9e authored by Marc Kleine-Budde's avatar Marc Kleine-Budde
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: stable@vger.kernel.org
Cc: John Whittington <git@jbrengineering.co.uk>
Link: https://lore.kernel.org/all/20230716-gs_usb-fix-time-stamp-counter-v1-1-9017cefcd9d5@pengutronix.de


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent 0dd1805f

drivers/net/can/usb/gs_usb.c

+22 −9
Original line number Diff line number Diff line
@@ -833,6 +833,7 @@ static int gs_can_open(struct net_device *netdev) 
		.mode = cpu_to_le32(GS_CAN_MODE_START),

	};

	struct gs_host_frame *hf;

	struct urb *urb = NULL;

	u32 ctrlmode;

	u32 flags = 0;

	int rc, i;
@@ -856,13 +857,14 @@ static int gs_can_open(struct net_device *netdev) 


	if (!parent->active_channels) {

		for (i = 0; i < GS_MAX_RX_URBS; i++) {

			struct urb *urb;

			u8 *buf;



			/* alloc rx urb */

			urb = usb_alloc_urb(0, GFP_KERNEL);

			if (!urb)

				return -ENOMEM;

			if (!urb) {

				rc = -ENOMEM;

				goto out_usb_kill_anchored_urbs;

			}



			/* alloc rx buffer */

			buf = kmalloc(dev->parent->hf_size_rx,
@@ -870,8 +872,8 @@ static int gs_can_open(struct net_device *netdev) 
			if (!buf) {

				netdev_err(netdev,

					   "No memory left for USB buffer\n");

				usb_free_urb(urb);

				return -ENOMEM;

				rc = -ENOMEM;

				goto out_usb_free_urb;

			}



			/* fill, anchor, and submit rx urb */
@@ -894,9 +896,7 @@ static int gs_can_open(struct net_device *netdev) 
				netdev_err(netdev,

					   "usb_submit failed (err=%d)\n", rc);



				usb_unanchor_urb(urb);

				usb_free_urb(urb);

				break;

				goto out_usb_unanchor_urb;

			}



			/* Drop reference,
@@ -945,7 +945,8 @@ static int gs_can_open(struct net_device *netdev) 
		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)

			gs_usb_timestamp_stop(dev);

		dev->can.state = CAN_STATE_STOPPED;

		return rc;



		goto out_usb_kill_anchored_urbs;

	}



	parent->active_channels++;
@@ -953,6 +954,18 @@ static int gs_can_open(struct net_device *netdev) 
		netif_start_queue(netdev);



	return 0;



out_usb_unanchor_urb:

	usb_unanchor_urb(urb);

out_usb_free_urb:

	usb_free_urb(urb);

out_usb_kill_anchored_urbs:

	if (!parent->active_channels)

		usb_kill_anchored_urbs(&dev->tx_submitted);



	close_candev(netdev);



	return rc;

}



static int gs_usb_get_state(const struct net_device *netdev,