[PATCH v2] ieee802154: cc2520: fix FIFOP work use-after-free
From: Fan Wu <hidden>
Date: 2026-08-09 02:21:09
Also in:
lkml, stable
Subsystem:
cc2520 ieee-802.15.4 radio driver, ieee 802.15.4 subsystem, networking drivers, the rest · Maintainers:
Stefan Schmidt, Alexander Aring, Miquel Raynal, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
The FIFOP interrupt handler queues cc2520_fifop_irqwork. On removal,
cc2520_remove() only flushes the work. The devm-managed FIFOP IRQ
remains active until after ->remove() returns and can queue the work
again after that flush, allowing it to run after the private data is
released.
Disable the work with disable_work_sync() instead of flushing it, so
the handler can no longer queue it once removal begins. Destroy
buffer_mutex last, since unregistering can invoke the driver's stop
callback, which uses it.
Install the SFD IRQ before cc2520_register(), so it is available when
the netdev becomes visible. Install the FIFOP IRQ afterwards, so a
registration failure cannot schedule RX work while its hardware is
being released; cc2520_register() no longer frees the hardware on its
own failure, leaving that to the probe cleanup.
Found by an in-house static analysis tool.
Fixes: 0da6bc8cc341 ("ieee802154: cc2520: adds driver for TI CC2520 radio")
Cc: stable@vger.kernel.org #v6.10+
Suggested-by: Miquel Raynal <miquel.raynal@bootlin.com>
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <redacted>
---
drivers/net/ieee802154/cc2520.c | 43 +++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 21 deletions(-)
Changes in v2:
- Use disable_work_sync() for the FIFOP work instead of explicitly
freeing the interrupts. Drops the interrupt-number fields and the
manual devm_free_irq() calls.
diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
index 2b7034193..1b588f8d3 100644
--- a/drivers/net/ieee802154/cc2520.c
+++ b/drivers/net/ieee802154/cc2520.c@@ -857,12 +857,10 @@ static int cc2520_register(struct cc2520_private *priv) dev_vdbg(&priv->spi->dev, "registered cc2520\n"); ret = ieee802154_register_hw(priv->hw); if (ret) - goto err_free_device; + goto err_ret; return 0; -err_free_device: - ieee802154_free_hw(priv->hw); err_ret: return ret; }
@@ -1116,19 +1114,7 @@ static int cc2520_probe(struct spi_device *spi) if (ret) goto err_hw_init; - /* Set up fifop interrupt */ - ret = devm_request_irq(&spi->dev, - gpiod_to_irq(fifop), - cc2520_fifop_isr, - IRQF_TRIGGER_RISING, - dev_name(&spi->dev), - priv); - if (ret) { - dev_err(&spi->dev, "could not get fifop irq\n"); - goto err_hw_init; - } - - /* Set up sfd interrupt */ + /* SFD completes synchronous TX; install before cc2520_register(). */ ret = devm_request_irq(&spi->dev, gpiod_to_irq(sfd), cc2520_sfd_isr,
@@ -1142,13 +1128,29 @@ static int cc2520_probe(struct spi_device *spi) ret = cc2520_register(priv); if (ret) - goto err_hw_init; + goto err_free_hw; + + /* FIFOP arms the RX work; install after cc2520_register(). */ + ret = devm_request_irq(&spi->dev, + gpiod_to_irq(fifop), + cc2520_fifop_isr, + IRQF_TRIGGER_RISING, + dev_name(&spi->dev), + priv); + if (ret) { + dev_err(&spi->dev, "could not get fifop irq\n"); + goto err_unregister; + } return 0; +err_unregister: + ieee802154_unregister_hw(priv->hw); +err_free_hw: + if (priv->hw) + ieee802154_free_hw(priv->hw); err_hw_init: mutex_destroy(&priv->buffer_mutex); - flush_work(&priv->fifop_irqwork); return ret; }
@@ -1156,11 +1158,10 @@ static void cc2520_remove(struct spi_device *spi) { struct cc2520_private *priv = spi_get_drvdata(spi); - mutex_destroy(&priv->buffer_mutex); - flush_work(&priv->fifop_irqwork); - + disable_work_sync(&priv->fifop_irqwork); ieee802154_unregister_hw(priv->hw); ieee802154_free_hw(priv->hw); + mutex_destroy(&priv->buffer_mutex); } static const struct spi_device_id cc2520_ids[] = {