[PATCH v3 0/2] cc2520: fix FIFOP work and RX tasklet use-after-free

[PATCH v3 0/2] cc2520: fix FIFOP work and RX tasklet use-after-free

[Fan Wu]

This series incorporates Miquel Raynal's disable_work_sync()
suggestion and separately fixes the registration-failure path.

Patch 1 fixes the removal path: the FIFOP IRQ can re-queue RX work after
cc2520_remove() flushes it, so the work may run after the private data is
freed.  Disable the work with disable_work_sync() and destroy the buffer
mutex last.

Patch 2 fixes a registration failure: with the FIFOP interrupt requested
before cc2520_register(), a failed registration could leave the mac802154
RX tasklet scheduled across ieee802154_free_hw().  Request the FIFOP
interrupt after registration (and the SFD interrupt before it).

Changes in v3:
- Split into two fixes (removal path, registration failure), per Miquel
  Raynal's review of v2.

Changes in v2:
- Use disable_work_sync() for the FIFOP work instead of explicitly
  freeing the interrupts, as suggested by Miquel Raynal.

Fan Wu (2):
  ieee802154: cc2520: fix FIFOP work use-after-free
  ieee802154: cc2520: fix RX tasklet use-after-free

 drivers/net/ieee802154/cc2520.c | 43 +++++++++++++++++----------------
 1 file changed, 22 insertions(+), 21 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] ieee802154: cc2520: fix FIFOP work use-after-free

[Fan Wu]

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 the
buffer mutex last, since the worker and the stop callback invoked
through ieee802154_unregister_hw() both take it.

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 <fanwu01@zju.edu.cn>
---
 drivers/net/ieee802154/cc2520.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
index 2b7034193..abfcfe072 100644
--- a/drivers/net/ieee802154/cc2520.c
+++ b/drivers/net/ieee802154/cc2520.c
@@ -1156,11 +1156,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[] = {
-- 
2.34.1

Re: [PATCH v3 1/2] ieee802154: cc2520: fix FIFOP work use-after-free

[Miquel Raynal]

> 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 the
> buffer mutex last, since the worker and the stop callback invoked
> through ieee802154_unregister_hw() both take it.
>
> 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 <fanwu01@zju.edu.cn>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

[PATCH v3 2/2] ieee802154: cc2520: fix RX tasklet use-after-free

[Fan Wu]

The FIFOP interrupt was requested before cc2520_register(), so a failed
registration could still have let the FIFOP handler queue RX work.  That
work calls ieee802154_rx_irqsafe(), scheduling the mac802154 RX tasklet,
which is killed only by ieee802154_unregister_hw(); on a failed
registration that does not run, so ieee802154_free_hw() could free it
while still pending.

Request the FIFOP interrupt after cc2520_register() so a registration
failure cannot schedule RX work, and the SFD interrupt before it, so
synchronous TX has its completion available once the netdev is visible.
cc2520_register() no longer frees the hardware on its own failure; the
probe cleanup does.

Found by an in-house static analysis tool.

Fixes: 0da6bc8cc341 ("ieee802154: cc2520: adds driver for TI CC2520 radio")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/net/ieee802154/cc2520.c | 38 +++++++++++++++++----------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
index abfcfe072..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;
 }
 
-- 
2.34.1

Re: [PATCH v3 2/2] ieee802154: cc2520: fix RX tasklet use-after-free

[Miquel Raynal]

Hi,

On 11/08/2026 at 01:03:59 GMT, Fan Wu <fanwu01@zju.edu.cn> wrote:

> The FIFOP interrupt was requested before cc2520_register(), so a failed
> registration could still have let the FIFOP handler queue RX work.

Really? How? SRXON is only set in ->start(), which means radio is off
until the user asks for it, which can only happen after proper netdev
registration.

It feels like the rest of this commit is pure AI hallucination, so I'll
stop there.

Thanks,
Miquèl

Re: [PATCH v3 2/2] ieee802154: cc2520: fix RX tasklet use-after-free

[Fan Wu]

You're right. I failed to trace the receive-enable path, please drop patch
2/2. Patch 1/2 is unchanged.

Sorry for the noise, and thanks for catching this.

> On Aug 11, 2026, at 16:28, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> Really? How? SRXON is only set in ->start(), which means radio is off
> until the user asks for it, which can only happen after proper netdev
> registration.
> 
> It feels like the rest of this commit is pure AI hallucination, so I'll
> stop there.
> 
> Thanks,
> Miquèl

Re: [PATCH v3 2/2] ieee802154: cc2520: fix RX tasklet use-after-free

[Miquel Raynal]

Hi,

On 11/08/2026 at 17:48:26 +08, Fan Wu <12321260@zju.edu.cn> wrote:

> You're right. I failed to trace the receive-enable path, please drop patch
> 2/2. Patch 1/2 is unchanged.
>
> Sorry for the noise, and thanks for catching this.

Please send a v4 with a single patch.

Thanks,
Miquèl