mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: ZhaoJinming <zhaojinming@uniontech.com>
To: horms@kernel.org, madalin.bucur@nxp.com, sean.anderson@linux.dev
Cc: netdev@vger.kernel.org, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, linux-kernel@vger.kernel.org,
	ZhaoJinming <zhaojinming@uniontech.com>
Subject: [PATCH net v2 2/2] net: fman: add error cleanup path in fman_probe
Date: Tue, 21 Jul 2026 16:58:41 +0800	[thread overview]
Message-ID: <20260721085841.488088-3-zhaojinming@uniontech.com> (raw)
In-Reply-To: <20260721085841.488088-1-zhaojinming@uniontech.com>

fman_init() and devm_request_irq() failure paths in fman_probe()
do not free fman and its sub-resources (keygen, muram allocations,
state, cfg), causing memory leaks on probe failure.

Add fman_muram_finish() to properly tear down a MURAM partition
(gen_pool_destroy + iounmap + kfree), complementing the existing
fman_muram_init().

Add fman_free_resources() that releases all fman sub-resources
in the correct order:
- devm_free_irq() for any already-registered IRQ handlers
- kfree(fman->keygen)
- free_init_resources() for MURAM CAM/FIFO allocations
- kfree(fman->cfg)
- fman_muram_finish(fman->muram) for the MURAM management object
- kfree(fman->state)
- kfree(fman)

Use two goto labels in fman_probe():
- err_irq: main IRQ registered but err_irq or enable() failed
  -- free main IRQ then fall through to release resources
- err_no_irq: no IRQ registered -- just release resources

The IRQ handlers must be explicitly freed before kfree(fman) to
avoid a window where a shared-IRQ spurious firing could dereference
the freed dev_id.

Clear fman->fifo_offset and fman->cam_offset after each
free_init_resources() call in fman_init() to prevent a double-free
when fman_free_resources() calls free_init_resources() again on the
same error paths.

Note: fman_config() is not changed -- it already frees fman
internally on all its error paths, so fman_probe() must not touch
fman after fman_config() fails.

v2:
- add explicit devm_free_irq() before kfree(fman) to eliminate
  a potential UAF window on the cleanup path
- add fman_muram_finish() for complete MURAM teardown
- add kfree(fman->cfg) to release config structure
- clear fifo_offset/cam_offset after free_init_resources() in
  fman_init() to prevent double-free

Fixes: 414fd46e7762 ("fsl/fman: Add FMan support")
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/net/ethernet/freescale/fman/fman.c    | 42 ++++++++++++++++---
 .../net/ethernet/freescale/fman/fman_muram.c  | 15 +++++++
 .../net/ethernet/freescale/fman/fman_muram.h  |  2 +
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 13913f152147..374f5b7305f8 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -1190,10 +1190,10 @@ static bool is_init_done(struct fman_cfg *cfg)
 
 static void free_init_resources(struct fman *fman)
 {
-	if (fman->cam_offset)
+	if (fman->cam_offset && !IS_ERR_VALUE(fman->cam_offset))
 		fman_muram_free_mem(fman->muram, fman->cam_offset,
 				    fman->cam_size);
-	if (fman->fifo_offset)
+	if (fman->fifo_offset && !IS_ERR_VALUE(fman->fifo_offset))
 		fman_muram_free_mem(fman->muram, fman->fifo_offset,
 				    fman->fifo_size);
 }
@@ -1963,6 +1963,8 @@ static int fman_init(struct fman *fman)
 	err = dma_init(fman);
 	if (err != 0) {
 		free_init_resources(fman);
+		fman->fifo_offset = 0;
+		fman->cam_offset = 0;
 		return err;
 	}
 
@@ -1975,6 +1977,8 @@ static int fman_init(struct fman *fman)
 					     fman->state->total_fifo_size);
 	if (IS_ERR_VALUE(fman->fifo_offset)) {
 		free_init_resources(fman);
+		fman->fifo_offset = 0;
+		fman->cam_offset = 0;
 		dev_err(fman->dev, "%s: MURAM alloc for BMI FIFO failed\n",
 			__func__);
 		return -ENOMEM;
@@ -1998,6 +2002,8 @@ static int fman_init(struct fman *fman)
 	fman->keygen = keygen_init(fman->kg_regs);
 	if (!fman->keygen) {
 		free_init_resources(fman);
+		fman->fifo_offset = 0;
+		fman->cam_offset = 0;
 		return -EINVAL;
 	}
 
@@ -2800,6 +2806,24 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	return ERR_PTR(err);
 }
 
