Hi Andrew,

Thank you for pointing me to the netdev maintainer documentation. I have
read section 1.7.4 and I understand the concern about standalone
cleanup conversions.

I would like to clarify the actual motivation behind the
devm_kzalloc() change. While it may appear to be a simple devm_
conversion on the surface, it is in fact fixing a use-after-free race
condition in the IRQF_SHARED error paths. Let me explain the problem
in detail.

PROBLEM
=======

In read_dts_node(), the driver allocates fman with kzalloc_obj() and
then registers shared interrupt handlers via devm_request_irq(), with
fman passed as the dev_id:

	L2700: fman = kzalloc_obj(*fman);
	...
	L2774: devm_request_irq(&of_dev->dev, irq, fman_irq,
	                         IRQF_SHARED, "fman", fman);
	L2783: devm_request_irq(&of_dev->dev, err_irq, fman_err_irq,
	                         IRQF_SHARED, "fman-err", fman);

The devres list after successful registration of these resources is:

	[fman (manual)] -> [main_irq handler] -> [err_irq handler] -> [ioremap]

There are three error paths after main_irq has been registered that
hit the fman_free label and call kfree(fman):

	- devm_request_irq(err_irq) fails         (L2779)  -- main_irq still registered
	- devm_platform_get_and_ioremap_resource() fails (L2797)  -- both IRQs registered
	- of_platform_populate() fails            (L2809)  -- both IRQs registered

Note that on the err_irq failure path (L2779), err_irq was not
successfully registered, so only main_irq remains in the devres list.
On the other two paths, both IRQ handlers are registered.

Taking of_platform_populate() failure as an example, the full error
cleanup sequence is:

  1. of_platform_populate() fails
  2. goto fman_free
  3. kfree(fman)                          -- fman is freed, but dev_id
                                            still points to this memory
  4. read_dts_node() returns ERR_PTR(err)
  5. fman_probe() returns error code
  6. Driver core calls device_unbind_cleanup() -> devres_release_all(),
     which releases devres resources in LIFO order:
     - Step 6a: free ioremap
     - Step 6b: devm_free_irq(err_irq)       -- err_irq handler unregistered
     - Step 6c: devm_free_irq(main_irq)      -- main_irq handler unregistered

     Until step 6c completes, at least one IRQ handler remains registered
     with the interrupt subsystem. During the window between step 3
     (kfree) and step 6c, a shared IRQ may fire and the handler will
     dereference the already-freed fman.

Because the handlers are registered with IRQF_SHARED, the kernel will
call fman_irq() and fman_err_irq() even when the interrupt is triggered
by another device sharing the same IRQ line.

Both handlers immediately dereference the fman pointer:

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

The same issue exists in fman_config(). Its err_fm_state error path
calls kfree(fman) while the devm IRQ handlers registered earlier in
read_dts_node() are still active:

	err_fm_state:
	    kfree(fman);                        -- free fman
	    return -EINVAL;                     -- devres cleanup runs after return

When fman_config() fails, fman_probe() returns -EINVAL at L2841 without
any cleanup, and the driver core then calls devres_release_all() which
releases the IRQ handlers -- again after fman has already been freed.

HOW devm_kzalloc() FIXES IT
============================

By allocating fman with devm_kzalloc(), it becomes the first entry in
the devres list:

	devres list: [fman] -> [main_irq] -> [err_irq] -> [ioremap]

Devres releases resources in LIFO order:

	1. free ioremap
	2. devm_free_irq(err_irq)               -- handlers unregistered
	3. devm_free_irq(main_irq)              -- handlers unregistered
	4. devm_kfree(fman)                     -- fman freed last, no UAF

Since fman is freed only after all IRQ handlers have been unregistered,
the use-after-free window is completely eliminated.

The manual kfree(fman) calls must also be removed to avoid double-free,
which is why they are dropped in this patch along with the allocation
change.

I will rework the patch with a commit message that clearly describes
this as a UAF fix rather than a cleanup conversion. Please let me know
if this explanation addresses the concern.

Best regards,
ZhaoJinming

On Mon, Jun 22, 2026 at 05:05:05PM +0800, ZhaoJinming wrote:

quoted

The driver now allocates the top-level struct fman with devm_kzalloc()

quoted

so that its lifetime is bound to the device and resources are released

quoted

automatically by the driver core on probe failure or device removal.

quoted

quoted

Remove the explicit kfree(fman) from the error paths in fman_config()

quoted

and read_dts_node() to avoid double-free/use-after-free and to follow

quoted

the devm_ allocation convention.

quoted

quoted

After of_find_matching_node() consumes fm_node's reference via

quoted

of_node_put(from), the post-muram error paths no longer need to clean

quoted

up fm_node, so replace goto fman_free with direct return ERR_PTR(err).

quoted

quoted

This change complements the existing use of devm_* resources (irq,

quoted

ioremap, etc.) and simplifies the error handling paths.

quoted

quoted

Signed-off-by: ZhaoJinming <redacted>

Please take a read of:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Please read it all, but see section 1.7.4.

??? Andrew

---

pw-bot: cr