linux-kernel - Re: [PATCH] backlight: (adp8870) fix a missing check for adp8870

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: <20181227093951.GA14579@ravnborg.org>
Date:   Thu, 27 Dec 2018 10:39:51 +0100
From:   Sam Ravnborg <sam@...nborg.org>
To:     Kangjie Lu <kjlu@....edu>
Cc:     Daniel Thompson <daniel.thompson@...aro.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Jingoo Han <jingoohan1@...il.com>, linux-fbdev@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        pakki001@....edu, Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH] backlight: (adp8870) fix a missing check for
 adp8870_write

Hi Kangjie

> adp8870_write() may fail. This fix checks if adp8870_write fails, and if
> so, returns its error code.
> 
> Signed-off-by: Kangjie Lu <kjlu@....edu>
> ---
>  drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..79901fb4fcd1 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
>  		if (!ret) {
>  			reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
>  			reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> -			adp8870_write(data->client, ADP8870_CFGR, reg_val);
> -		}
> -		mutex_unlock(&data->lock);
> +			ret = adp8870_write(data->client,
> +					ADP8870_CFGR, reg_val);
> +			if (ret) {
> +				mutex_unlock(&data->lock);
> +				return ret;
> +			}
> +		}	else
> +			mutex_unlock(&data->lock);
>  	}

Something looks wrong with the indent.
If you have braces around first part of if () then use
barces also after else. Then it is easier to follow the code.
In this case please consider another approach where you
have only a single mutex_unlock() both in the good and
in the error case.

	Sam