Thread (8 messages) 8 messages, 3 authors, 3d ago
WARM3d

[PATCH net v2 0/2] net: fman: fix IRQ-related bugs and add probe cleanup

From: ZhaoJinming <hidden>
Date: 2026-07-21 09:00:27
Also in: lkml

Hi all,

This v2 series addresses the feedback from the previous irq_ready flag
approach (v1) and the separate UAF fix patch (v3). It replaces both
with a single root-cause fix: moving IRQ registration after driver
initialization is complete.

Issues fixed in this series:

1. Pre-init NULL dereference (crash on shared IRQ): is_init_done(NULL)
   returns true, so the guard in fman_irq()/fman_err_irq() is bypassed
   when fman->cfg is NULL before fman_config() allocates it. Moving
   devm_request_irq() to after fman_init() eliminates this window.

2. Use-after-free on probe failure (UAF): kfree(fman) was called in
   read_dts_node() error paths while devm IRQ handlers were still
   registered. Moving IRQ registration later eliminates this window.

3. Interrupt storm risk on shared IRQ line: deferred enable() to after
   IRQ registration, so the hardware is never active without a handler.

4. Memory leaks on probe failure: added fman_free_resources() and
   fman_muram_finish() to properly release all sub-resources on
   error paths after fman_config() succeeds.

5. Double-free in free_init_resources(): cleared fifo_offset and
   cam_offset after each call in fman_init() error paths, and added
   IS_ERR_VALUE() guards to prevent -ENOMEM from being treated as a
   valid offset.

Pre-existing issues NOT addressed in this series
(suggested as follow-up patches):

- Missing .remove callback: the driver has no remove/unbind path,
  causing all kzalloc'd resources to leak on device removal. This
  is a larger issue affecting the entire driver lifecycle.

- Child device probe ordering: of_platform_populate() is called
  before dev_set_drvdata(), so MAC drivers probing synchronously
  will find no parent driver data. This is a pre-existing design
  issue in the probe sequence.

- Hardware not disabled on cleanup: if enable() or devm_request_irq()
  fails after fman_init() completes, the hardware remains active
  while all software structures are freed. The driver lacks a
  disable/stop function.

- fman_muram_alloc() failure stores -ENOMEM in fifo_offset: this is
  a pre-existing issue that is now mitigated by the IS_ERR_VALUE()
  guard in free_init_resources(), but the root cause (error code
  stored in an unsigned long field) remains.

v2 changes:
- Move devm_request_irq() to fman_probe() after init (replaces
  irq_ready + READ_ONCE approach from v1)
- Defer enable() to after IRQ registration to prevent interrupt storm
- Add proper error cleanup with fman_free_resources()
- Add fman_muram_finish() for complete MURAM teardown
- Clear fifo_offset/cam_offset after free_init_resources() to
  prevent double-free
- Add IS_ERR_VALUE() guards in free_init_resources()
- Supersede the separate UAF fix patch (v3)

v1: https://lore.kernel.org/netdev/20260629084529.3709393-1-zhaojinming@uniontech.com/ (local)

Signed-off-by: ZhaoJinming <redacted>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help