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]
Date: Tue, 2 Apr 2024 14:49:48 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Romain Gantois <romain.gantois@...tlin.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Magnus Damm <magnus.damm@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Clément Léger <clement.leger@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH net-next 2/3] net: stmmac: add support for RZ/N1 GMAC

On Tue, Apr 02, 2024 at 02:37:01PM +0200, Romain Gantois wrote:
> +	ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> +	if (ret)
> +		return ret;
> +
> +	ndev = platform_get_drvdata(pdev);
> +	priv = netdev_priv(ndev);
> +
> +	pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> +	if (pcs_node) {
> +		pcs = miic_create(dev, pcs_node);
> +		of_node_put(pcs_node);
> +		if (IS_ERR(pcs))
> +			return PTR_ERR(pcs);
> +
> +		priv->hw->phylink_pcs = pcs;
> +	}

I'm afraid that this fails at one of the most basic principles of kernel
multi-threaded programming. stmmac_dvr_probe() as part of its work calls
register_netdev() which publishes to userspace the network device.

Everything that is required must be setup _prior_ to publication to
userspace to avoid races, because as soon as the network device is
published, userspace can decide to bring that interface up. If one
hasn't finished the initialisation, the interface can be brought up
before that initialisation is complete.

I don't see anything obvious in the stmmac data structures that would
allow you to hook in at an appropriate point before the
register_netdev() but after the netdev has been created. The
priv->hw data structure is created by stmmac_hwif_init()

I see that drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c is also
guilty of this as well, and should be fixed. It's even worse because it
does a truck load of stuff after stmmac_dvr_probe() which it most
definitely should not be doing.

I definitely get the feeling that the structure of the stmmac driver
is really getting out of hand, and is making stuff harder for people,
and it's not improving over time - in fact, it's getting worse. It
needs a *lot* of work to bring it back to a sane model.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