* Re: [PATCH] net: dsa: microchip: drop the parent MDIO bus reference
2026-11-03 10:05 [PATCH] net: dsa: microchip: drop the parent MDIO bus reference Wentao Liang
@ 2026-11-03 10:10 ` netdev-bot+sinfo
2026-11-03 10:25 ` Krzysztof Kozlowski
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-11-03 10:10 UTC (permalink / raw)
To: Wentao Liang
Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel, stable
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- How the issue was discovered, e.g. hit in production, hit during
development, syzbot report, manual code inspection, LLM or static
analysis tool scan.
- Whether the issue was actually triggered, or is only theoretical
(e.g. found by code inspection). If it was triggered please include
the symptoms, like the stack trace or error messages.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: microchip: drop the parent MDIO bus reference
2026-11-03 10:05 [PATCH] net: dsa: microchip: drop the parent MDIO bus reference Wentao Liang
2026-11-03 10:10 ` netdev-bot+sinfo
@ 2026-11-03 10:25 ` Krzysztof Kozlowski
2026-11-05 22:29 ` Jakub Kicinski
2026-11-07 11:47 ` netdev-bot+sashiko
3 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-11-03 10:25 UTC (permalink / raw)
To: Wentao Liang
Cc: Eric Dumazet, Paolo Abeni, David S. Miller, Woojung Huh,
Vladimir Oltean, linux-kernel, stable, netdev, Jakub Kicinski,
Andrew Lunn, UNGLinuxDriver
On Tue, 03 Nov 2026 10:05:31 +0000, Wentao Liang wrote:
> ksz_mdio_register() obtains the parent MDIO bus with of_mdio_find_bus(),
> which takes a reference on the embedded struct device, and stores it in
> dev->parent_mdio_bus for the side MDIO accessors. Nothing ever drops
> that reference, so it is leaked on the probe error paths and when the
> switch device is torn down.
>
> Register a devres action to put the device when the DSA switch device
> is released. The child MDIO bus is unregistered before the action
> runs, so the parent bus stays valid for the whole lifetime of the
> switch.
>
> Fixes: 9afaf0eec2ab ("net: dsa: microchip: Refactor MDIO handling for side MDIO access")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/net/dsa/microchip/ksz_common.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.
2. You use multiple identities with this email address, thus I doubt that we
speak with an actual person.
3. Your bot does not even bother running checkpatch, right?
4. Nothing of this was reproduced nor tested.
5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: microchip: drop the parent MDIO bus reference
2026-11-03 10:05 [PATCH] net: dsa: microchip: drop the parent MDIO bus reference Wentao Liang
2026-11-03 10:10 ` netdev-bot+sinfo
2026-11-03 10:25 ` Krzysztof Kozlowski
@ 2026-11-05 22:29 ` Jakub Kicinski
2026-11-07 11:47 ` netdev-bot+sashiko
3 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2026-11-05 22:29 UTC (permalink / raw)
To: Wentao Liang
Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel,
stable
On Tue, 3 Nov 2026 10:05:31 +0000 Wentao Liang wrote:
> ksz_mdio_register() obtains the parent MDIO bus with of_mdio_find_bus(),
> which takes a reference on the embedded struct device, and stores it in
> dev->parent_mdio_bus for the side MDIO accessors. Nothing ever drops
> that reference, so it is leaked on the probe error paths and when the
> switch device is torn down.
Hi, since you're spamming the community with low-value fixes
we would like to ask you to not post any further networking
patches for the rest of this calendar year.
--
pv-ban: 2mo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: microchip: drop the parent MDIO bus reference
2026-11-03 10:05 [PATCH] net: dsa: microchip: drop the parent MDIO bus reference Wentao Liang
` (2 preceding siblings ...)
2026-11-05 22:29 ` Jakub Kicinski
@ 2026-11-07 11:47 ` netdev-bot+sashiko
3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-11-07 11:47 UTC (permalink / raw)
To: vulab
Cc: woojung.huh, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
kuba, pabeni, netdev, linux-kernel, stable
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The patch fixes code that never runs, and the reference leak it
describes is still live in mainline.
Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the cached
dev->parent_mdio_bus pointer is only protected by a struct device…
--
net: dsa: microchip: drop the parent MDIO bus reference
This patch adds a devres action to ksz_mdio_register(). The action calls
put_device() on the parent MDIO bus returned by of_mdio_find_bus(). The
goal is to stop leaking that struct device reference on probe error
paths and when the switch is torn down.
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 767c8fe8e819a..ddd821fa0d3e1 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
[ ... ]
> @@ -2431,6 +2442,12 @@ int ksz_mdio_register(struct ksz_device *dev)
> goto put_mdio_node;
> }
>
> + ret = devm_add_action_or_reset(ds->dev,
> + ksz_mdio_put_parent_bus,
> + parent_bus);
> + if (ret)
> + goto put_mdio_node;
> +
[Severity: High]
Can any current chip reach this branch? It sits behind this check in
ksz_mdio_register():
if (parent_bus_node && !dev->info->phy_side_mdio_supported) {
...
ret = -EINVAL;
goto put_mdio_node;
In ksz_switch_chips[], only the LAN9370-LAN9374 entries set
phy_side_mdio_supported = true. All of them use lan937x_switch_ops, so
their setup path is lan937x_setup()->lan937x_mdio_register(), not
ksz_mdio_register().
ksz_mdio_register() is called only from ksz8_setup(), ksz8463_setup()
and ksz9477_setup(). None of those chips support side MDIO. For them, a
DT with mdio-parent-bus returns -EINVAL before of_mdio_find_bus() is
called, so the new put_device() action is never registered.
Commit 03d10c776802 ("net: dsa: microchip: implement lan937x-specific MDIO
registration") copied the side MDIO logic into lan937x_main.c, and that
copy still takes the reference without dropping it:
drivers/net/dsa/microchip/lan937x_main.c:lan937x_mdio_register() {
...
} else if (parent_bus_node) {
parent_bus = of_mdio_find_bus(parent_bus_node);
if (!parent_bus) {
ret = -EPROBE_DEFER;
goto put_mdio_node;
}
dev->parent_mdio_bus = parent_bus;
}
...
}
Does the parent bus struct device reference still leak there? That
would happen on the later error paths (devm_mdiobus_alloc(),
lan937x_mdio_bus_preinit(), lan937x_create_phy_addr_map(),
ksz_parse_dt_phy_config(), devm_of_mdiobus_register()), on deferred
re-probes, and on every unbind of a LAN937x switch.
ksz_mdio_put_parent_bus() is static to ksz_common.c, so as written
lan937x_mdio_register() cannot use it. Should the fix go into
lan937x_mdio_register() instead? The side MDIO branch in
ksz_mdio_register() looks unreachable and could perhaps be removed.
With Cc: stable and Fixes: 9afaf0eec2ab, this would be backported to
trees both with and without the lan937x split. Would it only have an
effect in the trees without the split?
> dev->parent_mdio_bus = parent_bus;
> }
[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. The
commit message says:
"The child MDIO bus is unregistered before the action runs, so the
parent bus stays valid for the whole lifetime of the switch."
Does this hold for anything beyond the mii_bus memory? The reference
from of_mdio_find_bus() keeps struct mii_bus allocated. However, there
is no device_link to the parent MDIO controller and no
try_module_get(bus->owner).
The parent controller driver can be unbound through sysfs, or its
module unloaded, while the switch stays bound. PHY accesses on the side
bus would still go through:
ksz_parent_mdio_read()->mdiobus_read_nested()->__mdiobus_read()->bus->read()
__mdiobus_read() in drivers/net/phy/mdio_bus.c does not check
bus->state:
if (bus->read)
retval = bus->read(bus, addr, regnum);
Could the parent driver's read or write callback then run against
released devm resources or unloaded module text? That depends on the
specific parent MDIO driver. The same applies to ksz_parent_mdio_write()
and to the pointer cached in lan937x_mdio_register().
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103100531.1309946-1-vulab%40iscas.ac.cn
^ permalink raw reply [flat|nested] 5+ messages in thread