1. Patchew
  2. linux
  3. View series

[PATCH v2] can: esd_usb: add endpoint type validation

Ziyi Guo posted 1 patch 1 month, 2 weeks ago 
drivers/net/can/usb/esd_usb.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
[PATCH v2] can: esd_usb: add endpoint type validation
Posted by Ziyi Guo 1 month, 2 weeks ago 
esd_usb_probe() constructs bulk pipes for two endpoints without
verifying their transfer types:

  - usb_rcvbulkpipe(dev->udev, 1) for RX (version reply, async RX data)
  - usb_sndbulkpipe(dev->udev, 2) for TX (version query, CAN frames)

A malformed USB device can present these endpoints with transfer types
that differ from what the driver assumes, triggering the WARNING in
usb_submit_urb().

Use usb_find_common_endpoints() to discover and validate the first
bulk IN and bulk OUT endpoints at probe time, before any allocation.
Found pipes are saved to struct esd_usb and code uses them directly
instead of making pipes in place.

Similar to
- commit 136bed0bfd3b ("can: mcba_usb: properly check endpoint type")
  which established the usb_find_common_endpoints() + stored pipes
  pattern for CAN USB drivers.

Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
Suggested-by: Vincent Mailhol <mailhol@kernel.org>
Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
---
v2:
 - Use usb_find_common_endpoints() to discover and validate. 

 drivers/net/can/usb/esd_usb.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/usb/esd_usb.c b/drivers/net/can/usb/esd_usb.c
index 8cc924c47042..c1da0f3acf89 100644
--- a/drivers/net/can/usb/esd_usb.c
+++ b/drivers/net/can/usb/esd_usb.c
@@ -272,6 +272,9 @@ struct esd_usb {
 
 	struct usb_anchor rx_submitted;
 
+	unsigned int rx_pipe;
+	unsigned int tx_pipe;
+
 	int net_count;
 	u32 version;
 	int rxinitdone;
@@ -537,7 +540,7 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
 	}
 
 resubmit_urb:
-	usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
+	usb_fill_bulk_urb(urb, dev->udev, dev->rx_pipe,
 			  urb->transfer_buffer, ESD_USB_RX_BUFFER_SIZE,
 			  esd_usb_read_bulk_callback, dev);
 
