netdev - [net-next 20/25] can: flexcan: flexcan_close(): change order if commands to properly shut down the controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 20 Nov 2020 14:33:13 +0100
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org, linux-can@...r.kernel.org,
        kernel@...gutronix.de, Marc Kleine-Budde <mkl@...gutronix.de>
Subject: [net-next 20/25] can: flexcan: flexcan_close(): change order if commands to properly shut down the controller

There haven been reports, that the flexcan_close() soradically hangs during
simultanious ifdown, sending of CAN messages and probably open CAN bus:

| (__schedule) from [<808bbd34>] (schedule+0x90/0xb8)
| (schedule) from [<808bf274>] (schedule_timeout+0x1f8/0x24c)
| (schedule_timeout) from [<8016be44>] (msleep+0x18/0x1c)
| (msleep) from [<80746a64>] (napi_disable+0x60/0x70)
| (napi_disable) from [<8052fdd0>] (flexcan_close+0x2c/0x140)
| (flexcan_close) from [<80744930>] (__dev_close_many+0xb8/0xd8)
| (__dev_close_many) from [<8074db9c>] (__dev_change_flags+0xd0/0x1a0)
| (__dev_change_flags) from [<8074dc84>] (dev_change_flags+0x18/0x48)
| (dev_change_flags) from [<80760c24>] (do_setlink+0x44c/0x7b4)
| (do_setlink) from [<80761560>] (rtnl_newlink+0x374/0x68c)

I was unable to reproduce the issue, but a cleanup of the flexcan close
sequence has probably fixed the problem at the reporting user.

This patch changes the sequence in flexcan_close() to:
- stop the TX queue
- disable the interrupts on the chip level and wait via free_irq()
  synchronously for the interrupt handler to finish
- disable RX offload, which disables synchronously NAPI
- disable the flexcan on the chip level
- free RX offload
- disable the transceiver
- close the CAN device
- disable the clocks

Link: https://lore.kernel.org/r/20201119100917.3013281-6-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>
---
 drivers/net/can/flexcan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index e98368be1669..e85f20d18d67 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1754,15 +1754,15 @@ static int flexcan_close(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 
 	netif_stop_queue(dev);
+	flexcan_chip_interrupts_disable(dev);
+	free_irq(dev->irq, dev);
 	can_rx_offload_disable(&priv->offload);
 	flexcan_chip_stop_disable_on_error(dev);
-	flexcan_chip_interrupts_disable(dev);
 
 	can_rx_offload_del(&priv->offload);
-	free_irq(dev->irq, dev);
 	flexcan_transceiver_disable(priv);
-
 	close_candev(dev);
+
 	pm_runtime_put(priv->dev);
 
 	can_led_event(dev, CAN_LED_EVENT_STOP);
-- 
2.29.2