Thread (3 messages) flat view 3 messages, 2 authors, 5h ago
HOTtoday

[PATCH can] can: rockchip: rk3576: fix rtnl_lock deadlock during interface down under bus traffic

From: Cheng Liu <hidden>
Date: 2026-08-17 01:38:00
Also in: linux-can, lkml
Subsystem: arm/rockchip soc support, can network drivers, rockchip can-fd driver, the rest · Maintainers: Heiko Stuebner, Marc Kleine-Budde, Vincent Mailhol, Linus Torvalds

When bringing the CAN interface down (via `ip link set can0 down`) while
there is heavy incoming CAN traffic or continuous hardware error frames,
the system hangs and deadlocks. Existing and new networking operations
(such as `ifconfig`, SSH logins, Socket operations) hang indefinitely
waiting for `rtnl_lock`.

The deadlock occurs because `rk3576_canfd_close()` calls `napi_disable()`
before `rk3576_canfd_stop()`. Since hardware interrupts are still
active, incoming CAN frames and error interrupts continuously trigger
`napi_schedule()`, preventing `napi_disable()` from seeing the
`NAPI_STATE_SCHED` bit cleared and causing it to loop infinitely in
`msleep(1)`. Because `dev_close()` holds the global `rtnl_lock`, the
entire networking subsystem deadlocks:

Call trace:
  __switch_to+0xdc/0x120
  __schedule+0x2ac/0x840
  schedule+0x54/0xe0
  schedule_hrtimeout_range_clock+0x98/0x134
  usleep_range_state+0x7c/0xb0
  napi_disable+0xc0/0x110
  rk3576_canfd_close+0x44/0xd0
  __dev_close_many+0xb0/0x14c
  dev_change_flags+0x28/0x64
  do_setlink+0x618/0xe2c
  rtnetlink_rcv_msg+0x2a8/0x380

Fix this by:
1. Reordering `rk3576_canfd_close()` to call `rk3576_canfd_stop()` before
   `napi_disable()`, ensuring interrupts are disabled and controller is
   in reset mode before waiting for NAPI to complete.
2. Standardizing `rk3576_canfd_rx_poll()` to respect the NAPI
   quota/budget and properly complete NAPI polling via
   `napi_complete_done()`.
3. Guarding against NULL pointer dereference in `rk3576_canfd_err()` when
   `alloc_can_err_skb()` fails, and calling `can_bus_off()` upon bus-off.

Signed-off-by: Cheng Liu <redacted>
---
 drivers/net/can/rockchip/rk3576_canfd.c | 61 +++++++++++++------------
 1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/drivers/net/can/rockchip/rk3576_canfd.c b/drivers/net/can/rockchip/rk3576_canfd.c