@@ -626,9 +629,7 @@ static int esd_usb_send_msg(struct esd_usb *dev, union esd_usb_msg *msg)
 {
 	int actual_length;
 
-	return usb_bulk_msg(dev->udev,
-			    usb_sndbulkpipe(dev->udev, 2),
-			    msg,
+	return usb_bulk_msg(dev->udev, dev->tx_pipe, msg,
 			    msg->hdr.len * sizeof(u32), /* convert to # of bytes */
 			    &actual_length,
 			    1000);
@@ -639,12 +640,8 @@ static int esd_usb_wait_msg(struct esd_usb *dev,
 {
 	int actual_length;
 
-	return usb_bulk_msg(dev->udev,
-			    usb_rcvbulkpipe(dev->udev, 1),
-			    msg,
-			    sizeof(*msg),
-			    &actual_length,
-			    1000);
+	return usb_bulk_msg(dev->udev, dev->rx_pipe, msg,
+			    sizeof(*msg), &actual_length, 1000);
 }
 
 static int esd_usb_setup_rx_urbs(struct esd_usb *dev)
@@ -677,8 +674,7 @@ static int esd_usb_setup_rx_urbs(struct esd_usb *dev)
 
 		urb->transfer_dma = buf_dma;
 
-		usb_fill_bulk_urb(urb, dev->udev,
-				  usb_rcvbulkpipe(dev->udev, 1),
+		usb_fill_bulk_urb(urb, dev->udev, dev->rx_pipe,
 				  buf, ESD_USB_RX_BUFFER_SIZE,
 				  esd_usb_read_bulk_callback, dev);
 		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -903,7 +899,7 @@ static netdev_tx_t esd_usb_start_xmit(struct sk_buff *skb,
 	/* hnd must not be 0 - MSB is stripped in txdone handling */
 	msg->tx.hnd = BIT(31) | i; /* returned in TX done message */
 
-	usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
+	usb_fill_bulk_urb(urb, dev->udev, dev->tx_pipe, buf,
 			  msg->hdr.len * sizeof(u32), /* convert to # of bytes */
 			  esd_usb_write_bulk_callback, context);
 
@@ -1298,10 +1294,16 @@ static int esd_usb_probe_one_net(struct usb_interface *intf, int index)
 static int esd_usb_probe(struct usb_interface *intf,
 			 const struct usb_device_id *id)
 {
+	struct usb_endpoint_descriptor *ep_in, *ep_out;
 	struct esd_usb *dev;
 	union esd_usb_msg *msg;
 	int i, err;
 
+	err = usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out,
+					NULL, NULL);
+	if (err)
+		return err;
+
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
 		err = -ENOMEM;
@@ -1309,6 +1311,8 @@ static int esd_usb_probe(struct usb_interface *intf,
 	}
 
 	dev->udev = interface_to_usbdev(intf);
+	dev->rx_pipe = usb_rcvbulkpipe(dev->udev, ep_in->bEndpointAddress);
+	dev->tx_pipe = usb_sndbulkpipe(dev->udev, ep_out->bEndpointAddress);
 
 	init_usb_anchor(&dev->rx_submitted);
 
-- 
2.34.1
Re: [PATCH v2] can: esd_usb: add endpoint type validation
Posted by Marc Kleine-Budde 1 month ago 
On 13.02.2026 20:39:27, Ziyi Guo wrote:
> esd_usb_probe() constructs bulk pipes for two endpoints without
> verifying their transfer types:
>
>   - usb_rcvbulkpipe(dev->udev, 1) for RX (version reply, async RX data)
>   - usb_sndbulkpipe(dev->udev, 2) for TX (version query, CAN frames)
>
> A malformed USB device can present these endpoints with transfer types
> that differ from what the driver assumes, triggering the WARNING in
> usb_submit_urb().
>
> Use usb_find_common_endpoints() to discover and validate the first
> bulk IN and bulk OUT endpoints at probe time, before any allocation.
> Found pipes are saved to struct esd_usb and code uses them directly
> instead of making pipes in place.
>
> Similar to
> - commit 136bed0bfd3b ("can: mcba_usb: properly check endpoint type")
>   which established the usb_find_common_endpoints() + stored pipes
>   pattern for CAN USB drivers.
>
> Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
> Suggested-by: Vincent Mailhol <mailhol@kernel.org>
> Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>

Applied to linux-can

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH v2] can: esd_usb: add endpoint type validation
Posted by Vincent Mailhol 1 month, 1 week ago 
On 13/02/2026 at 21:39, Ziyi Guo wrote:
> esd_usb_probe() constructs bulk pipes for two endpoints without
> verifying their transfer types:
> 
>   - usb_rcvbulkpipe(dev->udev, 1) for RX (version reply, async RX data)
>   - usb_sndbulkpipe(dev->udev, 2) for TX (version query, CAN frames)
> 
> A malformed USB device can present these endpoints with transfer types
> that differ from what the driver assumes, triggering the WARNING in
> usb_submit_urb().
> 
> Use usb_find_common_endpoints() to discover and validate the first
> bulk IN and bulk OUT endpoints at probe time, before any allocation.
> Found pipes are saved to struct esd_usb and code uses them directly
> instead of making pipes in place.
> 
> Similar to
> - commit 136bed0bfd3b ("can: mcba_usb: properly check endpoint type")
>   which established the usb_find_common_endpoints() + stored pipes
>   pattern for CAN USB drivers.
> 
> Fixes: 96d8e90382dc ("can: Add driver for esd CAN-USB/2 device")
> Suggested-by: Vincent Mailhol <mailhol@kernel.org>
> Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>

Thanks for this v2.

Reviewed-by: Vincent Mailhol <mailhol@kernel.org>



Yours sincerely,
Vincent Mailhol

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.