linux-kernel - Re: [PATCH next v4 0/5] minmax: Relax type checks in min() and max().

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]

Date: Mon, 8 Jan 2024 12:46:19 +0100
From: Jiri Slaby <jirislaby@...il.com>
To: David Laight <David.Laight@...LAB.COM>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
 'Andy Shevchenko' <andriy.shevchenko@...ux.intel.com>,
 'Andrew Morton' <akpm@...ux-foundation.org>,
 "'Matthew Wilcox (Oracle)'" <willy@...radead.org>,
 'Christoph Hellwig' <hch@...radead.org>,
 "'Jason A. Donenfeld'" <Jason@...c4.com>
Subject: Re: [PATCH next v4 0/5] minmax: Relax type checks in min() and max().

Hi,

On 18. 09. 23, 10:14, David Laight wrote:
> The min() (etc) functions in minmax.h require that the arguments have
> exactly the same types.
> 
> However when the type check fails, rather than look at the types and
> fix the type of a variable/constant, everyone seems to jump on min_t().
> In reality min_t() ought to be rare - when something unusual is being
> done, not normality.
...
> David Laight (5):
>    minmax: Add umin(a, b) and umax(a, b)
>    minmax: Allow min()/max()/clamp() if the arguments have the same
>      signedness.
>    minmax: Fix indentation of __cmp_once() and __clamp_once()
>    minmax: Allow comparisons of 'int' against 'unsigned char/short'.
>    minmax: Relax check to allow comparison between unsigned arguments and
>      signed constants.

This slows down the build and increases the build memory consumption so 
that it causes OOMs.

In particular 6.7:
$ time make drivers/media/pci/solo6x10/solo6x10-p2m.i
...
   CPP [M] drivers/media/pci/solo6x10/solo6x10-p2m.i
real    0m45,002s
user    0m40,840s
sys     0m5,922s

$ git revert 867046cc7027703f60a46339ffde91a1970f2901
$ time make drivers/media/pci/solo6x10/solo6x10-p2m.i
...
   CPP [M] drivers/media/pci/solo6x10/solo6x10-p2m.i
real    0m11,132s
user    0m9,737s
sys     0m1,415s

$ git revert 4ead534fba42fc4fd41163297528d2aa731cd121
$ time make drivers/media/pci/solo6x10/solo6x10-p2m.i
...
   CPP [M] drivers/media/pci/solo6x10/solo6x10-p2m.i
real    0m3,711s
user    0m3,041s
sys     0m0,710s

Note it's only a preprocessor run. If you run a compiler on top of that, 
it even dies.

There is nothing special in that file, just:
if (SOLO_SDRAM_END(solo_dev) > solo_dev->sdram_size) {

which at some point expands to
         max(__SOLO_JPEG_MIN_SIZE(__solo),                               \
             min((__solo->sdram_size - SOLO_JPEG_EXT_ADDR(__solo)), 
0x00ff0000))

and that expands to a lot of stuff.

Note that _line_ is 519 kbytes on 6.6 already. And 6 MB on 6.7.

The file is 4.3M vs. 122M.

Could you investigate/fix/revert (at least) the above two commits?

thanks,
-- 
js