Re: [net-next,v21,2/7] net: mtip: The L2 switch driver for imx287

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: mtip: The L2 switch driver for imx287

This patch introduces support for the MTIP L2 switch embedded in the
imx287 SoC. It provides the core driver infrastructure including DMA
initialization, PHY/MDIO handling, and net device operations.

[ ... ]

+static int mtip_mii_probe(struct net_device *dev)
+{
+	struct mtip_ndev_priv *priv = netdev_priv(dev);
+	struct switch_enet_private *fep = priv->fep;
+	struct phy_device *phy_dev = NULL;
+	int port_idx = priv->portnum - 1;
+
+	if (fep->phy_np[port_idx]) {
+		phy_dev = of_phy_connect(dev, fep->phy_np[port_idx],
+					 &mtip_adjust_link, 0,
+					 fep->phy_interface[port_idx]);
+		if (!phy_dev) {
+			netdev_err(dev, "Unable to connect to phy\n");
+			return -ENODEV;
+		}
+	}
+
+	phy_set_max_speed(phy_dev, 100);

If fep->phy_np[port_idx] is NULL (no phy-handle in device tree), the
if block is skipped entirely. In that case phy_dev remains NULL from
initialization, and phy_set_max_speed() is called with a NULL pointer.

phy_set_max_speed() dereferences phydev->supported via __set_phy_supported(),
which would cause a NULL pointer dereference crash when the interface
is opened.

Should phy_set_max_speed() and the subsequent accesses to
fep->phy_dev[port_idx] be moved inside the if block, or should the
function return an error when phy_np is NULL?

+	fep->phy_dev[port_idx] = phy_dev;
+	fep->link[port_idx] = 0;
+	fep->full_duplex[port_idx] = 0;
+
+	dev_dbg(&dev->dev,
+		"MTIP PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
+		fep->phy_dev[port_idx]->drv->name,

This debug print also dereferences fep->phy_dev[port_idx]->drv->name,
which would crash if phy_dev is NULL.

+		phydev_name(fep->phy_dev[port_idx]),
+		fep->phy_dev[port_idx]->irq);
+
+	return 0;
+}

-- 
pw-bot: cr