netdev - Re: [PATCH] can: pch_can: initialize errc before using it

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]

Date:   Thu, 21 Jul 2022 09:02:58 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Vincent Mailhol <mailhol.vincent@...adoo.fr>
Cc:     linux-can@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        netdev@...r.kernel.org, kernel@...gutronix.de,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Frank Jungclaus <frank.jungclaus@....eu>,
        Stefan Mätje <Stefan.Maetje@....eu>
Subject: Re: [PATCH] can: pch_can: initialize errc before using it

On Fri, Jul 22, 2022 at 01:00:32AM +0900, Vincent Mailhol wrote:
> After commit 3a5c7e4611dd, the variable errc is accessed before being
> initialized, c.f. below W=2 warning:
> 
> | In function 'pch_can_error',
> |     inlined from 'pch_can_poll' at drivers/net/can/pch_can.c:739:4:
> | drivers/net/can/pch_can.c:501:29: warning: 'errc' may be used uninitialized [-Wmaybe-uninitialized]
> |   501 |                 cf->data[6] = errc & PCH_TEC;
> |       |                             ^
> | drivers/net/can/pch_can.c: In function 'pch_can_poll':
> | drivers/net/can/pch_can.c:484:13: note: 'errc' was declared here
> |   484 |         u32 errc, lec;
> |       |             ^~~~
> 
> Moving errc initialization up solves this issue.
> 
> Fixes: 3a5c7e4611dd ("can: pch_can: do not report txerr and rxerr during bus-off")
> Reported-by: Nathan Chancellor <nathan@...nel.org>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>

Heh, Marc just sent the same patch. Just in case this one gets picked up
instead:

Reviewed-by: Nathan Chancellor <nathan@...nel.org>

Thanks for the quick response!

> ---
>  drivers/net/can/pch_can.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 50f6719b3aa4..32804fed116c 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -489,6 +489,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
>  	if (!skb)
>  		return;
>  
> +	errc = ioread32(&priv->regs->errc);
>  	if (status & PCH_BUS_OFF) {
>  		pch_can_set_tx_all(priv, 0);
>  		pch_can_set_rx_all(priv, 0);
> @@ -502,7 +503,6 @@ static void pch_can_error(struct net_device *ndev, u32 status)
>  		cf->data[7] = (errc & PCH_REC) >> 8;
>  	}
>  
> -	errc = ioread32(&priv->regs->errc);
>  	/* Warning interrupt. */
>  	if (status & PCH_EWARN) {
>  		state = CAN_STATE_ERROR_WARNING;
> -- 
> 2.35.1
>