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

Cheng Liu <[email protected]>
Newsgroups org.kernel.vger.linux-can,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.