Thread (5 messages) flat view 5 messages, 2 authors, 2d ago
WARM2d

Revision v2 of 2 in this series.

Revisions (2)
  1. v1 [diff vs current]
  2. v2 current

[PATCH net v2 0/2] net: dsa: mt7530: fix two crashes on driver unbind

From: Aleksei Sviridkin <hidden>
Date: 2026-09-18 01:50:35
Also in: linux-mediatek, lkml, netdev

Unbinding the MT7530 driver from an MT7531 dereferences NULL in
regulator_disable(). On a Netcraze NC-1012 (MT7981B + MT7531, 6.18.44):

  # echo mdio-bus:1f > /sys/bus/mdio_bus/drivers/mt7530-mdio/unbind

oopses there, and the build it was found on sets CONFIG_PANIC_ON_OOPS, so
the board goes down with it. Fix that and the same command gets as far as
mt7530_remove_common(), which disposes interrupt mappings the switch's own
regmap-irq chip still owns; the regmap-irq thread then faults in
handle_nested_irq() later in the same teardown. rmmod reaches both, since
mdio_module_driver() calls .remove on module exit.

Patch 1 is the regulator one. mt7530_probe() requests the core and io
supplies only for ID_MT7530 and mt7530_setup() enables them under the same
test, but mt7530_remove() disables them unconditionally, so on an MT7621 or
an MT7531 both pointers are still NULL from devm_kzalloc(). It reaches the
MDIO front end only.

Patch 2 is the interrupt one, and it reaches further.
mt7530_remove_common() disposes the per-PHY interrupt mappings by hand from
.remove, while the regmap-irq chip that owns the domain is devm-registered
and its parent interrupt is only freed once .remove has returned.
regmap_del_irq_chip() disposes the same mappings itself, in an order that
cannot race, so the driver's call adds nothing but a window. That helper is
called from both front ends, so the defect also covers the MMIO parts -
MT7988, EN7581, AN7583 and EN7528 - which have no regulators and never meet
the first defect at all.

The order is not arbitrary. On an MT7531 the regulator fault happens in the
first thing mt7530_remove() does with the switch, so execution never
reaches the interrupt defect. The second only became visible once the first
was fixed, which is also how both came to be found on one board.

Found and verified there. Without patch 1 the unbind panics in
regulator_disable(); with patch 1 alone the panic moves on to
handle_nested_irq(); with both, three unbind/bind cycles run, two back to
back and a third after a pause. In the two whose dmesg was captured, each
unbind removes the switch from the driver directory and takes lan1 to lan4
with it, each bind brings them back, and lan1 relinks at 1Gbps/full after
both binds, lan4 after the second. uptime rose from 58 to 202 seconds
across the three without resetting and pstore gained no new record. The
third cycle stayed unbound long enough to read the descriptors: no mt7530
line in /proc/interrupts and no irq/79, irq/80 or irq/81 directory, and the
next bind reuses those three numbers - regmap-irq freeing and disposing
what the driver no longer touches. The kernel under test was identified by
the sha256 of its ELF notes section, read from /sys/kernel/notes on the
running board and computed in advance from the flashed image.

What hardware could not answer here. There is no MT7530 or MT7621 part on
this bench, so the ID_MT7530 branch that patch 1 adds was checked by
reading the generated code rather than by running it, and no MMIO part was
available to exercise patch 2 on that front end either. One unrelated WARN
remains across the unbind, from sysfs_remove_link() under
dsa_user_destroy(); it is a separate DSA teardown-ordering defect and is
not addressed here.

v2:
- patch 2 changes shape. Moving dsa_unregister_switch() in front of the
  dispose, as v1 did, only covers the phylib side: the regmap-irq chip is
  devm-registered, so its parent interrupt outlives .remove and the thread
  can still dispatch on a mapping the driver has just disposed. Dropping
  the call is what closes that: regmap_del_irq_chip() walks every hwirq
  below chip->num_irqs and disposes each one that maps, which covers
  whatever the driver created - the driver's own set is the user ports
  below MT7530_NUM_PHYS, hwirq 0 to 2 on the board below - and it does so
  after freeing the parent interrupt and before removing the domain.
- patch 2 points Fixes: at commit 254f6b272e3b ("dsa: mt7530: Utilize
  REGMAP_IRQ for interrupt handling") instead of commit ba751e28d442 ("net:
  dsa: mt7530: add interrupt support"). Before the regmap-irq conversion
  the driver owned the domain and removed it by hand, where
  irq_domain_remove() disposes nothing, so the call was required there; it
  became redundant when regmap-irq took the domain over.
- patch 1: same diff; the trailers are reordered and the message now names
  devm_kzalloc() as where the NULL comes from.
- v1: https://lore.kernel.org/netdev/20260914202421.2737079-1-f@lex.la/ (local)

Aleksei Sviridkin (2):
  net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621
  net: dsa: mt7530: leave the MDIO IRQ mappings to regmap-irq

 drivers/net/dsa/mt7530-mdio.c | 18 ++++++++++--------
 drivers/net/dsa/mt7530.c      |  3 ---
 2 files changed, 10 insertions(+), 11 deletions(-)

-- 
2.53.0

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help