Netdev List
 help / color / mirror / Atom feed
From: ZhaoJinming <zhaojinming@uniontech.com>
To: andrew@lunn.ch
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	horms@kernel.org, kuba@kernel.org, linux-kernel@vger.kernel.org,
	madalin.bucur@nxp.com, netdev@vger.kernel.org, pabeni@redhat.com,
	sean.anderson@linux.dev, zhaojinming@uniontech.com
Subject: [PATCH v3] net: fman: fix use-after-free on IRQF_SHARED handler after probe failure
Date: Wed, 24 Jun 2026 17:49:22 +0800	[thread overview]
Message-ID: <20260624094922.2971930-2-zhaojinming@uniontech.com> (raw)
In-Reply-To: <20260624094922.2971930-1-zhaojinming@uniontech.com>

In read_dts_node(), the fman structure is allocated with kzalloc_obj()
and then passed as dev_id to devm_request_irq() when registering
shared interrupt handlers:

    devm_request_irq(&of_dev->dev, irq, fman_irq, IRQF_SHARED, fman, fman);
    devm_request_irq(&of_dev->dev, err_irq, fman_err_irq, IRQF_SHARED,
                       fman-err, fman);

On error paths after IRQ registration (err_irq request failure,
ioremap failure, of_platform_populate failure), the error handling
jumps to the fman_free label which calls kfree(fman) and returns
ERR_PTR(err) from read_dts_node().

fman_probe() then returns the error to the driver core, which invokes
device_unbind_cleanup() -> devres_release_all() to clean up devres
resources in LIFO order. Since fman was allocated with kzalloc_obj()
(not devm), it was already freed at this point. However, the devm IRQ
handlers are still registered and will only be released by the
subsequent devres_release_all() call:

    kfree(fman)              <- fman freed, dev_id points to freed memory
    fman_probe() returns error
    devres_release_all():
      - free ioremap
      - devm_free_irq(err_irq)    <- handler still registered
      - devm_free_irq(main_irq)   <- handler still registered

During the window between kfree(fman) and devm_free_irq(main_irq),
the still-registered IRQF_SHARED handler may fire on behalf of another
device sharing the same IRQ line. The handler will dereference the
already-freed fman pointer:

    static irqreturn_t fman_irq(int irq, void *handle)
    {
        struct fman *fman = (struct fman *)handle;
        if (!is_init_done(fman->cfg))    <- accesses freed memory
            return IRQ_NONE;

The same problem exists in fman_config(). When fman_config() fails
at the err_fm_state error path, it calls kfree(fman) and returns
-EINVAL. fman_probe() returns this error without any cleanup, and
the driver core releases the IRQ handlers after fman has already
been freed.

Fix by explicitly calling devm_free_irq() before kfree(fman) on
all post-IRQ-registration error paths. devm_free_irq() removes the
IRQ from both the interrupt subsystem and the devres list, so
devres_release_all() will not attempt to free it again. This ensures
the IRQ handlers are fully unregistered before fman is freed,
eliminating the UAF window.

Store the main IRQ number in struct fman_dts_params so that
fman_config() can also access it for cleanup.

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/net/ethernet/freescale/fman/fman.c | 17 ++++++++++++++---
 drivers/net/ethernet/freescale/fman/fman.h |  1 +
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 013273a2de32..ba2338da0cea 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -1794,6 +1794,9 @@ static int fman_config(struct fman *fman)
 err_fm_drv:
 	kfree(fman->state);
 err_fm_state:
+	if (fman->dts_params.err_irq != 0)
+		devm_free_irq(fman->dev, fman->dts_params.err_irq, fman);
+	devm_free_irq(fman->dev, fman->dts_params.irq, fman);
 	kfree(fman);
 	return -EINVAL;
 }
@@ -2716,6 +2719,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	if (err < 0)
 		goto fman_node_put;
 	irq = err;
+	fman->dts_params.irq = irq;
 
 	/* Get the FM error interrupt */
 	err = platform_get_irq(of_dev, 1);
@@ -2786,7 +2790,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 		if (err < 0) {
 			dev_err(&of_dev->dev, "%s: irq %d allocation failed (error = %d)\n",
 				__func__, fman->dts_params.err_irq, err);
-			goto fman_free;
+			goto free_main_irq;
 		}
 	}
 
@@ -2794,7 +2798,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	if (IS_ERR(base_addr)) {
 		err = PTR_ERR(base_addr);
 		dev_err(&of_dev->dev, "%s: devm_ioremap() failed\n", __func__);
-		goto fman_free;
+		goto free_irqs;
 	}
 
 	fman->dts_params.base_addr = base_addr;
@@ -2806,7 +2810,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	if (err) {
 		dev_err(&of_dev->dev, "%s: of_platform_populate() failed\n",
 			__func__);
-		goto fman_free;
+		goto free_irqs;
 	}
 
 #ifdef CONFIG_DPAA_ERRATUM_A050385
@@ -2816,6 +2820,13 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 
 	return fman;
 
+free_irqs:
+	if (fman->dts_params.err_irq != 0)
+		devm_free_irq(&of_dev->dev, fman->dts_params.err_irq, fman);
+free_main_irq:
+	devm_free_irq(&of_dev->dev, irq, fman);
+	goto fman_free;
+
 fman_node_put:
 	of_node_put(fm_node);
 fman_free:
diff --git a/drivers/net/ethernet/freescale/fman/fman.h b/drivers/net/ethernet/freescale/fman/fman.h
index 74eb62eba0d7..d05f857c1c16 100644
--- a/drivers/net/ethernet/freescale/fman/fman.h
+++ b/drivers/net/ethernet/freescale/fman/fman.h
@@ -286,6 +286,7 @@ struct fman_dts_params {
 	struct resource *res;                   /* FMan memory resource */
 	u8 id;                                  /* FMan ID */
 
+	int irq;                                /* FMan main IRQ */
 	int err_irq;                            /* FMan Error IRQ */
 
 	u16 clk_freq;                           /* FMan clock freq (In Mhz) */
-- 
2.20.1


  reply	other threads:[~2026-06-24  9:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18  7:54 [PATCH] net: fman: fix clock and device node leak in probe error paths ZhaoJinming
2026-06-18  8:08 ` Madalin Bucur
2026-06-19 12:13 ` Simon Horman
2026-06-22  9:05   ` [PATCH v2 1/2] net: fman: fix clk reference leak in read_dts_node() ZhaoJinming
2026-06-22  9:05     ` [PATCH v2 2/2] net: fman: use devm_kzalloc() for fman and rely on devres ZhaoJinming
2026-06-22 10:36       ` Andrew Lunn
2026-06-23  6:16         ` 赵金明
2026-06-23 11:22           ` Andrew Lunn
2026-06-24  9:49             ` ZhaoJinming
2026-06-24  9:49               ` ZhaoJinming [this message]
2026-06-22 10:33     ` [PATCH v2 1/2] net: fman: fix clk reference leak in read_dts_node() Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260624094922.2971930-2-zhaojinming@uniontech.com \
    --to=zhaojinming@uniontech.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=madalin.bucur@nxp.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sean.anderson@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox