diff options
| author | Alban Bedel <alban.bedel@lht.dlh.de> | 2026-02-09 15:47:05 +0100 |
|---|---|---|
| committer | Sasha Levin <sashal@kernel.org> | 2026-03-12 07:09:47 -0400 |
| commit | e728f444c913a91d290d1824b4770780bbd6378e (patch) | |
| tree | c7d44d625fe713e0fc04758b3249dfaa9e53e6e6 /drivers/net | |
| parent | dee4cef3748a1898e764ed853d2017a8e09ba2cc (diff) | |
can: mcp251x: fix deadlock in error path of mcp251x_open
[ Upstream commit ab3f894de216f4a62adc3b57e9191888cbf26885 ]
The mcp251x_open() function call free_irq() in its error path with the
mpc_lock mutex held. But if an interrupt already occurred the
interrupt handler will be waiting for the mpc_lock and free_irq() will
deadlock waiting for the handler to finish.
This issue is similar to the one fixed in commit 7dd9c26bd6cf ("can:
mcp251x: fix deadlock if an interrupt occurs during mcp251x_open") but
for the error path.
To solve this issue move the call to free_irq() after the lock is
released. Setting `priv->force_quit = 1` beforehand ensure that the IRQ
handler will exit right away once it acquired the lock.
Signed-off-by: Alban Bedel <alban.bedel@lht.dlh.de>
Link: https://patch.msgid.link/20260209144706.2261954-1-alban.bedel@lht.dlh.de
Fixes: bf66f3736a94 ("can: mcp251x: Move to threaded interrupts instead of workqueues.")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Diffstat (limited to 'drivers/net')
| -rw-r--r-- | drivers/net/can/spi/mcp251x.c | 15 |
1 files changed, 14 insertions, 1 deletions
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c index fa97adf25b73..bb7782582f40 100644 --- a/drivers/net/can/spi/mcp251x.c +++ b/drivers/net/can/spi/mcp251x.c @@ -1214,6 +1214,7 @@ static int mcp251x_open(struct net_device *net) { struct mcp251x_priv *priv = netdev_priv(net); struct spi_device *spi = priv->spi; + bool release_irq = false; unsigned long flags = 0; int ret; @@ -1257,12 +1258,24 @@ static int mcp251x_open(struct net_device *net) return 0; out_free_irq: - free_irq(spi->irq, priv); + /* The IRQ handler might be running, and if so it will be waiting + * for the lock. But free_irq() must wait for the handler to finish + * so calling it here would deadlock. + * + * Setting priv->force_quit will let the handler exit right away + * without any access to the hardware. This make it safe to call + * free_irq() after the lock is released. + */ + priv->force_quit = 1; + release_irq = true; + mcp251x_hw_sleep(spi); out_close: mcp251x_power_enable(priv->transceiver, 0); close_candev(net); mutex_unlock(&priv->mcp_lock); + if (release_irq) + free_irq(spi->irq, priv); return ret; } |