mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] net: dsa: microchip: drop the parent MDIO bus reference
@ 2026-11-03 10:05 Wentao Liang
  2026-11-03 10:10 ` netdev-bot+sinfo
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Wentao Liang @ 2026-11-03 10:05 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-kernel, Wentao Liang, stable

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(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 767c8fe8e819..ddd821fa0d3e 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2392,6 +2392,17 @@ int ksz_parse_dt_phy_config(struct ksz_device *dev, struct mii_bus *bus,
 	return 0;
 }
 
+/**
+ * ksz_mdio_put_parent_bus - Drop the reference on a parent MDIO bus
+ * @data: mii_bus obtained from of_mdio_find_bus()
+ */
+static void ksz_mdio_put_parent_bus(void *data)
+{
+	struct mii_bus *bus = data;
+
+	put_device(&bus->dev);
+}
+
 /**
  * ksz_mdio_register - Register and configure the MDIO bus for the KSZ device.
  * @dev: Pointer to the KSZ device structure.
@@ -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;
+
 		dev->parent_mdio_bus = parent_bus;
 	}
 
-- 
2.34.1


^ 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
                   ` (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

end of thread, other threads:[~2026-11-07 11:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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

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®