From fa7b514d2b2894e052b8e94c7a29feb98e90093f Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Sat, 19 Mar 2022 08:31:28 -0700 Subject: can: mcp251xfd: mcp251xfd_register_get_dev_id(): fix return of error value Clang static analysis reports this issue: | mcp251xfd-core.c:1813:7: warning: The left operand | of '&' is a garbage value | FIELD_GET(MCP251XFD_REG_DEVID_ID_MASK, dev_id), | ^ ~~~~~~ dev_id is set in a successful call to mcp251xfd_register_get_dev_id(). Though the status of calls made by mcp251xfd_register_get_dev_id() are checked and handled, their status' are not returned. So return err. Fixes: 55e5b97f003e ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN") Link: https://lore.kernel.org/all/20220319153128.2164120-1-trix@redhat.com Signed-off-by: Tom Rix Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 325024be7b04..f9dd8fdba12b 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -1786,7 +1786,7 @@ mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, u32 *dev_id, out_kfree_buf_rx: kfree(buf_rx); - return 0; + return err; } #define MCP251XFD_QUIRK_ACTIVE(quirk) \ -- cgit v1.2.3 From 2e8e79c416aae1de224c0f1860f2e3350fa171f8 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 17 Mar 2022 08:57:35 +0100 Subject: can: m_can: m_can_tx_handler(): fix use after free of skb can_put_echo_skb() will clone skb then free the skb. Move the can_put_echo_skb() for the m_can version 3.0.x directly before the start of the xmit in hardware, similar to the 3.1.x branch. Fixes: 80646733f11c ("can: m_can: update to support CAN FD features") Link: https://lore.kernel.org/all/20220317081305.739554-1-mkl@pengutronix.de Cc: stable@vger.kernel.org Reported-by: Hangyu Hua Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 1a4b56f6fa8c..b3b5bc1c803b 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1637,8 +1637,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) if (err) goto out_fail; - can_put_echo_skb(skb, dev, 0, 0); - if (cdev->can.ctrlmode & CAN_CTRLMODE_FD) { cccr = m_can_read(cdev, M_CAN_CCCR); cccr &= ~CCCR_CMR_MASK; @@ -1655,6 +1653,9 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev) m_can_write(cdev, M_CAN_CCCR, cccr); } m_can_write(cdev, M_CAN_TXBTIE, 0x1); + + can_put_echo_skb(skb, dev, 0, 0); + m_can_write(cdev, M_CAN_TXBAR, 0x1); /* End of xmit function for version 3.0.x */ } else { -- cgit v1.2.3 From c70222752228a62135cee3409dccefd494a24646 Mon Sep 17 00:00:00 2001 From: Hangyu Hua Date: Mon, 28 Feb 2022 16:36:39 +0800 Subject: can: ems_usb: ems_usb_start_xmit(): fix double dev_kfree_skb() in error path There is no need to call dev_kfree_skb() when usb_submit_urb() fails beacause can_put_echo_skb() deletes the original skb and can_free_echo_skb() deletes the cloned skb. Link: https://lore.kernel.org/all/20220228083639.38183-1-hbh25y@gmail.com Fixes: 702171adeed3 ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface") Cc: stable@vger.kernel.org Cc: Sebastian Haas Signed-off-by: Hangyu Hua Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/ems_usb.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers') diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c index 7bedceffdfa3..bbec3311d893 100644 --- a/drivers/net/can/usb/ems_usb.c +++ b/drivers/net/can/usb/ems_usb.c @@ -819,7 +819,6 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne usb_unanchor_urb(urb); usb_free_coherent(dev->udev, size, buf, urb->transfer_dma); - dev_kfree_skb(skb); atomic_dec(&dev->active_tx_urbs); -- cgit v1.2.3 From 3d3925ff6433f98992685a9679613a2cc97f3ce2 Mon Sep 17 00:00:00 2001 From: Hangyu Hua Date: Fri, 11 Mar 2022 16:06:14 +0800 Subject: can: usb_8dev: usb_8dev_start_xmit(): fix double dev_kfree_skb() in error path There is no need to call dev_kfree_skb() when usb_submit_urb() fails because can_put_echo_skb() deletes original skb and can_free_echo_skb() deletes the cloned skb. Fixes: 0024d8ad1639 ("can: usb_8dev: Add support for USB2CAN interface from 8 devices") Link: https://lore.kernel.org/all/20220311080614.45229-1-hbh25y@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Hangyu Hua Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'drivers') diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c index 431af1ec1e3c..b638604bf1ee 100644 --- a/drivers/net/can/usb/usb_8dev.c +++ b/drivers/net/can/usb/usb_8dev.c @@ -663,9 +663,20 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb, atomic_inc(&priv->active_tx_urbs); err = usb_submit_urb(urb, GFP_ATOMIC); - if (unlikely(err)) - goto failed; - else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) + if (unlikely(err)) { + can_free_echo_skb(netdev, context->echo_index, NULL); + + usb_unanchor_urb(urb); + usb_free_coherent(priv->udev, size, buf, urb->transfer_dma); + + atomic_dec(&priv->active_tx_urbs); + + if (err == -ENODEV) + netif_device_detach(netdev); + else + netdev_warn(netdev, "failed tx_urb %d\n", err); + stats->tx_dropped++; + } else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) /* Slow down tx path */ netif_stop_queue(netdev); @@ -684,19 +695,6 @@ nofreecontext: return NETDEV_TX_BUSY; -failed: - can_free_echo_skb(netdev, context->echo_index, NULL); - - usb_unanchor_urb(urb); - usb_free_coherent(priv->udev, size, buf, urb->transfer_dma); - - atomic_dec(&priv->active_tx_urbs); - - if (err == -ENODEV) - netif_device_detach(netdev); - else - netdev_warn(netdev, "failed tx_urb %d\n", err); - nomembuf: usb_free_urb(urb); -- cgit v1.2.3 From 04c9b00ba83594a29813d6b1fb8fdc93a3915174 Mon Sep 17 00:00:00 2001 From: Hangyu Hua Date: Fri, 11 Mar 2022 16:02:08 +0800 Subject: can: mcba_usb: mcba_usb_start_xmit(): fix double dev_kfree_skb in error path There is no need to call dev_kfree_skb() when usb_submit_urb() fails because can_put_echo_skb() deletes original skb and can_free_echo_skb() deletes the cloned skb. Fixes: 51f3baad7de9 ("can: mcba_usb: Add support for Microchip CAN BUS Analyzer") Link: https://lore.kernel.org/all/20220311080208.45047-1-hbh25y@gmail.com Signed-off-by: Hangyu Hua Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/mcba_usb.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers') diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c index 77bddff86252..7c198eb5bc9c 100644 --- a/drivers/net/can/usb/mcba_usb.c +++ b/drivers/net/can/usb/mcba_usb.c @@ -364,7 +364,6 @@ static netdev_tx_t mcba_usb_start_xmit(struct sk_buff *skb, xmit_failed: can_free_echo_skb(priv->netdev, ctx->ndx, NULL); mcba_usb_free_ctx(ctx); - dev_kfree_skb(skb); stats->tx_dropped++; return NETDEV_TX_OK; -- cgit v1.2.3 From 136bed0bfd3bc9c95c88aafff2d22ecb3a919f23 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Sun, 13 Mar 2022 13:09:03 +0300 Subject: can: mcba_usb: properly check endpoint type Syzbot reported warning in usb_submit_urb() which is caused by wrong endpoint type. We should check that in endpoint is actually present to prevent this warning. Found pipes are now saved to struct mcba_priv and code uses them directly instead of making pipes in place. Fail log: | usb 5-1: BOGUS urb xfer, pipe 3 != type 1 | WARNING: CPU: 1 PID: 49 at drivers/usb/core/urb.c:502 usb_submit_urb+0xed2/0x18a0 drivers/usb/core/urb.c:502 | Modules linked in: | CPU: 1 PID: 49 Comm: kworker/1:2 Not tainted 5.17.0-rc6-syzkaller-00184-g38f80f42147f #0 | Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 | Workqueue: usb_hub_wq hub_event | RIP: 0010:usb_submit_urb+0xed2/0x18a0 drivers/usb/core/urb.c:502 | ... | Call Trace: | | mcba_usb_start drivers/net/can/usb/mcba_usb.c:662 [inline] | mcba_usb_probe+0x8a3/0xc50 drivers/net/can/usb/mcba_usb.c:858 | usb_probe_interface+0x315/0x7f0 drivers/usb/core/driver.c:396 | call_driver_probe drivers/base/dd.c:517 [inline] Fixes: 51f3baad7de9 ("can: mcba_usb: Add support for Microchip CAN BUS Analyzer") Link: https://lore.kernel.org/all/20220313100903.10868-1-paskripkin@gmail.com Reported-and-tested-by: syzbot+3bc1dce0cc0052d60fde@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin Reviewed-by: Vincent Mailhol Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/mcba_usb.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c index 7c198eb5bc9c..c45a814e1de2 100644 --- a/drivers/net/can/usb/mcba_usb.c +++ b/drivers/net/can/usb/mcba_usb.c @@ -33,10 +33,6 @@ #define MCBA_USB_RX_BUFF_SIZE 64 #define MCBA_USB_TX_BUFF_SIZE (sizeof(struct mcba_usb_msg)) -/* MCBA endpoint numbers */ -#define MCBA_USB_EP_IN 1 -#define MCBA_USB_EP_OUT 1 - /* Microchip command id */ #define MBCA_CMD_RECEIVE_MESSAGE 0xE3 #define MBCA_CMD_I_AM_ALIVE_FROM_CAN 0xF5 @@ -83,6 +79,8 @@ struct mcba_priv { atomic_t free_ctx_cnt; void *rxbuf[MCBA_MAX_RX_URBS]; dma_addr_t rxbuf_dma[MCBA_MAX_RX_URBS]; + int rx_pipe; + int tx_pipe; }; /* CAN frame */ @@ -268,10 +266,8 @@ static netdev_tx_t mcba_usb_xmit(struct mcba_priv *priv, memcpy(buf, usb_msg, MCBA_USB_TX_BUFF_SIZE); - usb_fill_bulk_urb(urb, priv->udev, - usb_sndbulkpipe(priv->udev, MCBA_USB_EP_OUT), buf, - MCBA_USB_TX_BUFF_SIZE, mcba_usb_write_bulk_callback, - ctx); + usb_fill_bulk_urb(urb, priv->udev, priv->tx_pipe, buf, MCBA_USB_TX_BUFF_SIZE, + mcba_usb_write_bulk_callback, ctx); urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; usb_anchor_urb(urb, &priv->tx_submitted); @@ -607,7 +603,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb) resubmit_urb: usb_fill_bulk_urb(urb, priv->udev, - usb_rcvbulkpipe(priv->udev, MCBA_USB_EP_OUT), + priv->rx_pipe, urb->transfer_buffer, MCBA_USB_RX_BUFF_SIZE, mcba_usb_read_bulk_callback, priv); @@ -652,7 +648,7 @@ static int mcba_usb_start(struct mcba_priv *priv) urb->transfer_dma = buf_dma; usb_fill_bulk_urb(urb, priv->udev, - usb_rcvbulkpipe(priv->udev, MCBA_USB_EP_IN), + priv->rx_pipe, buf, MCBA_USB_RX_BUFF_SIZE, mcba_usb_read_bulk_callback, priv); urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; @@ -806,6 +802,13 @@ static int mcba_usb_probe(struct usb_interface *intf, struct mcba_priv *priv; int err; struct usb_device *usbdev = interface_to_usbdev(intf); + struct usb_endpoint_descriptor *in, *out; + + err = usb_find_common_endpoints(intf->cur_altsetting, &in, &out, NULL, NULL); + if (err) { + dev_err(&intf->dev, "Can't find endpoints\n"); + return err; + } netdev = alloc_candev(sizeof(struct mcba_priv), MCBA_MAX_TX_URBS); if (!netdev) { @@ -851,6 +854,9 @@ static int mcba_usb_probe(struct usb_interface *intf, goto cleanup_free_candev; } + priv->rx_pipe = usb_rcvbulkpipe(priv->udev, in->bEndpointAddress); + priv->tx_pipe = usb_sndbulkpipe(priv->udev, out->bEndpointAddress); + devm_can_led_init(netdev); /* Start USB dev only if we have successfully registered CAN device */ -- cgit v1.2.3 From 50d34a0d151dc7abbdbec781bd7f09f2b3cbf01a Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 29 Mar 2022 21:29:43 +0200 Subject: can: gs_usb: gs_make_candev(): fix memory leak for devices with extended bit timing configuration Some CAN-FD capable devices offer extended bit timing information for the data bit timing. The information must be read with an USB control message. The memory for this message is allocated but not free()ed (in the non error case). This patch adds the missing free. Fixes: 6679f4c5e5a6 ("can: gs_usb: add extended bt_const feature") Link: https://lore.kernel.org/all/20220329193450.659726-1-mkl@pengutronix.de Reported-by: syzbot+4d0ae90a195b269f102d@syzkaller.appspotmail.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers') diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 67408e316062..b29ba9138866 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -1092,6 +1092,8 @@ static struct gs_can *gs_make_candev(unsigned int channel, dev->data_bt_const.brp_inc = le32_to_cpu(bt_const_extended->dbrp_inc); dev->can.data_bittiming_const = &dev->data_bt_const; + + kfree(bt_const_extended); } SET_NETDEV_DEV(netdev, &intf->dev); -- cgit v1.2.3