netdev - Re: [PATCH] net: dsa: microchip: fix NULL pointer dereference on platform init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231206003706.w3azftqx7nopn4go@skbuf>
Date: Wed, 6 Dec 2023 02:37:06 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Daniel Danzberger <dd@...edd.com>
Cc: woojung.huh@...rochip.com, UNGLinuxDriver@...rochip.com,
	netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
	Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH] net: dsa: microchip: fix NULL pointer dereference on
 platform init

On Tue, Dec 05, 2023 at 11:15:58PM +0100, Daniel Danzberger wrote:
> The lan* devices are then bridged together:
> 
> root@...-ngr421:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> br-lan		7fff.00139555cfd7	no		lan4
> 							lan2
> 							lan3
> 							lan1

Ok, so it forwards traffic because the ports are bridged, not that it
does that by default.

I see in your output that the internal PHY network interfaces are
registered and functional, and now I know how that works.

ksz_mdio_register() is indeed bypassed by the lack of OF as I suspected,
so it's not that function which is responsible for creating the MDIO bus.
But this piece of code from dsa_switch_setup() will kick in:

	if (!ds->user_mii_bus && ds->ops->phy_read) {
		ds->user_mii_bus = mdiobus_alloc();
		if (!ds->user_mii_bus) {
			err = -ENOMEM;
			goto teardown;
		}

		dsa_user_mii_bus_init(ds);

		dn = of_get_child_by_name(ds->dev->of_node, "mdio");

		err = of_mdiobus_register(ds->user_mii_bus, dn);
		of_node_put(dn);
		if (err < 0)
			goto free_user_mii_bus;
	}

aka DSA will set up a ds->user_mii_bus which calls into ds->ops->phy_read()
and ds->ops->phy_write() to access the internal PHYs. Although this bus
may be OF-based (if "dn" above is non-NULL), the of_mdiobus_register()
implementation simply calls mdiobus_register() if there is no OF node.

So, surprisingly, there is enough redundancy between DSA mechanisms that
platform_data kinda works.