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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y3eMMc7maaPCKUNS@lunn.ch>
Date:   Fri, 18 Nov 2022 14:44:17 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Hui Tang <tanghui20@...wei.com>
Cc:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        mw@...ihalf.com, linux@...linux.org.uk, pabeni@...hat.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        yusongping@...wei.com
Subject: Re: [PATCH net v2] net: mdio-ipq4019: fix possible invalid pointer
 dereference

> 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