index 2c0d7f056..d0f581bce 100644
--- a/drivers/net/can/rockchip/rk3576_canfd.c
+++ b/drivers/net/can/rockchip/rk3576_canfd.c
@@ -859,31 +859,28 @@ static int rk3576_canfd_rx_poll(struct napi_struct *napi, int quota)
 {
 	struct net_device *ndev = napi->dev;
 	struct rk3576_canfd *rcan = netdev_priv(ndev);
-	int work_done = 0, cnt = 0;
+	int work_done = 0;
+	u32 frames_avail;
 
 	if (rcan->use_dma) {
-		while (work_done < rcan->quota)
+		while (work_done < rcan->quota && work_done < quota)
 			work_done += rk3576_canfd_rx(ndev, work_done);
 
-		if (work_done <= rcan->rx_fifo_depth) {
-			napi_complete_done(napi, work_done);
-			rk3576_canfd_write(rcan, CANFD_INT_MASK, INT_ENABLE);
+		if (work_done < quota) {
+			if (napi_complete_done(napi, work_done))
+				rk3576_canfd_write(rcan, CANFD_INT_MASK, INT_ENABLE);
 		}
 	} else {
-		quota = (rk3576_canfd_read(rcan, CANFD_STR_STATE) & rcan->rx_fifo_mask) >>
-			rcan->rx_fifo_shift;
-		quota = quota / rcan->rx_max_data;
-		cnt = (rk3576_canfd_read(rcan, CANFD_STR_STATE) & INTM_CNT_MASK) >> INTM_CNT_SHIFT;
-		if (quota != cnt)
-			quota = ((rk3576_canfd_read(rcan, CANFD_STR_STATE) & rcan->rx_fifo_mask) >>
-				rcan->rx_fifo_shift) / rcan->rx_max_data;
-
-		while (work_done < quota)
+		frames_avail = (rk3576_canfd_read(rcan, CANFD_STR_STATE) & rcan->rx_fifo_mask) >>
+			       rcan->rx_fifo_shift;
+		frames_avail = frames_avail / rcan->rx_max_data;
+
+		while (work_done < frames_avail && work_done < quota)
 			work_done += rk3576_canfd_rx(ndev, CANFD_RXFRD);
 
-		if (work_done <= rcan->rx_fifo_depth) {
-			napi_complete_done(napi, work_done);
-			rk3576_canfd_write(rcan, CANFD_INT_MASK, INT_ENABLE);
+		if (work_done < quota) {
+			if (napi_complete_done(napi, work_done))
+				rk3576_canfd_write(rcan, CANFD_INT_MASK, INT_ENABLE);
 		}
 	}
 	return work_done;
@@ -926,7 +923,7 @@ static int rk3576_canfd_err(struct net_device *ndev, u32 isr)
 {
 	struct rk3576_canfd *rcan = netdev_priv(ndev);
 	struct net_device_stats *stats = &ndev->stats;
-	struct can_frame *cf;
+	struct can_frame *cf = NULL;
 	struct sk_buff *skb;
 	unsigned int rxerr, txerr;
 	u32 sta_reg;
@@ -945,17 +942,21 @@ static int rk3576_canfd_err(struct net_device *ndev, u32 isr)
 	if (isr & BUS_OFF_INT) {
 		rcan->can.state = CAN_STATE_BUS_OFF;
 		rcan->can.can_stats.bus_off++;
-		cf->can_id |= CAN_ERR_BUSOFF;
+		can_bus_off(ndev);
+		if (skb)
+			cf->can_id |= CAN_ERR_BUSOFF;
 	} else if (isr & PASSIVE_ERR_INT) {
 		rcan->can.can_stats.error_passive++;
 		rcan->can.state = CAN_STATE_ERROR_PASSIVE;
 		/* error passive state */
-		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = (txerr > rxerr) ?
-					CAN_ERR_CRTL_TX_WARNING :
-					CAN_ERR_CRTL_RX_WARNING;
-		cf->data[6] = txerr;
-		cf->data[7] = rxerr;
+		if (skb) {
+			cf->can_id |= CAN_ERR_CRTL;
+			cf->data[1] = (txerr > rxerr) ?
+						CAN_ERR_CRTL_TX_WARNING :
+						CAN_ERR_CRTL_RX_WARNING;
+			cf->data[6] = txerr;
+			cf->data[7] = rxerr;
+		}
 	}
 	if (sta_reg & ERR_WARNING_STATE) {
 		rcan->can.can_stats.error_warning++;
@@ -984,9 +985,11 @@ static int rk3576_canfd_err(struct net_device *ndev, u32 isr)
 		can_free_echo_skb(ndev, 0, NULL);
 		netif_start_queue(ndev);
 	}
-	stats->rx_packets++;
-	stats->rx_bytes += cf->can_dlc;
-	netif_rx(skb);
+	if (skb) {
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+		netif_rx(skb);
+	}
 
 	return 0;
 }
@@ -1070,8 +1073,8 @@ static int rk3576_canfd_close(struct net_device *ndev)
 	struct rk3576_canfd *rcan = netdev_priv(ndev);
 
 	netif_stop_queue(ndev);
-	napi_disable(&rcan->napi);
 	rk3576_canfd_stop(ndev);
+	napi_disable(&rcan->napi);
 	close_candev(ndev);
 	pm_runtime_put(rcan->dev);
 
-- 
2.34.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help