From ee1dfbdd8b4b6de85e96ae2059dc9c1bdb6b49b5 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 21 Nov 2024 11:08:25 +0100 Subject: [PATCH 01/15] can: dev: can_set_termination(): allow sleeping GPIOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In commit 6e86a1543c37 ("can: dev: provide optional GPIO based termination support") GPIO based termination support was added. For no particular reason that patch uses gpiod_set_value() to set the GPIO. This leads to the following warning, if the systems uses a sleeping GPIO, i.e. behind an I2C port expander: | WARNING: CPU: 0 PID: 379 at /drivers/gpio/gpiolib.c:3496 gpiod_set_value+0x50/0x6c | CPU: 0 UID: 0 PID: 379 Comm: ip Not tainted 6.11.0-20241016-1 #1 823affae360cc91126e4d316d7a614a8bf86236c Replace gpiod_set_value() by gpiod_set_value_cansleep() to allow the use of sleeping GPIOs. Cc: Nicolai Buchwitz Cc: Lino Sanfilippo Cc: stable@vger.kernel.org Reported-by: Leonard Göhrs Tested-by: Leonard Göhrs Fixes: 6e86a1543c37 ("can: dev: provide optional GPIO based termination support") Link: https://patch.msgid.link/20241121-dev-fix-can_set_termination-v1-1-41fa6e29216d@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c index 6792c14fd7eb..681643ab3780 100644 --- a/drivers/net/can/dev/dev.c +++ b/drivers/net/can/dev/dev.c @@ -468,7 +468,7 @@ static int can_set_termination(struct net_device *ndev, u16 term) else set = 0; - gpiod_set_value(priv->termination_gpio, set); + gpiod_set_value_cansleep(priv->termination_gpio, set); return 0; } From 889b2ae9139a87b3390f7003cb1bb3d65bf90a26 Mon Sep 17 00:00:00 2001 From: Alexander Kozhinov Date: Fri, 18 Oct 2024 23:24:26 +0200 Subject: [PATCH 02/15] can: gs_usb: add usb endpoint address detection at driver probe step There is an approach made to implement gs_usb firmware/driver based on Zephyr RTOS. It was found that USB stack of Zephyr RTOS overwrites USB EP addresses, if they have different last 4 bytes in absence of other endpoints. For example in case of gs_usb candlelight firmware EP-IN is 0x81 and EP-OUT 0x02. If there are no additional USB endpoints, Zephyr RTOS will overwrite EP-OUT to 0x01. More information can be found in the discussion with Zephyr RTOS USB stack maintainer here: https://github.com/zephyrproject-rtos/zephyr/issues/67812 There are already two different gs_usb FW driver implementations based on Zephyr RTOS: 1. https://github.com/CANnectivity/cannectivity (by: https://github.com/henrikbrixandersen) 2. https://github.com/zephyrproject-rtos/zephyr/compare/main...KozhinovAlexander:zephyr:gs_usb (by: https://github.com/KozhinovAlexander) At the moment both Zephyr RTOS implementations use dummy USB endpoint, to overcome described USB stack behavior from Zephyr itself. Since Zephyr RTOS is intended to be used on microcontrollers with very constrained amount of resources (ROM, RAM) and additional endpoint requires memory, it is more convenient to update the gs_usb driver in the Linux kernel. To fix this problem, update the gs_usb driver from using hard coded endpoint numbers to evaluate the endpoint descriptors and use the endpoints provided there. Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices") Reviewed-by: Vincent Mailhol Signed-off-by: Alexander Kozhinov Link: https://patch.msgid.link/20241018212450.31746-1-ak.alexander.kozhinov@gmail.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index bc86e9b329fd..b6f4de375df7 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -43,9 +43,6 @@ #define USB_XYLANTA_SAINT3_VENDOR_ID 0x16d0 #define USB_XYLANTA_SAINT3_PRODUCT_ID 0x0f30 -#define GS_USB_ENDPOINT_IN 1 -#define GS_USB_ENDPOINT_OUT 2 - /* Timestamp 32 bit timer runs at 1 MHz (1 µs tick). Worker accounts * for timer overflow (will be after ~71 minutes) */ @@ -336,6 +333,9 @@ struct gs_usb { unsigned int hf_size_rx; u8 active_channels; + + unsigned int pipe_in; + unsigned int pipe_out; }; /* 'allocate' a tx context. @@ -687,7 +687,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb) resubmit_urb: usb_fill_bulk_urb(urb, parent->udev, - usb_rcvbulkpipe(parent->udev, GS_USB_ENDPOINT_IN), + parent->pipe_in, hf, dev->parent->hf_size_rx, gs_usb_receive_bulk_callback, parent); @@ -819,7 +819,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb, } usb_fill_bulk_urb(urb, dev->udev, - usb_sndbulkpipe(dev->udev, GS_USB_ENDPOINT_OUT), + dev->parent->pipe_out, hf, dev->hf_size_tx, gs_usb_xmit_callback, txc); @@ -925,8 +925,7 @@ static int gs_can_open(struct net_device *netdev) /* fill, anchor, and submit rx urb */ usb_fill_bulk_urb(urb, dev->udev, - usb_rcvbulkpipe(dev->udev, - GS_USB_ENDPOINT_IN), + dev->parent->pipe_in, buf, dev->parent->hf_size_rx, gs_usb_receive_bulk_callback, parent); @@ -1413,6 +1412,7 @@ static int gs_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_device *udev = interface_to_usbdev(intf); + struct usb_endpoint_descriptor *ep_in, *ep_out; struct gs_host_frame *hf; struct gs_usb *parent; struct gs_host_config hconf = { @@ -1422,6 +1422,13 @@ static int gs_usb_probe(struct usb_interface *intf, unsigned int icount, i; int rc; + rc = usb_find_common_endpoints(intf->cur_altsetting, + &ep_in, &ep_out, NULL, NULL); + if (rc) { + dev_err(&intf->dev, "Required endpoints not found\n"); + return rc; + } + /* send host config */ rc = usb_control_msg_send(udev, 0, GS_USB_BREQ_HOST_FORMAT, @@ -1466,6 +1473,10 @@ static int gs_usb_probe(struct usb_interface *intf, usb_set_intfdata(intf, parent); parent->udev = udev; + /* store the detected endpoints */ + parent->pipe_in = usb_rcvbulkpipe(parent->udev, ep_in->bEndpointAddress); + parent->pipe_out = usb_sndbulkpipe(parent->udev, ep_out->bEndpointAddress); + for (i = 0; i < icount; i++) { unsigned int hf_size_rx = 0; From 9e66242504f49e17481d8e197730faba7d99c934 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:42 +0100 Subject: [PATCH 03/15] can: c_can: c_can_handle_bus_err(): update statistics if skb allocation fails Ensure that the statistics are always updated, even if the skb allocation fails. Fixes: 4d6d26537940 ("can: c_can: fix {rx,tx}_errors statistics") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-2-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/c_can/c_can_main.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/net/can/c_can/c_can_main.c b/drivers/net/can/c_can/c_can_main.c index 511615dc3341..cc371d0c9f3c 100644 --- a/drivers/net/can/c_can/c_can_main.c +++ b/drivers/net/can/c_can/c_can_main.c @@ -1014,49 +1014,57 @@ static int c_can_handle_bus_err(struct net_device *dev, /* propagate the error condition to the CAN stack */ skb = alloc_can_err_skb(dev, &cf); - if (unlikely(!skb)) - return 0; /* check for 'last error code' which tells us the * type of the last error to occur on the CAN bus */ - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; + if (likely(skb)) + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; switch (lec_type) { case LEC_STUFF_ERROR: netdev_dbg(dev, "stuff error\n"); - cf->data[2] |= CAN_ERR_PROT_STUFF; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_STUFF; stats->rx_errors++; break; case LEC_FORM_ERROR: netdev_dbg(dev, "form error\n"); - cf->data[2] |= CAN_ERR_PROT_FORM; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_FORM; stats->rx_errors++; break; case LEC_ACK_ERROR: netdev_dbg(dev, "ack error\n"); - cf->data[3] = CAN_ERR_PROT_LOC_ACK; + if (likely(skb)) + cf->data[3] = CAN_ERR_PROT_LOC_ACK; stats->tx_errors++; break; case LEC_BIT1_ERROR: netdev_dbg(dev, "bit1 error\n"); - cf->data[2] |= CAN_ERR_PROT_BIT1; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_BIT1; stats->tx_errors++; break; case LEC_BIT0_ERROR: netdev_dbg(dev, "bit0 error\n"); - cf->data[2] |= CAN_ERR_PROT_BIT0; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_BIT0; stats->tx_errors++; break; case LEC_CRC_ERROR: netdev_dbg(dev, "CRC error\n"); - cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ; + if (likely(skb)) + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ; stats->rx_errors++; break; default: break; } + if (unlikely(!skb)) + return 0; + netif_receive_skb(skb); return 1; } From ee6bf3677ae03569d833795064e17f605c2163c7 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:43 +0100 Subject: [PATCH 04/15] can: sun4i_can: sun4i_can_err(): call can_change_state() even if cf is NULL Call the function can_change_state() if the allocation of the skb fails, as it handles the cf parameter when it is null. Additionally, this ensures that the statistics related to state error counters (i. e. warning, passive, and bus-off) are updated. Fixes: 0738eff14d81 ("can: Allwinner A10/A20 CAN Controller support - Kernel module") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-3-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/sun4i_can.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c index 360158c295d3..17f94cca93fb 100644 --- a/drivers/net/can/sun4i_can.c +++ b/drivers/net/can/sun4i_can.c @@ -629,10 +629,10 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, u8 status) tx_state = txerr >= rxerr ? state : 0; rx_state = txerr <= rxerr ? state : 0; - if (likely(skb)) - can_change_state(dev, cf, tx_state, rx_state); - else - priv->can.state = state; + /* The skb allocation might fail, but can_change_state() + * handles cf == NULL. + */ + can_change_state(dev, cf, tx_state, rx_state); if (state == CAN_STATE_BUS_OFF) can_bus_off(dev); } From 9ad86d377ef4a19c75a9c639964879a5b25a433b Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:45 +0100 Subject: [PATCH 05/15] can: hi311x: hi3110_can_ist(): fix potential use-after-free The commit a22bd630cfff ("can: hi311x: do not report txerr and rxerr during bus-off") removed the reporting of rxerr and txerr even in case of correct operation (i. e. not bus-off). The error count information added to the CAN frame after netif_rx() is a potential use after free, since there is no guarantee that the skb is in the same state. It might be freed or reused. Fix the issue by postponing the netif_rx() call in case of txerr and rxerr reporting. Fixes: a22bd630cfff ("can: hi311x: do not report txerr and rxerr during bus-off") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-5-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/hi311x.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c index 148d974ebb21..b67464df25ff 100644 --- a/drivers/net/can/spi/hi311x.c +++ b/drivers/net/can/spi/hi311x.c @@ -671,9 +671,9 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) tx_state = txerr >= rxerr ? new_state : 0; rx_state = txerr <= rxerr ? new_state : 0; can_change_state(net, cf, tx_state, rx_state); - netif_rx(skb); if (new_state == CAN_STATE_BUS_OFF) { + netif_rx(skb); can_bus_off(net); if (priv->can.restart_ms == 0) { priv->force_quit = 1; @@ -684,6 +684,7 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) cf->can_id |= CAN_ERR_CNT; cf->data[6] = txerr; cf->data[7] = rxerr; + netif_rx(skb); } } From ef5034aed9e03c649386f50e67a0867062d00d7f Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:46 +0100 Subject: [PATCH 06/15] can: hi311x: hi3110_can_ist(): update state error statistics if skb allocation fails Ensure that the statistics related to state error counters (i. e. warning, passive, and bus-off) are updated even in case the skb allocation fails. Additionally, also handle bus-off state is now. Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-6-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/hi311x.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c index b67464df25ff..25d9b32f5701 100644 --- a/drivers/net/can/spi/hi311x.c +++ b/drivers/net/can/spi/hi311x.c @@ -663,8 +663,6 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) u8 rxerr, txerr; skb = alloc_can_err_skb(net, &cf); - if (!skb) - break; txerr = hi3110_read(spi, HI3110_READ_TEC); rxerr = hi3110_read(spi, HI3110_READ_REC); @@ -673,14 +671,15 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) can_change_state(net, cf, tx_state, rx_state); if (new_state == CAN_STATE_BUS_OFF) { - netif_rx(skb); + if (skb) + netif_rx(skb); can_bus_off(net); if (priv->can.restart_ms == 0) { priv->force_quit = 1; hi3110_hw_sleep(spi); break; } - } else { + } else if (skb) { cf->can_id |= CAN_ERR_CNT; cf->data[6] = txerr; cf->data[7] = rxerr; From 988d4222bf9039a875a3d48f2fe35c317831ff68 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:47 +0100 Subject: [PATCH 07/15] can: m_can: m_can_handle_lec_err(): fix {rx,tx}_errors statistics The m_can_handle_lec_err() function was incorrectly incrementing only the receive error counter, even in cases of bit or acknowledgment errors that occur during transmission. Fix the issue by incrementing the appropriate counter based on the type of error. Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-7-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 16e9e7d7527d..533bcb77c9f9 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -695,47 +695,60 @@ static int m_can_handle_lec_err(struct net_device *dev, u32 timestamp = 0; cdev->can.can_stats.bus_error++; - stats->rx_errors++; /* propagate the error condition to the CAN stack */ skb = alloc_can_err_skb(dev, &cf); - if (unlikely(!skb)) - return 0; /* check for 'last error code' which tells us the * type of the last error to occur on the CAN bus */ - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; + if (likely(skb)) + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; switch (lec_type) { case LEC_STUFF_ERROR: netdev_dbg(dev, "stuff error\n"); - cf->data[2] |= CAN_ERR_PROT_STUFF; + stats->rx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_STUFF; break; case LEC_FORM_ERROR: netdev_dbg(dev, "form error\n"); - cf->data[2] |= CAN_ERR_PROT_FORM; + stats->rx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_FORM; break; case LEC_ACK_ERROR: netdev_dbg(dev, "ack error\n"); - cf->data[3] = CAN_ERR_PROT_LOC_ACK; + stats->tx_errors++; + if (likely(skb)) + cf->data[3] = CAN_ERR_PROT_LOC_ACK; break; case LEC_BIT1_ERROR: netdev_dbg(dev, "bit1 error\n"); - cf->data[2] |= CAN_ERR_PROT_BIT1; + stats->tx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_BIT1; break; case LEC_BIT0_ERROR: netdev_dbg(dev, "bit0 error\n"); - cf->data[2] |= CAN_ERR_PROT_BIT0; + stats->tx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_BIT0; break; case LEC_CRC_ERROR: netdev_dbg(dev, "CRC error\n"); - cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ; + stats->rx_errors++; + if (likely(skb)) + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ; break; default: break; } + if (unlikely(!skb)) + return 0; + if (cdev->is_peripheral) timestamp = m_can_get_timestamp(cdev); From bb03d568bb21b4afe7935d1943bcf68ddea3ea45 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:48 +0100 Subject: [PATCH 08/15] can: ifi_canfd: ifi_canfd_handle_lec_err(): fix {rx,tx}_errors statistics The ifi_canfd_handle_lec_err() function was incorrectly incrementing only the receive error counter, even in cases of bit or acknowledgment errors that occur during transmission. Fix the issue by incrementing the appropriate counter based on the type of error. Fixes: 5bbd655a8bd0 ("can: ifi: Add more detailed error reporting") Signed-off-by: Dario Binacchi Reviewed-by: Marek Vasut Link: https://patch.msgid.link/20241122221650.633981-8-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/ifi_canfd/ifi_canfd.c | 58 ++++++++++++++++++--------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c index d32b10900d2f..c86b57d47085 100644 --- a/drivers/net/can/ifi_canfd/ifi_canfd.c +++ b/drivers/net/can/ifi_canfd/ifi_canfd.c @@ -390,36 +390,55 @@ static int ifi_canfd_handle_lec_err(struct net_device *ndev) return 0; priv->can.can_stats.bus_error++; - stats->rx_errors++; /* Propagate the error condition to the CAN stack. */ skb = alloc_can_err_skb(ndev, &cf); - if (unlikely(!skb)) - return 0; /* Read the error counter register and check for new errors. */ - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; + if (likely(skb)) + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; - if (errctr & IFI_CANFD_ERROR_CTR_OVERLOAD_FIRST) - cf->data[2] |= CAN_ERR_PROT_OVERLOAD; + if (errctr & IFI_CANFD_ERROR_CTR_OVERLOAD_FIRST) { + stats->rx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_OVERLOAD; + } - if (errctr & IFI_CANFD_ERROR_CTR_ACK_ERROR_FIRST) - cf->data[3] = CAN_ERR_PROT_LOC_ACK; + if (errctr & IFI_CANFD_ERROR_CTR_ACK_ERROR_FIRST) { + stats->tx_errors++; + if (likely(skb)) + cf->data[3] = CAN_ERR_PROT_LOC_ACK; + } - if (errctr & IFI_CANFD_ERROR_CTR_BIT0_ERROR_FIRST) - cf->data[2] |= CAN_ERR_PROT_BIT0; + if (errctr & IFI_CANFD_ERROR_CTR_BIT0_ERROR_FIRST) { + stats->tx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_BIT0; + } - if (errctr & IFI_CANFD_ERROR_CTR_BIT1_ERROR_FIRST) - cf->data[2] |= CAN_ERR_PROT_BIT1; + if (errctr & IFI_CANFD_ERROR_CTR_BIT1_ERROR_FIRST) { + stats->tx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_BIT1; + } - if (errctr & IFI_CANFD_ERROR_CTR_STUFF_ERROR_FIRST) - cf->data[2] |= CAN_ERR_PROT_STUFF; + if (errctr & IFI_CANFD_ERROR_CTR_STUFF_ERROR_FIRST) { + stats->rx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_STUFF; + } - if (errctr & IFI_CANFD_ERROR_CTR_CRC_ERROR_FIRST) - cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ; + if (errctr & IFI_CANFD_ERROR_CTR_CRC_ERROR_FIRST) { + stats->rx_errors++; + if (likely(skb)) + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ; + } - if (errctr & IFI_CANFD_ERROR_CTR_FORM_ERROR_FIRST) - cf->data[2] |= CAN_ERR_PROT_FORM; + if (errctr & IFI_CANFD_ERROR_CTR_FORM_ERROR_FIRST) { + stats->rx_errors++; + if (likely(skb)) + cf->data[2] |= CAN_ERR_PROT_FORM; + } /* Reset the error counter, ack the IRQ and re-enable the counter. */ writel(IFI_CANFD_ERROR_CTR_ER_RESET, priv->base + IFI_CANFD_ERROR_CTR); @@ -427,6 +446,9 @@ static int ifi_canfd_handle_lec_err(struct net_device *ndev) priv->base + IFI_CANFD_INTERRUPT); writel(IFI_CANFD_ERROR_CTR_ER_ENABLE, priv->base + IFI_CANFD_ERROR_CTR); + if (unlikely(!skb)) + return 0; + netif_receive_skb(skb); return 1; From 3e4645931655776e757f9fb5ae29371cd7cb21a2 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:49 +0100 Subject: [PATCH 09/15] can: hi311x: hi3110_can_ist(): fix {rx,tx}_errors statistics The hi3110_can_ist() function was incorrectly incrementing only the receive error counter, even in cases of bit or acknowledgment errors that occur during transmission. The fix the issue by incrementing the appropriate counter based on the type of error. Fixes: 57e83fb9b746 ("can: hi311x: Add Holt HI-311x CAN driver") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-9-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/hi311x.c | 45 ++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c index 25d9b32f5701..09ae218315d7 100644 --- a/drivers/net/can/spi/hi311x.c +++ b/drivers/net/can/spi/hi311x.c @@ -696,27 +696,38 @@ static irqreturn_t hi3110_can_ist(int irq, void *dev_id) /* Check for protocol errors */ if (eflag & HI3110_ERR_PROTOCOL_MASK) { skb = alloc_can_err_skb(net, &cf); - if (!skb) - break; + if (skb) + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; priv->can.can_stats.bus_error++; - priv->net->stats.rx_errors++; - if (eflag & HI3110_ERR_BITERR) - cf->data[2] |= CAN_ERR_PROT_BIT; - else if (eflag & HI3110_ERR_FRMERR) - cf->data[2] |= CAN_ERR_PROT_FORM; - else if (eflag & HI3110_ERR_STUFERR) - cf->data[2] |= CAN_ERR_PROT_STUFF; - else if (eflag & HI3110_ERR_CRCERR) - cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ; - else if (eflag & HI3110_ERR_ACKERR) - cf->data[3] |= CAN_ERR_PROT_LOC_ACK; + if (eflag & HI3110_ERR_BITERR) { + priv->net->stats.tx_errors++; + if (skb) + cf->data[2] |= CAN_ERR_PROT_BIT; + } else if (eflag & HI3110_ERR_FRMERR) { + priv->net->stats.rx_errors++; + if (skb) + cf->data[2] |= CAN_ERR_PROT_FORM; + } else if (eflag & HI3110_ERR_STUFERR) { + priv->net->stats.rx_errors++; + if (skb) + cf->data[2] |= CAN_ERR_PROT_STUFF; + } else if (eflag & HI3110_ERR_CRCERR) { + priv->net->stats.rx_errors++; + if (skb) + cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ; + } else if (eflag & HI3110_ERR_ACKERR) { + priv->net->stats.tx_errors++; + if (skb) + cf->data[3] |= CAN_ERR_PROT_LOC_ACK; + } - cf->data[6] = hi3110_read(spi, HI3110_READ_TEC); - cf->data[7] = hi3110_read(spi, HI3110_READ_REC); netdev_dbg(priv->net, "Bus Error\n"); - netif_rx(skb); + if (skb) { + cf->data[6] = hi3110_read(spi, HI3110_READ_TEC); + cf->data[7] = hi3110_read(spi, HI3110_READ_REC); + netif_rx(skb); + } } } From 2c4ef3af4b028a0eaaf378df511d3b425b1df61f Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:50 +0100 Subject: [PATCH 10/15] can: sja1000: sja1000_err(): fix {rx,tx}_errors statistics The sja1000_err() function only incremented the receive error counter and never the transmit error counter, even if the ECC_DIR flag reported that an error had occurred during transmission. Increment the receive/transmit error counter based on the value of the ECC_DIR flag. Fixes: 429da1cc841b ("can: Driver for the SJA1000 CAN controller") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-10-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/sja1000/sja1000.c | 65 ++++++++++++++++++------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c index ddb3247948ad..4d245857ef1c 100644 --- a/drivers/net/can/sja1000/sja1000.c +++ b/drivers/net/can/sja1000/sja1000.c @@ -416,8 +416,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) int ret = 0; skb = alloc_can_err_skb(dev, &cf); - if (skb == NULL) - return -ENOMEM; txerr = priv->read_reg(priv, SJA1000_TXERR); rxerr = priv->read_reg(priv, SJA1000_RXERR); @@ -425,8 +423,11 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) if (isrc & IRQ_DOI) { /* data overrun interrupt */ netdev_dbg(dev, "data overrun interrupt\n"); - cf->can_id |= CAN_ERR_CRTL; - cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; + if (skb) { + cf->can_id |= CAN_ERR_CRTL; + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; + } + stats->rx_over_errors++; stats->rx_errors++; sja1000_write_cmdreg(priv, CMD_CDO); /* clear bit */ @@ -452,7 +453,7 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) else state = CAN_STATE_ERROR_ACTIVE; } - if (state != CAN_STATE_BUS_OFF) { + if (state != CAN_STATE_BUS_OFF && skb) { cf->can_id |= CAN_ERR_CNT; cf->data[6] = txerr; cf->data[7] = rxerr; @@ -460,33 +461,38 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) if (isrc & IRQ_BEI) { /* bus error interrupt */ priv->can.can_stats.bus_error++; - stats->rx_errors++; ecc = priv->read_reg(priv, SJA1000_ECC); + if (skb) { + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; + /* set error type */ + switch (ecc & ECC_MASK) { + case ECC_BIT: + cf->data[2] |= CAN_ERR_PROT_BIT; + break; + case ECC_FORM: + cf->data[2] |= CAN_ERR_PROT_FORM; + break; + case ECC_STUFF: + cf->data[2] |= CAN_ERR_PROT_STUFF; + break; + default: + break; + } - /* set error type */ - switch (ecc & ECC_MASK) { - case ECC_BIT: - cf->data[2] |= CAN_ERR_PROT_BIT; - break; - case ECC_FORM: - cf->data[2] |= CAN_ERR_PROT_FORM; - break; - case ECC_STUFF: - cf->data[2] |= CAN_ERR_PROT_STUFF; - break; - default: - break; + /* set error location */ + cf->data[3] = ecc & ECC_SEG; } - /* set error location */ - cf->data[3] = ecc & ECC_SEG; - /* Error occurred during transmission? */ - if ((ecc & ECC_DIR) == 0) - cf->data[2] |= CAN_ERR_PROT_TX; + if ((ecc & ECC_DIR) == 0) { + stats->tx_errors++; + if (skb) + cf->data[2] |= CAN_ERR_PROT_TX; + } else { + stats->rx_errors++; + } } if (isrc & IRQ_EPI) { /* error passive interrupt */ @@ -502,8 +508,10 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) netdev_dbg(dev, "arbitration lost interrupt\n"); alc = priv->read_reg(priv, SJA1000_ALC); priv->can.can_stats.arbitration_lost++; - cf->can_id |= CAN_ERR_LOSTARB; - cf->data[0] = alc & 0x1f; + if (skb) { + cf->can_id |= CAN_ERR_LOSTARB; + cf->data[0] = alc & 0x1f; + } } if (state != priv->can.state) { @@ -516,6 +524,9 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) can_bus_off(dev); } + if (!skb) + return -ENOMEM; + netif_rx(skb); return ret; From 595a81988a6fe06eb5849e972c8b9cb21c4e0d54 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:51 +0100 Subject: [PATCH 11/15] can: sun4i_can: sun4i_can_err(): fix {rx,tx}_errors statistics The sun4i_can_err() function only incremented the receive error counter and never the transmit error counter, even if the STA_ERR_DIR flag reported that an error had occurred during transmission. Increment the receive/transmit error counter based on the value of the STA_ERR_DIR flag. Fixes: 0738eff14d81 ("can: Allwinner A10/A20 CAN Controller support - Kernel module") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-11-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/sun4i_can.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c index 17f94cca93fb..4311c1f0eafd 100644 --- a/drivers/net/can/sun4i_can.c +++ b/drivers/net/can/sun4i_can.c @@ -579,11 +579,9 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, u8 status) /* bus error interrupt */ netdev_dbg(dev, "bus error interrupt\n"); priv->can.can_stats.bus_error++; - stats->rx_errors++; + ecc = readl(priv->base + SUN4I_REG_STA_ADDR); if (likely(skb)) { - ecc = readl(priv->base + SUN4I_REG_STA_ADDR); - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; switch (ecc & SUN4I_STA_MASK_ERR) { @@ -601,9 +599,15 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, u8 status) >> 16; break; } - /* error occurred during transmission? */ - if ((ecc & SUN4I_STA_ERR_DIR) == 0) + } + + /* error occurred during transmission? */ + if ((ecc & SUN4I_STA_ERR_DIR) == 0) { + if (likely(skb)) cf->data[2] |= CAN_ERR_PROT_TX; + stats->tx_errors++; + } else { + stats->rx_errors++; } } if (isrc & SUN4I_INT_ERR_PASSIVE) { From 72a7e2e74b3075959f05e622bae09b115957dffe Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:52 +0100 Subject: [PATCH 12/15] can: ems_usb: ems_usb_rx_err(): fix {rx,tx}_errors statistics The ems_usb_rx_err() function only incremented the receive error counter and never the transmit error counter, even if the ECC_DIR flag reported that an error had occurred during transmission. Increment the receive/transmit error counter based on the value of the ECC_DIR flag. Fixes: 702171adeed3 ("ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-12-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/ems_usb.c | 58 ++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c index 050c0b49938a..5355bac4dccb 100644 --- a/drivers/net/can/usb/ems_usb.c +++ b/drivers/net/can/usb/ems_usb.c @@ -335,15 +335,14 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg) struct net_device_stats *stats = &dev->netdev->stats; skb = alloc_can_err_skb(dev->netdev, &cf); - if (skb == NULL) - return; if (msg->type == CPC_MSG_TYPE_CAN_STATE) { u8 state = msg->msg.can_state; if (state & SJA1000_SR_BS) { dev->can.state = CAN_STATE_BUS_OFF; - cf->can_id |= CAN_ERR_BUSOFF; + if (skb) + cf->can_id |= CAN_ERR_BUSOFF; dev->can.can_stats.bus_off++; can_bus_off(dev->netdev); @@ -361,44 +360,53 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg) /* bus error interrupt */ dev->can.can_stats.bus_error++; - stats->rx_errors++; - cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; + if (skb) { + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; - switch (ecc & SJA1000_ECC_MASK) { - case SJA1000_ECC_BIT: - cf->data[2] |= CAN_ERR_PROT_BIT; - break; - case SJA1000_ECC_FORM: - cf->data[2] |= CAN_ERR_PROT_FORM; - break; - case SJA1000_ECC_STUFF: - cf->data[2] |= CAN_ERR_PROT_STUFF; - break; - default: - cf->data[3] = ecc & SJA1000_ECC_SEG; - break; + switch (ecc & SJA1000_ECC_MASK) { + case SJA1000_ECC_BIT: + cf->data[2] |= CAN_ERR_PROT_BIT; + break; + case SJA1000_ECC_FORM: + cf->data[2] |= CAN_ERR_PROT_FORM; + break; + case SJA1000_ECC_STUFF: + cf->data[2] |= CAN_ERR_PROT_STUFF; + break; + default: + cf->data[3] = ecc & SJA1000_ECC_SEG; + break; + } } /* Error occurred during transmission? */ - if ((ecc & SJA1000_ECC_DIR) == 0) - cf->data[2] |= CAN_ERR_PROT_TX; + if ((ecc & SJA1000_ECC_DIR) == 0) { + stats->tx_errors++; + if (skb) + cf->data[2] |= CAN_ERR_PROT_TX; + } else { + stats->rx_errors++; + } - if (dev->can.state == CAN_STATE_ERROR_WARNING || - dev->can.state == CAN_STATE_ERROR_PASSIVE) { + if (skb && (dev->can.state == CAN_STATE_ERROR_WARNING || + dev->can.state == CAN_STATE_ERROR_PASSIVE)) { cf->can_id |= CAN_ERR_CRTL; cf->data[1] = (txerr > rxerr) ? CAN_ERR_CRTL_TX_PASSIVE : CAN_ERR_CRTL_RX_PASSIVE; } } else if (msg->type == CPC_MSG_TYPE_OVERRUN) { - cf->can_id |= CAN_ERR_CRTL; - cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; + if (skb) { + cf->can_id |= CAN_ERR_CRTL; + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; + } stats->rx_over_errors++; stats->rx_errors++; } - netif_rx(skb); + if (skb) + netif_rx(skb); } /* From d7b916540c2ba3d2a88c27b2a6287b39d8eac052 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Fri, 22 Nov 2024 23:15:53 +0100 Subject: [PATCH 13/15] can: f81604: f81604_handle_can_bus_errors(): fix {rx,tx}_errors statistics The f81604_handle_can_bus_errors() function only incremented the receive error counter and never the transmit error counter, even if the ECC_DIR flag reported that an error had occurred during transmission. Increment the receive/transmit error counter based on the value of the ECC_DIR flag. Fixes: 88da17436973 ("can: usb: f81604: add Fintek F81604 support") Signed-off-by: Dario Binacchi Link: https://patch.msgid.link/20241122221650.633981-13-dario.binacchi@amarulasolutions.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/f81604.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/usb/f81604.c b/drivers/net/can/usb/f81604.c index bc0c8903fe77..e0cfa1460b0b 100644 --- a/drivers/net/can/usb/f81604.c +++ b/drivers/net/can/usb/f81604.c @@ -526,7 +526,6 @@ static void f81604_handle_can_bus_errors(struct f81604_port_priv *priv, netdev_dbg(netdev, "bus error interrupt\n"); priv->can.can_stats.bus_error++; - stats->rx_errors++; if (skb) { cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; @@ -548,10 +547,15 @@ static void f81604_handle_can_bus_errors(struct f81604_port_priv *priv, /* set error location */ cf->data[3] = data->ecc & F81604_SJA1000_ECC_SEG; + } - /* Error occurred during transmission? */ - if ((data->ecc & F81604_SJA1000_ECC_DIR) == 0) + /* Error occurred during transmission? */ + if ((data->ecc & F81604_SJA1000_ECC_DIR) == 0) { + stats->tx_errors++; + if (skb) cf->data[2] |= CAN_ERR_PROT_TX; + } else { + stats->rx_errors++; } set_bit(F81604_CLEAR_ECC, &priv->clear_flags); From 30447a1bc0e066e492552b3e5ffeb63c1605dfe2 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 24 Nov 2024 18:42:56 +0100 Subject: [PATCH 14/15] can: mcp251xfd: mcp251xfd_get_tef_len(): work around erratum DS80000789E 6. Commit b8e0ddd36ce9 ("can: mcp251xfd: tef: prepare to workaround broken TEF FIFO tail index erratum") introduced mcp251xfd_get_tef_len() to get the number of unhandled transmit events from the Transmit Event FIFO (TEF). As the TEF has no head index, the driver uses the TX-FIFO's tail index instead, assuming that send frames are completed. When calculating the number of unhandled TEF events, that commit didn't take mcp2518fd erratum DS80000789E 6. into account. According to that erratum, the FIFOCI bits of a FIFOSTA register, here the TX-FIFO tail index might be corrupted. However here it seems the bit indicating that the TX-FIFO is empty (MCP251XFD_REG_FIFOSTA_TFERFFIF) is not correct while the TX-FIFO tail index is. Assume that the TX-FIFO is indeed empty if: - Chip's head and tail index are equal (len == 0). - The TX-FIFO is less than half full. (The TX-FIFO empty case has already been checked at the beginning of this function.) - No free buffers in the TX ring. If the TX-FIFO is assumed to be empty, assume that the TEF is full and return the number of elements in the TX-FIFO (which equals the number of TEF elements). If these assumptions are false, the driver might read to many objects from the TEF. mcp251xfd_handle_tefif_one() checks the sequence numbers and will refuse to process old events. Reported-by: Renjaya Raga Zenta Closes: https://patch.msgid.link/CAJ7t6HgaeQ3a_OtfszezU=zB-FqiZXqrnATJ3UujNoQJJf7GgA@mail.gmail.com Fixes: b8e0ddd36ce9 ("can: mcp251xfd: tef: prepare to workaround broken TEF FIFO tail index erratum") Tested-by: Renjaya Raga Zenta Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20241126-mcp251xfd-fix-length-calculation-v2-1-c2ed516ed6ba@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c index d3ac865933fd..e94321849fd7 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c @@ -21,6 +21,11 @@ static inline bool mcp251xfd_tx_fifo_sta_empty(u32 fifo_sta) return fifo_sta & MCP251XFD_REG_FIFOSTA_TFERFFIF; } +static inline bool mcp251xfd_tx_fifo_sta_less_than_half_full(u32 fifo_sta) +{ + return fifo_sta & MCP251XFD_REG_FIFOSTA_TFHRFHIF; +} + static inline int mcp251xfd_tef_tail_get_from_chip(const struct mcp251xfd_priv *priv, u8 *tef_tail) @@ -147,7 +152,29 @@ mcp251xfd_get_tef_len(struct mcp251xfd_priv *priv, u8 *len_p) BUILD_BUG_ON(sizeof(tx_ring->obj_num) != sizeof(len)); len = (chip_tx_tail << shift) - (tail << shift); - *len_p = len >> shift; + len >>= shift; + + /* According to mcp2518fd erratum DS80000789E 6. the FIFOCI + * bits of a FIFOSTA register, here the TX-FIFO tail index + * might be corrupted. + * + * However here it seems the bit indicating that the TX-FIFO + * is empty (MCP251XFD_REG_FIFOSTA_TFERFFIF) is not correct + * while the TX-FIFO tail index is. + * + * We assume the TX-FIFO is empty, i.e. all pending CAN frames + * haven been send, if: + * - Chip's head and tail index are equal (len == 0). + * - The TX-FIFO is less than half full. + * (The TX-FIFO empty case has already been checked at the + * beginning of this function.) + * - No free buffers in the TX ring. + */ + if (len == 0 && mcp251xfd_tx_fifo_sta_less_than_half_full(fifo_sta) && + mcp251xfd_get_tx_free(tx_ring) == 0) + len = tx_ring->obj_num; + + *len_p = len; return 0; } From a8c695005bfe6569acd73d777ca298ddddd66105 Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Tue, 5 Nov 2024 12:48:23 +0300 Subject: [PATCH 15/15] can: j1939: j1939_session_new(): fix skb reference counting Since j1939_session_skb_queue() does an extra skb_get() for each new skb, do the same for the initial one in j1939_session_new() to avoid refcount underflow. Reported-by: syzbot+d4e8dc385d9258220c31@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=d4e8dc385d9258220c31 Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Signed-off-by: Dmitry Antipov Tested-by: Oleksij Rempel Acked-by: Oleksij Rempel Link: https://patch.msgid.link/20241105094823.2403806-1-dmantipov@yandex.ru [mkl: clean up commit message] Signed-off-by: Marc Kleine-Budde --- net/can/j1939/transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 319f47df3330..95f7a7e65a73 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -1505,7 +1505,7 @@ static struct j1939_session *j1939_session_new(struct j1939_priv *priv, session->state = J1939_SESSION_NEW; skb_queue_head_init(&session->skb_queue); - skb_queue_tail(&session->skb_queue, skb); + skb_queue_tail(&session->skb_queue, skb_get(skb)); skcb = j1939_skb_to_cb(skb); memcpy(&session->skcb, skcb, sizeof(session->skcb));