linux-kernel - Re: [PATCH 25/44] drivers/pci: use min() instead of min

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]

Message-ID: <20251124214240.03af61f9@pumpkin>
Date: Mon, 24 Nov 2025 21:42:40 +0000
From: David Laight <david.laight.linux@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org, Bjorn Helgaas
 <bhelgaas@...gle.com>
Subject: Re: [PATCH 25/44] drivers/pci: use min() instead of min_t()

On Mon, 24 Nov 2025 15:04:10 -0600
Bjorn Helgaas <helgaas@...nel.org> wrote:

> On Wed, Nov 19, 2025 at 10:41:21PM +0000, david.laight.linux@...il.com wrote:
> > From: David Laight <david.laight.linux@...il.com>
> > 
> > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > and so cannot discard significant bits.
> > 
> > In this case although pci_hotplug_bus_size is 'long' it is constrained
> > to be <= 255.
> > 
> > Detected by an extra check added to min_t().  
> 
> I don't mind applying this, but it sure would be nice to have a hint
> at the max() and max_t() definitions about when and why to prefer one
> over the other.

When I've sorted out local commits for all the other 400 files I've
had to change to get allmodconfig to build I'm going to look at
minmax.h and checkpatch.pl.

The current bit in checkpatch that suggests transforming
	min(a, (type)b) => min_t(type, a, b)
isn't even a good idea.
The latter is min((type)a, (type)b) - which isn't the same.
at least in with min(a, (type)b)) the truncation is obvious.

I think I'll try to get checkpatch to reject min_t(u8|s8|u16|s16, ...
outright - remember the values get promoted the 'int'.
Unless the code is doing something really obscure they can all be
replaced with a plain min().

Then get minmax.h to reject u8 casts when one of the values is 255
(and u16 casts with 65535) - particularly for clamp_t().
That will error a small number of files - but only a handful.

I need to find the #if that is set for a W=1 build so that I can 'error'
more of the 'dodgy' cases.
That might include all the u8|s8|u16|s16 ones.

But I'll have to leave all the u32 casts of u64 values (on 64bit) for later
(they are typically u32 and size_t).
I've not yet looked for u32 casts of u64 values (long v long long) on 32bit.
I suspect there are some real bugs there.
Maybe I'll try to get them into the W=2 build no one does :-)

I'm retired, need to find something to do ...
(apart from getting to PoGo level 80)

	David