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

Date: Sun, 21 Jan 2024 22:18:05 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Linus Torvalds' <torvalds@...ux-foundation.org>
CC: Stephen Rothwell <sfr@...b.auug.org.au>, Jiri Slaby <jirislaby@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.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().

From: Linus Torvalds
> Sent: 20 January 2024 21:34
> 
> [ Going through some pending issues now that I've mostly emptied my pull queue ]
> 
> On Wed, 10 Jan 2024 at 14:58, David Laight <David.Laight@...lab.com> wrote:
> >
> > The first check in __types_ok() can go, the second one (with the '+ 0')
> > (added to promote char to int) includes the first one.
> 
> That turns out to not be true. An expression like
> 
>   min(u8, unsigned int)
> 
> is fine because the underlying types are compatible.
> 
> But the promotion to 'int' makes the first argument be a signed
> integer, and is no longer compatible with the second argument.

Yes, I realised that afterwards.

This version is much simpler though.

+/* Allow unsigned compares against non-negative signed constants. */
+#define __is_ok_unsigned(x) \
+       (!is_signed_type(typeof(x)) || (__is_constexpr(x) ? (x) >= 0 : 0))
+
+/* Check for signed after promoting unsigned char/short to int */
+#define __is_ok_signed(x) is_signed_type(typeof((x) + 0))
+
+/* Allow if both x and y are valid for either signed or unsigned compares. */
+#define __types_ok(x, y)                               \
+       ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
+        (__is_ok_unsigned(x) && __is_ok_unsigned(y)))

And _Statc_assert() only needs a compile-time constant, not
a constant expression - so no need for all the __builtin_choose_expr().

I'll post the actual patch series in a couple of days.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)