Re: [PATCH net v2] net: mdio-ipq4019: fix possible invalid pointer dereference

From: Andrew Lunn
Date: Fri Nov 18 2022 - 08:44:49 EST


> So, the code should be as follows, is that right?
>
> + void __iomem *devm_ioremap_resource_optional(struct device *dev,
> + const struct resource *res)
> + {
> + void __iomem *base;
> +
> + base = __devm_ioremap_resource(dev, res, DEVM_IOREMAP);
> + if (IS_ERR(base) && PTR_ERR(base) == -ENOMEM)
> + return NULL;
> +
> + return base;
> + }
>
>
> [...]
> res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - if (res)
> + if (res) {
> + priv->eth_ldo_rdy = devm_ioremap_resource_optional(&pdev->dev, res)
> + if (IS_ERR(priv->eth_ldo_rdy))
> + return PTR_ERR(priv->eth_ldo_rdy);
> + }
> [...]

Yes, that is the basic concept.

The only thing i might change is the double meaning of -ENOMEM.
__devm_ioremap_resource() allocates memory, and if that memory
allocation fails, it returns -ENOMEM. If the resource does not exist,
it also returns -ENOMEM. So you cannot tell these two error conditions
apart. Most of the other get_foo() calls return -ENODEV if the
gpio/regulator/clock does not exist, so you can tell if you are out of
memory. But ioremap is specifically about memory so -ENOMEM actually
makes sense.

If you are out of memory, it seems likely the problem is not going to
go away quickly, so the next allocation will also fail, and hopefully
the error handling will then work. So i don't think it is major
issue. So yes, go with the code above.

Andrew