linux-kernel - Re: [PATCH][thermal-next] thermal: brcmstb: remove two redundant integer range checks

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] [day] [month] [year] [list]

Date:   Thu, 7 Sep 2017 19:59:36 -0700
From:   Eduardo Valentin <edubezval@...il.com>
To:     Brian Norris <computersforpeace@...il.com>
Cc:     Colin King <colin.king@...onical.com>,
        Markus Mayer <mmayer@...adcom.com>,
        bcm-kernel-feedback-list@...adcom.com,
        Zhang Rui <rui.zhang@...el.com>,
        Gregory Fong <gregory.0xf0@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        linux-pm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH][thermal-next] thermal: brcmstb: remove two redundant
 integer range checks

Hello,

On Wed, Sep 06, 2017 at 11:40:46AM -0700, Brian Norris wrote:
> Hi,
> 
> On Sun, Sep 03, 2017 at 02:16:19PM +0100, Colin King wrote:
> > From: Colin Ian King <colin.king@...onical.com>
> > 
> > The comparisons for integer low on low > INT_MAX and also
> > integer high > INT_MAX are never going to be true since an
> > int type cannot be greater than INT_MAX.  Remove these redundant
> > checks.
> > 
> > Detected by: CoverityScan CID#1455245, 1455248 ("Operands don't affect
> >   result (CONSTANT_EXPRESSION_RESULT)")
> > 
> > Signed-off-by: Colin Ian King <colin.king@...onical.com>
> > ---
> >  drivers/thermal/broadcom/brcmstb_thermal.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> > index 87b8e7a86ee3..bcb3945feea6 100644
> > --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> > +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> > @@ -278,8 +278,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> >  	dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
> >  
> >  	if (low) {
> > -		if (low > INT_MAX)
> > -			low = INT_MAX;
> >  		avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
> >  		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
> >  	} else {
> > @@ -287,8 +285,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> >  	}
> >  
> >  	if (high < ULONG_MAX) {
> 
> Dan's robots noticed that the above condition is not useful either (on
> architectures where INT_MAX < ULONG_MAX -- i.e., all?), since 'high' is
> 'int', not 'unsigned long'.
> 
> Should probably fix that
> 
> s/ULONG_MAX/INT_MAX/

Yeah, that would probably make sense. I might be also the case that the
driver was considering long because that was the type used to represent
temperature some time back in the thermal subsystem. So, the limits on
the driver may be outdated.

> 
> ?
> 
> Brian
> 
> > -		if (high > INT_MAX)
> > -			high = INT_MAX;
> >  		avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
> >  		avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
> >  	} else {
> > -- 
> > 2.14.1
> >