linux-kernel - Re: [PATCH 08/10] mfd: atmel-smc: Silence comparison of unsigned expression

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: <20200630121023.GL1179328@dell>
Date:   Tue, 30 Jun 2020 13:10:23 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>
Subject: Re: [PATCH 08/10] mfd: atmel-smc: Silence comparison of unsigned
 expression < 0 is always false warning (W=1)

On Fri, 26 Jun 2020, Alexandre Belloni wrote:

> Hi,
> 
> On 25/06/2020 07:46:17+0100, Lee Jones wrote:
> > GENMASK and it's callees conduct checking to ensure the passed
> > parameters are valid.  One of those checks is for '< 0'.  So if an
> > unsigned value is passed, in an invalid comparison takes place.
> > 
> > Judging from the current code, it looks as though 'unsigned int'
> > is the correct type to use, so simply cast these small values
> > with no chance of being false negative to signed int for
> > comparison/error checking purposes.
> > 
> 
> I've been thinking about that one but shouldn't the proper fix be in
> GENMASK? My understanding is that this happens because l is 0 and I
> don't think GENMASK would ever expect negative number. What about simply
> checking that h != l when l is 0?

Looks like Rikard Falkeborn recently submitted a patch to 'fix' this
issue.  However, Linus slammed it, decrying that it's the warning
that's wrong, not the code.  Looks like the type-limits warning might
be getting downgraded to W=2 instead (although I don't see a patch
yet).

I'm going to drop this patch.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog