linux-kernel - Re: [PATCH 1/7 v2] net: bcmgenet: Fix platform_get

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: <7fd6f043-3e66-a03a-e2fa-c71cffe5019e@gmail.com>
Date:   Mon, 4 Dec 2017 17:01:57 -0800
From:   Doug Berger <opendmb@...il.com>
To:     Arvind Yadav <arvind.yadav.cs@...il.com>, wg@...ndegger.com,
        mkl@...gutronix.de, michal.simek@...inx.com, f.fainelli@...il.com,
        davem@...emloft.net
Cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/7 v2] net: bcmgenet: Fix platform_get_irq's error
 checking

On 12/04/2017 09:48 AM, Arvind Yadav wrote:
> The platform_get_irq() function returns negative number if an error occurs,
> Zero if No irq is found and positive number if irq gets successful.
> platform_get_irq() error checking only for zero is not correct.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@...il.com>
> ---
> changes in v2:
>              commit message was not correct.
> 
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 24b4f4c..e2f1268 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3371,7 +3371,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
>  	priv->irq0 = platform_get_irq(pdev, 0);
>  	priv->irq1 = platform_get_irq(pdev, 1);
>  	priv->wol_irq = platform_get_irq(pdev, 2);
> -	if (!priv->irq0 || !priv->irq1) {
> +	if (priv->irq0 <= 0 || priv->irq1 <= 0 || priv->wol_irq <= 0) {
>  		dev_err(&pdev->dev, "can't find IRQs\n");
>  		err = -EINVAL;
>  		goto err;
> 

The absence of a Wake-on-LAN interrupt (wol_irq <= 0) is not a terminal
error for the driver so it should not be included in this check.

The error checking for irq0 and irq1 is appropriate to add, but it
sounds like David Miller is proposing changing platform_get_irq() so
I'll let that dust settle before saying whether <= or < is appropriate.

Thanks,
    Doug