Commit 6d547995 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde
Browse files

Merge patch series "can: gs_usb: fix USB bulk in and out callbacks" 

Marc Kleine-Budde <mkl@pengutronix.de> says:

The bulk-out callback gs_usb_xmit_callback() does not take care of the
cleanup of failed transfers of URBs. The 1st patch adds the missing
cleanup.

The bulk-in callback gs_usb_receive_bulk_callback() accesses the buffer of
the URB without checking how much data has actually been received. The last
2 patches fix this problem.

Link: https://patch.msgid.link/20251114-gs_usb-fix-usb-callbacks-v1-0-a29b42eacada@pengutronix.de


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents 30db4451 395d988f

drivers/net/can/usb/gs_usb.c

+87 −13
Original line number Diff line number Diff line
@@ -261,7 +261,13 @@ struct canfd_quirk { 
	u8 quirk;

} __packed;



/* struct gs_host_frame::echo_id == GS_HOST_FRAME_ECHO_ID_RX indicates

 * a regular RX'ed CAN frame

 */

#define GS_HOST_FRAME_ECHO_ID_RX 0xffffffff



struct gs_host_frame {

	struct_group(header,

		u32 echo_id;

		__le32 can_id;
@@ -269,6 +275,7 @@ struct gs_host_frame { 
		u8 channel;

		u8 flags;

		u8 reserved;

	);



	union {

		DECLARE_FLEX_ARRAY(struct classic_can, classic_can);
@@ -568,6 +575,37 @@ gs_usb_get_echo_skb(struct gs_can *dev, struct sk_buff *skb, 
	return len;

}



static unsigned int

gs_usb_get_minimum_rx_length(const struct gs_can *dev, const struct gs_host_frame *hf,

			     unsigned int *data_length_p)

{

	unsigned int minimum_length, data_length = 0;



	if (hf->flags & GS_CAN_FLAG_FD) {

		if (hf->echo_id == GS_HOST_FRAME_ECHO_ID_RX)

			data_length = can_fd_dlc2len(hf->can_dlc);



		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)

			/* timestamp follows data field of max size */

			minimum_length = struct_size(hf, canfd_ts, 1);

		else

			minimum_length = sizeof(hf->header) + data_length;

	} else {

		if (hf->echo_id == GS_HOST_FRAME_ECHO_ID_RX &&

		    !(hf->can_id & cpu_to_le32(CAN_RTR_FLAG)))

			data_length = can_cc_dlc2len(hf->can_dlc);



		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)

			/* timestamp follows data field of max size */

			minimum_length = struct_size(hf, classic_can_ts, 1);

		else

			minimum_length = sizeof(hf->header) + data_length;

	}



	*data_length_p = data_length;

	return minimum_length;

}



static void gs_usb_receive_bulk_callback(struct urb *urb)

{

	struct gs_usb *parent = urb->context;
@@ -576,6 +614,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) 
	int rc;

	struct net_device_stats *stats;

	struct gs_host_frame *hf = urb->transfer_buffer;

	unsigned int minimum_length, data_length;

	struct gs_tx_context *txc;

	struct can_frame *cf;

	struct canfd_frame *cfd;
@@ -594,6 +633,15 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) 
		return;

	}



	minimum_length = sizeof(hf->header);

	if (urb->actual_length < minimum_length) {

		dev_err_ratelimited(&parent->udev->dev,

				    "short read (actual_length=%u, minimum_length=%u)\n",

				    urb->actual_length, minimum_length);



		goto resubmit_urb;

	}



	/* device reports out of range channel id */

	if (hf->channel >= parent->channel_cnt)

		goto device_detach;
@@ -609,20 +657,33 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) 
	if (!netif_running(netdev))

		goto resubmit_urb;



	if (hf->echo_id == -1) { /* normal rx */

	minimum_length = gs_usb_get_minimum_rx_length(dev, hf, &data_length);

	if (urb->actual_length < minimum_length) {

		stats->rx_errors++;

		stats->rx_length_errors++;



		if (net_ratelimit())

			netdev_err(netdev,

				   "short read (actual_length=%u, minimum_length=%u)\n",

				   urb->actual_length, minimum_length);



		goto resubmit_urb;

	}



	if (hf->echo_id == GS_HOST_FRAME_ECHO_ID_RX) { /* normal rx */

		if (hf->flags & GS_CAN_FLAG_FD) {

			skb = alloc_canfd_skb(netdev, &cfd);

			if (!skb)

				return;



			cfd->can_id = le32_to_cpu(hf->can_id);

			cfd->len = can_fd_dlc2len(hf->can_dlc);

			cfd->len = data_length;

			if (hf->flags & GS_CAN_FLAG_BRS)

				cfd->flags |= CANFD_BRS;

			if (hf->flags & GS_CAN_FLAG_ESI)

				cfd->flags |= CANFD_ESI;



			memcpy(cfd->data, hf->canfd->data, cfd->len);

			memcpy(cfd->data, hf->canfd->data, data_length);

		} else {

			skb = alloc_can_skb(netdev, &cf);

			if (!skb)
@@ -631,7 +692,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) 
			cf->can_id = le32_to_cpu(hf->can_id);

			can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);



			memcpy(cf->data, hf->classic_can->data, 8);

			memcpy(cf->data, hf->classic_can->data, data_length);



			/* ERROR frames tell us information about the controller */

			if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
@@ -687,7 +748,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) 
resubmit_urb:

	usb_fill_bulk_urb(urb, parent->udev,

			  parent->pipe_in,

			  hf, dev->parent->hf_size_rx,

			  hf, parent->hf_size_rx,

			  gs_usb_receive_bulk_callback, parent);



	rc = usb_submit_urb(urb, GFP_ATOMIC);
@@ -750,8 +811,21 @@ static void gs_usb_xmit_callback(struct urb *urb) 
	struct gs_can *dev = txc->dev;

	struct net_device *netdev = dev->netdev;



	if (urb->status)

		netdev_info(netdev, "usb xmit fail %u\n", txc->echo_id);

	if (!urb->status)

		return;



	if (urb->status != -ESHUTDOWN && net_ratelimit())

		netdev_info(netdev, "failed to xmit URB %u: %pe\n",

			    txc->echo_id, ERR_PTR(urb->status));



	netdev->stats.tx_dropped++;

	netdev->stats.tx_errors++;



	can_free_echo_skb(netdev, txc->echo_id, NULL);

	gs_free_tx_context(txc);

	atomic_dec(&dev->active_tx_urbs);



	netif_wake_queue(netdev);

}



static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,