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

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: <20260131015433.972337-1-kuba@kernel.org>
Date: Fri, 30 Jan 2026 17:54:33 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: lukasz.majewski@...lbox.org
Cc: Jakub Kicinski <kuba@...nel.org>,
	devicetree@...r.kernel.org,
	horms@...nel.org,
	krzk+dt@...nel.org,
	s.hauer@...gutronix.de,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	kernel@...gutronix.de,
	richardcochran@...il.com,
	shawnguo@...nel.org,
	edumazet@...gle.com,
	linux-arm-kernel@...ts.infradead.org,
	andrew+netdev@...n.ch,
	pabeni@...hat.com,
	wahrenst@....net,
	davem@...emloft.net,
	festevam@...il.com,
	conor+dt@...nel.org,
	imx@...ts.linux.dev,
	andrew@...n.ch,
	robh@...nel.org
Subject: 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.

> diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
> new file mode 100644
> index 000000000000..7b61ba9642e3
> --- /dev/null
> +++ b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c

[ ... ]

> +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