linux-kernel - Re: Buiild error in i915/xe

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: <0f28a0b7-c24d-47d8-92f0-8957207ea309@roeck-us.net>
Date: Sat, 18 Jan 2025 13:59:36 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: David Laight <david.laight.linux@...il.com>,
 David Laight <David.Laight@...lab.com>, Arnd Bergmann <arnd@...nel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Jens Axboe <axboe@...nel.dk>, Matthew Wilcox <willy@...radead.org>,
 Christoph Hellwig <hch@...radead.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Dan Carpenter <dan.carpenter@...aro.org>,
 "Jason A . Donenfeld" <Jason@...c4.com>,
 "pedro.falcato@...il.com" <pedro.falcato@...il.com>,
 Mateusz Guzik <mjguzik@...il.com>, "linux-mm@...ck.org"
 <linux-mm@...ck.org>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 intel-xe@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Jani Nikula <jani.nikula@...ux.intel.com>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>
Subject: Re: Buiild error in i915/xe

On 1/18/25 13:21, Linus Torvalds wrote:
> On Sat, 18 Jan 2025 at 09:49, Guenter Roeck <linux@...ck-us.net> wrote:
>>
>> No idea why the compiler would know that the values are invalid.
> 
> It's not that the compiler knows tat they are invalid, but I bet what
> happens is in scale() (and possibly other places that do similar
> checks), which does this:
> 
>          WARN_ON(source_min > source_max);
>          ...
>          source_val = clamp(source_val, source_min, source_max);
> 
> and the compiler notices that the ordering comparison in the first
> WARN_ON() is the same as the one in clamp(), so it basically converts
> the logic to
> 
>          if (source_min > source_max) {
>                  WARN(..);
>                  /* Do the clamp() knowing that source_min > source_max */
>                  source_val = clamp(source_val, source_min, source_max);
>          } else {
>                  /* Do the clamp knowing that source_min <= source_max */
>                  source_val = clamp(source_val, source_min, source_max);
>          }
> 
> (obviously I dropped the other WARN_ON in the conversion, it wasn't
> relevant for this case).
> 
> And now that first clamp() case is done with source_min > source_max,
> and it triggers that build error because that's invalid.
> 
> So the condition is not statically true in the *source* code, but in
> the "I have moved code around to combine tests" case it now *is*
> statically true as far as the compiler is concerned.
> 

Yes, turns out I can reproduce the problem by adding WARN_ON() ahead
of similar clamp() calls (see below). However, I can only reproduce it
with gcc 13.3 for parisc. I don't see the problem with other cross compilers
(I tried arm, powerpc, and loongarch64). Compilers are weird :-(.

I am not sure what to do here. That kind of problem seems difficult
to avoid, and I am sure we will hit it again elsewhere. Should I declare
gcc 13.x off limits for parisc builds ?

Guenter

---
diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index 505c562a5daa..71c0da31a9d2 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -179,6 +179,7 @@ static void mousedev_abs_event(struct input_dev *dev, struct mousedev *mousedev,
                 if (size == 0)
                         size = xres ? : 1;

+               WARN_ON(min > max);
                 value = clamp(value, min, max);

                 mousedev->packet.x = ((value - min) * xres) / size;