+static void fman_free_resources(struct fman *fman, struct device *dev,
+				bool irq_registered)
+{
+	/* Free IRQs first while fman is still valid */
+	if (irq_registered) {
+		if (fman->dts_params.err_irq != 0)
+			devm_free_irq(dev, fman->dts_params.err_irq, fman);
+		devm_free_irq(dev, fman->dts_params.irq, fman);
+	}
+
+	kfree(fman->keygen);
+	free_init_resources(fman);
+	kfree(fman->cfg);
+	fman_muram_finish(fman->muram);
+	kfree(fman->state);
+	kfree(fman);
+}
+
 static int fman_probe(struct platform_device *of_dev)
 {
 	struct fman *fman;
@@ -2820,7 +2844,7 @@ static int fman_probe(struct platform_device *of_dev)
 
 	if (fman_init(fman) != 0) {
 		dev_err(dev, "%s: FMan init failed\n", __func__);
-		return -EINVAL;
+		goto err_no_irq;
 	}
 
 	/* Register IRQ handlers only after initialization is complete.
@@ -2838,7 +2862,7 @@ static int fman_probe(struct platform_device *of_dev)
 	if (err < 0) {
 		dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
 			__func__, fman->dts_params.irq, err);
-		return err;
+		goto err_no_irq;
 	}
 
 	if (fman->dts_params.err_irq != 0) {
@@ -2848,7 +2872,7 @@ static int fman_probe(struct platform_device *of_dev)
 		if (err < 0) {
 			dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
 				__func__, fman->dts_params.err_irq, err);
-			return err;
+			goto err_irq;
 		}
 	}
 
@@ -2863,7 +2887,7 @@ static int fman_probe(struct platform_device *of_dev)
 	err = enable(fman);
 	if (err != 0) {
 		dev_err(dev, "%s: FMan enable failed\n", __func__);
-		return err;
+		goto err_irq;
 	}
 
 	if (fman->dts_params.err_irq == 0) {
@@ -2891,6 +2915,12 @@ static int fman_probe(struct platform_device *of_dev)
 	dev_dbg(dev, "FMan%d probed\n", fman->dts_params.id);
 
 	return 0;
+
+err_irq:
+	devm_free_irq(dev, fman->dts_params.irq, fman);
+err_no_irq:
+	fman_free_resources(fman, dev, false);
+	return err ?: -EINVAL;
 }
 
 static const struct of_device_id fman_match[] = {
diff --git a/drivers/net/ethernet/freescale/fman/fman_muram.c b/drivers/net/ethernet/freescale/fman/fman_muram.c
index 6ac7c2b0cb19..6c2b4f7a02b8 100644
--- a/drivers/net/ethernet/freescale/fman/fman_muram.c
+++ b/drivers/net/ethernet/freescale/fman/fman_muram.c
@@ -129,3 +129,18 @@ void fman_muram_free_mem(struct muram_info *muram, unsigned long offset,
 
 	gen_pool_free(muram->pool, addr, size);
 }
+
+/**
+ * fman_muram_finish
+ * @muram:	FM-MURAM module pointer.
+ *
+ * Frees all resources associated with a MURAM partition.
+ */
+void fman_muram_finish(struct muram_info *muram)
+{
+	if (!muram)
+		return;
+	iounmap(muram->vbase);
+	gen_pool_destroy(muram->pool);
+	kfree(muram);
+}
diff --git a/drivers/net/ethernet/freescale/fman/fman_muram.h b/drivers/net/ethernet/freescale/fman/fman_muram.h
index 3643af61bae2..a5cb544c0f08 100644
--- a/drivers/net/ethernet/freescale/fman/fman_muram.h
+++ b/drivers/net/ethernet/freescale/fman/fman_muram.h
@@ -23,4 +23,6 @@ unsigned long fman_muram_alloc(struct muram_info *muram, size_t size);
 void fman_muram_free_mem(struct muram_info *muram, unsigned long offset,
 			 size_t size);
 
+void fman_muram_finish(struct muram_info *muram);
+
 #endif /* __FM_MURAM_EXT */
-- 
2.20.1


  parent reply	other threads:[~2026-07-21  9:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03  7:43 [PATCH net v2 1/2] net: fman: move IRQ registration after init to prevent NULL deref and UAF ZhaoJinming
2026-07-03  7:43 ` [PATCH net v2 2/2] net: fman: add error cleanup path in fman_probe ZhaoJinming
2026-07-09 14:56   ` Simon Horman
2026-07-09 14:52 ` [PATCH net v2 1/2] net: fman: move IRQ registration after init to prevent NULL deref and UAF Simon Horman
2026-07-21  8:58   ` [PATCH net v2 0/2] net: fman: fix IRQ-related bugs and add probe cleanup ZhaoJinming
2026-07-21  8:58     ` [PATCH net v2 1/2] net: fman: move IRQ registration after init to prevent NULL deref and UAF ZhaoJinming
2026-07-21  8:58     ` ZhaoJinming [this message]
2026-07-23 17:26     ` [PATCH net v2 0/2] net: fman: fix IRQ-related bugs and add probe cleanup Jakub Kicinski

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=20260721085841.488088-3-zhaojinming@uniontech.com \
    --to=zhaojinming@uniontech.com \
    --cc=andrew+netdev@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

all inboxes | Powered by JetHome®