linux-kernel - Re: [ 05/56] drivers/leds/leds-lp5521.c: fix lp5521

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: <1354418247.7995.49.camel@deadeye.wl.decadent.org.uk>
Date:	Sun, 02 Dec 2012 03:17:27 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	alan@...rguk.ukuu.org.uk, Milo <Milo.Kim@...com>,
	Richard Purdie <rpurdie@...ys.net>,
	Bryan Wu <bryan.wu@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [ 05/56] drivers/leds/leds-lp5521.c: fix lp5521_read() error
 handling

On Fri, 2012-11-30 at 10:54 -0800, Greg Kroah-Hartman wrote:
> 3.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Dan Carpenter <dan.carpenter@...cle.com>
> 
> commit 5bc9ad774c063f6b41965e7314f2c26aa5e465a0 upstream.
> 
> Gcc 4.6.2 complains that:
> 
>   drivers/leds/leds-lp5521.c: In function `lp5521_load_program':
>   drivers/leds/leds-lp5521.c:214:21: warning: `mode' may be used uninitialized in this function [-Wuninitialized]
>   drivers/leds/leds-lp5521.c: In function `lp5521_probe':
>   drivers/leds/leds-lp5521.c:788:5: warning: `buf' may be used uninitialized in this function [-Wuninitialized]
>   drivers/leds/leds-lp5521.c:740:6: warning: `ret' may be used uninitialized in this function [-Wuninitialized]
> 
> These are real problems if lp5521_read() returns an error.  When that
> happens we should handle it, instead of ignoring it or doing a bitwise
> OR with all the other error codes and continuing.
[...]
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
[...]
> @@ -785,7 +789,7 @@ static int __devinit lp5521_probe(struct
>  	 * LP5521_REG_ENABLE register will not have any effect - strange!
>  	 */
>  	ret = lp5521_read(client, LP5521_REG_R_CURRENT, &buf);
> -	if (buf != LP5521_REG_R_CURR_DEFAULT) {
> +	if (ret || buf != LP5521_REG_R_CURR_DEFAULT) {
>  		dev_err(&client->dev, "error in resetting chip\n");
>  		goto fail2;
>  	}

lp5521_probe() is going to return 0 if ret == 0 &&
buf != LP5521_REG_R_CURR_DEFAULT here.  It needs to set an error code.

Ben.

-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)