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

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: <Y3Y94/My9Al4pw+h@lunn.ch>
Date:   Thu, 17 Nov 2022 14:57:55 +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

On Thu, Nov 17, 2022 at 05:05:14PM +0800, Hui Tang wrote:
> priv->eth_ldo_rdy is saved the return value of devm_ioremap_resource(),
> which !IS_ERR() should be used to check.
> 
> Fixes: 23a890d493e3 ("net: mdio: Add the reset function for IPQ MDIO driver")
> Signed-off-by: Hui Tang <tanghui20@...wei.com>
> ---
> v1 -> v2: set priv->eth_ldo_rdy NULL, if devm_ioremap_resource() failed
> ---
>  drivers/net/mdio/mdio-ipq4019.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
> index 4eba5a91075c..dfd1647eac36 100644
> --- a/drivers/net/mdio/mdio-ipq4019.c
> +++ b/drivers/net/mdio/mdio-ipq4019.c
> @@ -231,8 +231,11 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  	/* The platform resource is provided on the chipset IPQ5018 */
>  	/* This resource is optional */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	if (res)
> +	if (res) {
>  		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
> +		if (IS_ERR(priv->eth_ldo_rdy))
> +			priv->eth_ldo_rdy = NULL;
> +	}

As i said, please add devm_ioremap_resource_optional().  Follow the
concept of devm_clk_get_optional(), devm_gpiod_get_optional(),
devm_reset_control_get_optional(), devm_reset_control_get_optional(),
platform_get_irq_byname_optional() etc.

All these will not return an error if the resource you are trying to
get does not exist. They instead return NULL, or something which other
API members understand as does not exist, but thats O.K.

These functions however do return errors for real problem, ENOMEM,
EINVAL etc. These should not be ignored.

You should then use this new function for all your other patches where
the resource is optional.

       Andrew