Re: [PATCH net-next v3 5/8] net: mdio: Add SoC-e SWIP MDIO controller driver

[Vasilij Strassheim]

On Thu, 2026-09-24 at 10:40 +0000, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mdio: null pointer dereference on devm_ioremap failure
> --
> 
> commit b20b4c686880ab360f1cde6275ae5af3a5306146
> Author: Vasilij Strassheim <v.strassheim@xxxxxxxxxxxxx>
> 
> net: mdio: Add SoC-e SWIP MDIO controller driver
>     
> This commit adds a driver for the MDIO controller integrated into SoC-e
> Ethernet switch IP cores. It supports Clause 22 and 45 transactions while
> managing shared transaction control registers.
> 
> > diff --git a/drivers/net/mdio/mdio-soce.c b/drivers/net/mdio/mdio-soce.c
> > new file mode 100644
> > index 0000000000000..59c0b3da483eb
> > --- /dev/null
> > +++ b/drivers/net/mdio/mdio-soce.c
> [ ... ]
> > +static void __iomem *soce_mdio_iomap(struct device *dev, int index)
> > +{
> > +	struct resource res;
> > +	int ret;
> > +
> > +	ret = of_address_to_resource(dev->of_node, index, &res);
> > +	if (ret)
> > +		return IOMEM_ERR_PTR(ret);
> > +
> > +	return devm_ioremap(dev, res.start, resource_size(&res));
> > +}
> [ ... ]
> > +static int soce_mdio_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct soce_mdio *priv;
> > +	struct mii_bus *bus;
> > +
> > +	bus = devm_mdiobus_alloc_size(dev, sizeof(*priv));
> > +	if (!bus)
> > +		return -ENOMEM;
> > +
> > +	priv = bus->priv;
> > +	priv->data = soce_mdio_iomap(dev, SOCE_MDIO_DATA_IOMAP_IDX);
> > +	if (IS_ERR(priv->data))
> > +		return PTR_ERR(priv->data);
> > +
> > +	priv->ctrl = soce_mdio_iomap(dev, SOCE_MDIO_CTRL_IOMAP_IDX);
> > +	if (IS_ERR(priv->ctrl))
> > +		return PTR_ERR(priv->ctrl);
> 
> [Severity: High]
> If devm_ioremap() fails in soce_mdio_iomap(), it returns NULL. Because
> IS_ERR(NULL) evaluates to false, will this bypass the error check and allow
> the driver to proceed with priv->data and priv->ctrl set to NULL?
> 
> If the driver proceeds with NULL pointers, will subsequent PHY accesses 
> through soce_mdio_read() and soce_mdio_write() pass those NULL pointers to 
> readl() and writel(), leading to a kernel panic?

That's true, I mixed up the APIs. That will be corrected with the
redesign to MFD.