linux-kernel - [PATCH 0/1] Slightly relax the type checking done by 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-next>] [day] [month] [year] [list]

Message-ID: <cfc6c0f0fd4c4724890be8a8397c2cbe@AcuMS.aculab.com>
Date:   Fri, 25 Nov 2022 15:00:40 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     LKML <linux-kernel@...r.kernel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>
CC:     Steven Rostedt <rostedt@...dmis.org>,
        'Joe Perches' <joe@...ches.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH 0/1] Slightly relax the type checking done by min() and max().

The min() and max() defines include a type check to avoid the unexpected
  behaviour when a negative value is compared against and unsigned value.
However a lot of code hits this check and uses min_t() to avoid the error.
Many of these are just plain wrong.

Those casting to u8 or u16 are particularly suspect, eg:
drivers/usb/misc/usb251xb.c:528:
		hub->max_current_sp = min_t(u8, property_u32 / 2000, 50);

This patch does two changes:
- Replace typeof(x) with typeof((x) + 0) to promote char/short to int.
- Add an (int) cast to constants between 0 and MAX_INT so the compiler
  doesn't promote the 'other side' of the comparison to an unsinged type.
  If this is done the type test is arranged to always succeed.

The following can also be done (with some lateral thought):
- Allow all comparisons where both types are signed. 
- Allow all comparisons where both types are unsigned. 
- Allow comparisons where the larger type is signed.

In addition most of the min_t() calls are there to compare a signed type
(that holds a non-negative value) with an unsigned value.
The definition:
#define min_unsigned(x,y) min((x) + 0u + 0ull, (y) + 0u + 0ull)
will do an unsigned comparision without 'accidentally' masking off
any non-zero high bits.

With those extra changes there can be a 'duck shoot' on min_t().

David Laight (1):
  Slightly relax the type checking done by min() and max().

 include/linux/minmax.h | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

-- 
2.17.1

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