Re: [PATCH net-next v2 7/7] net: ethernet: adi: adin1110: Add OA TC6 protocol support for ADIN1110

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net: adin1110: NULL netdev_ops dereference in
  adin1110_oa_tc6_probe()
--

commit a8946759404622a473f14b5de5bca9f8c82eea9c
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: adi: adin1110: Add OA TC6 protocol support for ADIN1110

This commit adds OPEN Alliance TC6 SPI protocol support for the ADIN1110
using the oa_tc6 framework when the adi,spi-oa-tc6 property is present.

> diff --git a/drivers/net/ethernet/adi/adin1110.c b/drivers/net/ethernet/adi/adin1110.c
> index 53fac3dc8be62..3f054565acb0e 100644
> --- a/drivers/net/ethernet/adi/adin1110.c
> +++ b/drivers/net/ethernet/adi/adin1110.c

[ ... ]

> +static int adin1110_oa_tc6_probe(struct adin1110_priv *priv)
> +{
> +	struct adin1110_port_priv *port_priv;
> +	struct oa_tc6_quirks quirks = {0};
> +	struct net_device *netdev;
> +	struct device *dev;
> +	u32 val;
> +	int ret;
> +
> +	dev = &priv->spidev->dev;
> +
> +	if (priv->cfg->id == ADIN2111_MAC)
> +		return dev_err_probe(dev, -ENODEV,
> +				     "OA TC6 isn't supported for ADIN2111\n");
> +
> +	netdev = devm_alloc_etherdev(dev, sizeof(*port_priv));
> +	if (!netdev)
> +		return -ENOMEM;
> +
> +	port_priv = netdev_priv(netdev);
> +	port_priv->netdev = netdev;
> +	port_priv->priv = priv;
> +	port_priv->cfg = priv->cfg;
> +	port_priv->nr = 0;
> +	priv->ports[0] = port_priv;
> +	SET_NETDEV_DEV(netdev, dev);
> +
> +	quirks.hwreset_settle_us = 50000;
> +	quirks.softreset_settle_us = 10000;
> +
> +	port_priv->tc6 = oa_tc6_init(priv->spidev, netdev, &quirks);
> +	if (PTR_ERR_OR_ZERO(port_priv->tc6) == -EPROTO)
> +		return dev_err_probe(dev, -ENODEV,
> +				     "no OA TC6 reply, check the SPI_CFG1 strap and adi,spi-oa-tc6\n");
> +	if (IS_ERR(port_priv->tc6))
> +		return PTR_ERR(port_priv->tc6);
> +
> +	priv->read_reg = adin1110_read_reg_tc6;
> +	priv->write_reg = adin1110_write_reg_tc6;
> +
> +	ret = devm_add_action_or_reset(dev, adin1110_oa_tc6_remove, port_priv->tc6);
> +	if (ret)
> +		return ret;
> +
> +	ret = adin1110_read_reg(priv, ADIN1110_PHY_ID, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (val != priv->cfg->phy_id_val)
> +		return dev_err_probe(dev, -ENODEV, "PHY ID expected: %x, read: %x\n",
> +				     priv->cfg->phy_id_val, val);
> +
> +	/* The OA TC6 framework doesn't append the FCS, let the MAC do it. */
> +	ret = adin1110_set_bits(priv, ADIN1110_CONFIG2, ADIN1110_CRC_APPEND,
> +				ADIN1110_CRC_APPEND);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (device_get_ethdev_address(dev, netdev))
> +		eth_hw_addr_random(netdev);
> +
> +	netdev->if_port = IF_PORT_10BASET;
> +	netdev->netdev_ops = &adin1110_tc6_netdev_ops;

[Severity: High]
Does calling oa_tc6_init() before setting netdev->netdev_ops cause a NULL
pointer dereference when CONFIG_NET_SHAPER is enabled?

oa_tc6_init() calls oa_tc6_phy_init() -> phy_connect_direct() ->
phy_attach_direct() -> phy_link_topo_add_phy() -> netdev_need_ops_lock(),
which dereferences dev->netdev_ops->net_shaper_ops while netdev->netdev_ops
is still NULL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109085952.105331-1-iustin.stolniceanu@analog.com?part=7