netdev - Re: [PATCH net-next v4 4/5] dpaa2-eth: add MAC/PHY support through phylink

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:   Sun, 17 Nov 2019 16:13:51 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Ioana Ciornei <ioana.ciornei@....com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        laurentiu.tudor@....com, andrew@...n.ch, f.fainelli@...il.com
Subject: Re: [PATCH net-next v4 4/5] dpaa2-eth: add MAC/PHY support through
 phylink

On Thu, Oct 31, 2019 at 01:18:31AM +0200, Ioana Ciornei wrote:
> The dpaa2-eth driver now has support for connecting to its associated
> PHY device found through standard OF bindings.
> 
> This happens when the DPNI object (that the driver probes on) gets
> connected to a DPMAC. When that happens, the device tree is looked up by
> the DPMAC ID, and the associated PHY bindings are found.
> 
> The old logic of handling the net device's link state by hand still
> needs to be kept, as the DPNI can be connected to other devices on the
> bus than a DPMAC: other DPNI, DPSW ports, etc. This logic is only
> engaged when there is no DPMAC (and therefore no phylink instance)
> attached.
> 
> The MC firmware support multiple type of DPMAC links: TYPE_FIXED,
> TYPE_PHY. The TYPE_FIXED mode does not require any DPMAC management from
> Linux side, and as such, the driver will not handle such a DPMAC.
> 
> Although PHYLINK typically handles SFP cages and in-band AN modes, for
> the moment the driver only supports the RGMII interfaces found on the
> LX2160A. Support for other modes will come later.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@....com>
> Reviewed-by: Andrew Lunn <andrew@...n.ch>

...

> @@ -3363,6 +3425,13 @@ static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
>  	if (status & DPNI_IRQ_EVENT_ENDPOINT_CHANGED) {
>  		set_mac_addr(netdev_priv(net_dev));
>  		update_tx_fqids(priv);
> +
> +		rtnl_lock();
> +		if (priv->mac)
> +			dpaa2_eth_disconnect_mac(priv);
> +		else
> +			dpaa2_eth_connect_mac(priv);
> +		rtnl_unlock();

Sorry, but this locking is deadlock prone.

You're taking rtnl_lock().
dpaa2_eth_connect_mac() calls dpaa2_mac_connect().
dpaa2_mac_connect() calls phylink_create().
phylink_create() calls phylink_register_sfp().
phylink_register_sfp() calls sfp_bus_add_upstream().
sfp_bus_add_upstream() calls rtnl_lock() *** DEADLOCK ***.

Neither phylink_create() nor phylink_destroy() may be called holding
the rtnl lock.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up