linux-kernel - Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced.

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:   Sun, 26 Jan 2020 12:04:39 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Saurav Girepunje <saurav.girepunje@...il.com>
Cc:     vireshk@...nel.org, johan@...nel.org, elder@...nel.org,
        gregkh@...uxfoundation.org, greybus-dev@...ts.linaro.org,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        saurav.girepunje@...mail.com
Subject: Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced.

On Sun, Jan 26, 2020 at 02:01:30PM +0530, Saurav Girepunje wrote:
> Fix the warning reported by cocci check.
> 
> Changes:
> 
> In queue_work fw dereference before it actually get assigned.
> move queue_work before gb_bootrom_set_timeout.

Nope. As I said yesterday, you need to verify the output of any static
checkers you use.

The code may be unnecessarily subtle, but there's no way fw can be
dereferenced before being initialised currently.

> -queue_work:
>   	/* Refresh timeout */
>   	if (!ret && (offset + size == fw->size))

Specifically, the second operand is never evaluated if ret is non-zero.

>   		next_request = NEXT_REQ_READY_TO_BOOT;
> -	else
> -		next_request = NEXT_REQ_GET_FIRMWARE;
>   
> +queue_work:
>   	gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS);
>   
>   	return ret;

Johan