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

From: Hui Tang
Date: Sat Nov 19 2022 - 00:41:15 EST




On 2022/11/18 21:44, Andrew Lunn wrote:
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.


Hi, Andrew

My new patchset is ready, but I just found out that another patch has been
applied to netdev/net.git. Can I solve the problem in present way? And I
will add devm_ioremap_resource_optional() helper later to optimize related
drivers. How about this?

Thanks.