[PATCH 00/44] Change a lot of min_t() that might mask high bits

[PATCH 00/44] Change a lot of min_t() that might mask high bits

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

It in not uncommon for code to use min_t(uint, a, b) when one of a or b
is 64bit and can have a value that is larger than 2^32;
This is particularly prevelant with:
	uint_var = min_t(uint, uint_var, uint64_expression);

Casts to u8 and u16 are very likely to discard significant bits.

These can be detected at compile time by changing min_t(), for example:
#define CHECK_SIZE(fn, type, val) \
	BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
		!statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
		fn "() significant bits of '" #val "' may be discarded")

#define min_t(type, x, y) ({ \
	CHECK_SIZE("min_t", type, x); \
	CHECK_SIZE("min_t", type, y); \
	__cmp_once(min, type, x, y); })

(and similar changes to max_t() and clamp_t().)

This shows up some real bugs, some unlikely bugs and some false positives.
In most cases both arguments are unsigned type (just different ones)
and min_t() can just be replaced by min().

The patches are all independant and are most of the ones needed to
get the x86-64 kernel I build to compile.
I've not tried building an allyesconfig or allmodconfig kernel.
I've also not included the patch to minmax.h itself.

I've tried to put the patches that actually fix things first.
The last one is 0009.

I gave up on fixing sched/fair.c - it is too broken for a single patch!
The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
needs multiple/larger changes to make it 'sane'.

I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
from 124 to under 100 to be able to send the cover letter.
The individual patches only go to the addresses found for the associated files.
That reduces the number of emails to a less unsane number.

David Laight (44):
  x86/asm/bitops: Change the return type of variable__ffs() to unsigned
    int
  ext4: Fix saturation of 64bit inode times for old filesystems
  perf: Fix branch stack callchain limit
  io_uring/net: Change some dubious min_t()
  ipc/msg: Fix saturation of percpu counts in msgctl_info()
  bpf: Verifier, remove some unusual uses of min_t() and max_t()
  net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
  net: ethtool: Use min3() instead of nested min_t(u16,...)
  ipv6: __ip6_append_data() don't abuse max_t() casts
  x86/crypto: ctr_crypt() use min() instead of min_t()
  arch/x96/kvm: use min() instead of min_t()
  block: use min() instead of min_t()
  drivers/acpi: use min() instead of min_t()
  drivers/char/hw_random: use min3() instead of nested min_t()
  drivers/char/tpm: use min() instead of min_t()
  drivers/crypto/ccp: use min() instead of min_t()
  drivers/cxl: use min() instead of min_t()
  drivers/gpio: use min() instead of min_t()
  drivers/gpu/drm/amd: use min() instead of min_t()
  drivers/i2c/busses: use min() instead of min_t()
  drivers/net/ethernet/realtek: use min() instead of min_t()
  drivers/nvme: use min() instead of min_t()
  arch/x86/mm: use min() instead of min_t()
  drivers/nvmem: use min() instead of min_t()
  drivers/pci: use min() instead of min_t()
  drivers/scsi: use min() instead of min_t()
  drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
    limits
  drivers/usb/storage: use min() instead of min_t()
  drivers/xen: use min() instead of min_t()
  fs: use min() or umin() instead of min_t()
  block: bvec.h: use min() instead of min_t()
  nodemask: use min() instead of min_t()
  ipc: use min() instead of min_t()
  bpf: use min() instead of min_t()
  bpf_trace: use min() instead of min_t()
  lib/bucket_locks: use min() instead of min_t()
  lib/crypto/mpi: use min() instead of min_t()
  lib/dynamic_queue_limits: use max() instead of max_t()
  mm: use min() instead of min_t()
  net: Don't pass bitfields to max_t()
  net/core: Change loop conditions so min() can be used
  net: use min() instead of min_t()
  net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
  net/mptcp: Change some dubious min_t(int, ...) to min()

 arch/x86/crypto/aesni-intel_glue.c            |  3 +-
 arch/x86/include/asm/bitops.h                 | 18 +++++-------
 arch/x86/kvm/emulate.c                        |  3 +-
 arch/x86/kvm/lapic.c                          |  2 +-
 arch/x86/kvm/mmu/mmu.c                        |  2 +-
 arch/x86/mm/pat/set_memory.c                  | 12 ++++----
 block/blk-iocost.c                            |  6 ++--
 block/blk-settings.c                          |  2 +-
 block/partitions/efi.c                        |  3 +-
 drivers/acpi/property.c                       |  2 +-
 drivers/char/hw_random/core.c                 |  2 +-
 drivers/char/tpm/tpm1-cmd.c                   |  2 +-
 drivers/char/tpm/tpm_tis_core.c               |  4 +--
 drivers/crypto/ccp/ccp-dev.c                  |  2 +-
 drivers/cxl/core/mbox.c                       |  2 +-
 drivers/gpio/gpiolib-acpi-core.c              |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  2 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 drivers/i2c/busses/i2c-designware-master.c    |  2 +-
 drivers/net/ethernet/realtek/r8169_main.c     |  3 +-
 drivers/nvme/host/pci.c                       |  3 +-
 drivers/nvme/host/zns.c                       |  3 +-
 drivers/nvmem/core.c                          |  2 +-
 drivers/pci/probe.c                           |  3 +-
 drivers/scsi/hosts.c                          |  2 +-
 drivers/tty/vt/selection.c                    |  9 +++---
 drivers/usb/storage/protocol.c                |  3 +-
 drivers/xen/grant-table.c                     |  2 +-
 fs/buffer.c                                   |  2 +-
 fs/exec.c                                     |  2 +-
 fs/ext4/ext4.h                                |  2 +-
 fs/ext4/mballoc.c                             |  3 +-
 fs/ext4/resize.c                              |  2 +-
 fs/ext4/super.c                               |  2 +-
 fs/fat/dir.c                                  |  4 +--
 fs/fat/file.c                                 |  3 +-
 fs/fuse/dev.c                                 |  2 +-
 fs/fuse/file.c                                |  8 ++---
 fs/splice.c                                   |  2 +-
 include/linux/bvec.h                          |  3 +-
 include/linux/nodemask.h                      |  9 +++---
 include/linux/perf_event.h                    |  2 +-
 include/net/tcp_ecn.h                         |  5 ++--
 io_uring/net.c                                |  6 ++--
 ipc/mqueue.c                                  |  4 +--
 ipc/msg.c                                     |  6 ++--
 kernel/bpf/core.c                             |  4 +--
 kernel/bpf/log.c                              |  2 +-
 kernel/bpf/verifier.c                         | 29 +++++++------------
 kernel/trace/bpf_trace.c                      |  2 +-
 lib/bucket_locks.c                            |  2 +-
 lib/crypto/mpi/mpicoder.c                     |  2 +-
 lib/dynamic_queue_limits.c                    |  2 +-
 mm/gup.c                                      |  4 +--
 mm/memblock.c                                 |  2 +-
 mm/memory.c                                   |  2 +-
 mm/percpu.c                                   |  2 +-
 mm/truncate.c                                 |  3 +-
 mm/vmscan.c                                   |  2 +-
 net/core/datagram.c                           |  6 ++--
 net/core/flow_dissector.c                     |  7 ++---
 net/core/net-sysfs.c                          |  3 +-
 net/core/skmsg.c                              |  4 +--
 net/ethtool/cmis_cdb.c                        |  7 ++---
 net/ipv4/fib_trie.c                           |  2 +-
 net/ipv4/tcp_input.c                          |  4 +--
 net/ipv4/tcp_output.c                         |  5 ++--
 net/ipv4/tcp_timer.c                          |  4 +--
 net/ipv6/addrconf.c                           |  8 ++---
 net/ipv6/ip6_output.c                         |  7 +++--
 net/ipv6/ndisc.c                              |  5 ++--
 net/mptcp/protocol.c                          |  8 ++---
 net/netlink/genetlink.c                       |  9 +++---
 net/packet/af_packet.c                        |  2 +-
 net/unix/af_unix.c                            |  4 +--
 76 files changed, 141 insertions(+), 176 deletions(-)

-- 
2.39.5

[PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

min_t() and max_t() are normally used to change the signedness
of a positive value to avoid a signed-v-unsigned compare warning.

However they are used here to convert an unsigned 64bit pattern
to a signed to a 32/64bit signed number.
To avoid any confusion use plain min()/max() and explicitely cast
the u64 expression to the correct signed value.

Use a simple max() for the max_pkt_offset calulation and delete the
comment about why the cast to u32 is safe.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 kernel/bpf/verifier.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ff40e5e65c43..22fa9769fbdb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
 	struct tnum var32_off = tnum_subreg(reg->var_off);
 
 	/* min signed is max(sign bit) | min(other bits) */
-	reg->s32_min_value = max_t(s32, reg->s32_min_value,
-			var32_off.value | (var32_off.mask & S32_MIN));
+	reg->s32_min_value = max(reg->s32_min_value,
+			(s32)(var32_off.value | (var32_off.mask & S32_MIN)));
 	/* max signed is min(sign bit) | max(other bits) */
-	reg->s32_max_value = min_t(s32, reg->s32_max_value,
-			var32_off.value | (var32_off.mask & S32_MAX));
-	reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
+	reg->s32_max_value = min(reg->s32_max_value,
+			(s32)(var32_off.value | (var32_off.mask & S32_MAX)));
+	reg->u32_min_value = max(reg->u32_min_value, (u32)var32_off.value);
 	reg->u32_max_value = min(reg->u32_max_value,
 				 (u32)(var32_off.value | var32_off.mask));
 }
@@ -2332,11 +2332,11 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
 static void __update_reg64_bounds(struct bpf_reg_state *reg)
 {
 	/* min signed is max(sign bit) | min(other bits) */
-	reg->smin_value = max_t(s64, reg->smin_value,
-				reg->var_off.value | (reg->var_off.mask & S64_MIN));
+	reg->smin_value = max(reg->smin_value,
+				(s64)(reg->var_off.value | (reg->var_off.mask & S64_MIN)));
 	/* max signed is min(sign bit) | max(other bits) */
-	reg->smax_value = min_t(s64, reg->smax_value,
-				reg->var_off.value | (reg->var_off.mask & S64_MAX));
+	reg->smax_value = min(reg->smax_value,
+				(s64)(reg->var_off.value | (reg->var_off.mask & S64_MAX)));
 	reg->umin_value = max(reg->umin_value, reg->var_off.value);
 	reg->umax_value = min(reg->umax_value,
 			      reg->var_off.value | reg->var_off.mask);
@@ -6128,15 +6128,8 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
 		return err;
 	}
 
-	/* __check_mem_access has made sure "off + size - 1" is within u16.
-	 * reg->umax_value can't be bigger than MAX_PACKET_OFF which is 0xffff,
-	 * otherwise find_good_pkt_pointers would have refused to set range info
-	 * that __check_mem_access would have rejected this pkt access.
-	 * Therefore, "off + reg->umax_value + size - 1" won't overflow u32.
-	 */
-	env->prog->aux->max_pkt_offset =
-		max_t(u32, env->prog->aux->max_pkt_offset,
-		      off + reg->umax_value + size - 1);
+	env->prog->aux->max_pkt_offset = max(env->prog->aux->max_pkt_offset,
+					     off + reg->umax_value + size - 1);
 
 	return err;
 }
-- 
2.39.5

Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[Alexei Starovoitov]

On Wed, Nov 19, 2025 at 2:42 PM <david.laight.linux@gmail.com> wrote:
>
> From: David Laight <david.laight.linux@gmail.com>
>
> min_t() and max_t() are normally used to change the signedness
> of a positive value to avoid a signed-v-unsigned compare warning.
>
> However they are used here to convert an unsigned 64bit pattern
> to a signed to a 32/64bit signed number.
> To avoid any confusion use plain min()/max() and explicitely cast
> the u64 expression to the correct signed value.
>
> Use a simple max() for the max_pkt_offset calulation and delete the
> comment about why the cast to u32 is safe.
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  kernel/bpf/verifier.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ff40e5e65c43..22fa9769fbdb 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
>         struct tnum var32_off = tnum_subreg(reg->var_off);
>
>         /* min signed is max(sign bit) | min(other bits) */
> -       reg->s32_min_value = max_t(s32, reg->s32_min_value,
> -                       var32_off.value | (var32_off.mask & S32_MIN));
> +       reg->s32_min_value = max(reg->s32_min_value,
> +                       (s32)(var32_off.value | (var32_off.mask & S32_MIN)));
>         /* max signed is min(sign bit) | max(other bits) */
> -       reg->s32_max_value = min_t(s32, reg->s32_max_value,
> -                       var32_off.value | (var32_off.mask & S32_MAX));
> -       reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
> +       reg->s32_max_value = min(reg->s32_max_value,
> +                       (s32)(var32_off.value | (var32_off.mask & S32_MAX)));

Nack.
This is plain ugly for no good reason.
Leave the code as-is.

pw-bot: cr

Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[David Laight]

On Fri, 21 Nov 2025 13:40:36 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Wed, Nov 19, 2025 at 2:42 PM <david.laight.linux@gmail.com> wrote:
> >
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > min_t() and max_t() are normally used to change the signedness
> > of a positive value to avoid a signed-v-unsigned compare warning.
> >
> > However they are used here to convert an unsigned 64bit pattern
> > to a signed to a 32/64bit signed number.
> > To avoid any confusion use plain min()/max() and explicitely cast
> > the u64 expression to the correct signed value.
> >
> > Use a simple max() for the max_pkt_offset calulation and delete the
> > comment about why the cast to u32 is safe.
> >
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> >  kernel/bpf/verifier.c | 29 +++++++++++------------------
> >  1 file changed, 11 insertions(+), 18 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index ff40e5e65c43..22fa9769fbdb 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
> >         struct tnum var32_off = tnum_subreg(reg->var_off);
> >
> >         /* min signed is max(sign bit) | min(other bits) */
> > -       reg->s32_min_value = max_t(s32, reg->s32_min_value,
> > -                       var32_off.value | (var32_off.mask & S32_MIN));
> > +       reg->s32_min_value = max(reg->s32_min_value,
> > +                       (s32)(var32_off.value | (var32_off.mask & S32_MIN)));
> >         /* max signed is min(sign bit) | max(other bits) */
> > -       reg->s32_max_value = min_t(s32, reg->s32_max_value,
> > -                       var32_off.value | (var32_off.mask & S32_MAX));
> > -       reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
> > +       reg->s32_max_value = min(reg->s32_max_value,
> > +                       (s32)(var32_off.value | (var32_off.mask & S32_MAX)));  
> 
> Nack.
> This is plain ugly for no good reason.
> Leave the code as-is.

It is really horrid before.
From what i remember var32_off.value (and .mask) are both u64.
The pattern actually patches that used a few lines down the file.

I've been trying to build allmodconfig with the size test added to min_t()
and max_t().
The number of real (or potentially real) bugs I've found is stunning.
The only fix is to nuke min_t() and max_t() to they can't be used.

The basic problem is the people have used the type of the target not that
of the largest parameter.
The might be ok for ulong v uint (on 64bit), but there are plenty of places
where u16 and u8 are used - a lot are pretty much buggy.

Perhaps the worst ones I've found are with clamp_t(),
this is from 2/44:
-		(raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX));	\
+		(raw_inode)->xtime = cpu_to_le32(clamp((ts).tv_sec, S32_MIN, S32_MAX));	\
If also found clamp_t(u8, xxx, 0, 255).

There are just so many broken examples.

	David




> 
> pw-bot: cr

Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[Alexei Starovoitov]

On Fri, Nov 21, 2025 at 2:21 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Fri, 21 Nov 2025 13:40:36 -0800
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> > On Wed, Nov 19, 2025 at 2:42 PM <david.laight.linux@gmail.com> wrote:
> > >
> > > From: David Laight <david.laight.linux@gmail.com>
> > >
> > > min_t() and max_t() are normally used to change the signedness
> > > of a positive value to avoid a signed-v-unsigned compare warning.
> > >
> > > However they are used here to convert an unsigned 64bit pattern
> > > to a signed to a 32/64bit signed number.
> > > To avoid any confusion use plain min()/max() and explicitely cast
> > > the u64 expression to the correct signed value.
> > >
> > > Use a simple max() for the max_pkt_offset calulation and delete the
> > > comment about why the cast to u32 is safe.
> > >
> > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > ---
> > >  kernel/bpf/verifier.c | 29 +++++++++++------------------
> > >  1 file changed, 11 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index ff40e5e65c43..22fa9769fbdb 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
> > >         struct tnum var32_off = tnum_subreg(reg->var_off);
> > >
> > >         /* min signed is max(sign bit) | min(other bits) */
> > > -       reg->s32_min_value = max_t(s32, reg->s32_min_value,
> > > -                       var32_off.value | (var32_off.mask & S32_MIN));
> > > +       reg->s32_min_value = max(reg->s32_min_value,
> > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MIN)));
> > >         /* max signed is min(sign bit) | max(other bits) */
> > > -       reg->s32_max_value = min_t(s32, reg->s32_max_value,
> > > -                       var32_off.value | (var32_off.mask & S32_MAX));
> > > -       reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
> > > +       reg->s32_max_value = min(reg->s32_max_value,
> > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MAX)));
> >
> > Nack.
> > This is plain ugly for no good reason.
> > Leave the code as-is.
>
> It is really horrid before.
> From what i remember var32_off.value (and .mask) are both u64.
> The pattern actually patches that used a few lines down the file.
>
> I've been trying to build allmodconfig with the size test added to min_t()
> and max_t().
> The number of real (or potentially real) bugs I've found is stunning.
> The only fix is to nuke min_t() and max_t() to they can't be used.

No. min_t() is going to stay. It's not broken and
this crusade against it is inappropriate.

> The basic problem is the people have used the type of the target not that
> of the largest parameter.
> The might be ok for ulong v uint (on 64bit), but there are plenty of places
> where u16 and u8 are used - a lot are pretty much buggy.
>
> Perhaps the worst ones I've found are with clamp_t(),
> this is from 2/44:
> -               (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX));      \
> +               (raw_inode)->xtime = cpu_to_le32(clamp((ts).tv_sec, S32_MIN, S32_MAX)); \
> If also found clamp_t(u8, xxx, 0, 255).
>
> There are just so many broken examples.

clamp_t(u8, xxx, 0, 255) is not wrong. It's silly, but
it's doing the right thing and one can argue and explicit
clamp values serve as a documentation.
clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX)) is indeed incorrect,
but it's a bug in the implementation of __clamp_once().
Fix it, instead of spamming people with "_t" removal.

Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[David Laight]

On Sun, 23 Nov 2025 08:39:51 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Fri, Nov 21, 2025 at 2:21 PM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Fri, 21 Nov 2025 13:40:36 -0800
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >  
> > > On Wed, Nov 19, 2025 at 2:42 PM <david.laight.linux@gmail.com> wrote:  
> > > >
> > > > From: David Laight <david.laight.linux@gmail.com>
> > > >
> > > > min_t() and max_t() are normally used to change the signedness
> > > > of a positive value to avoid a signed-v-unsigned compare warning.
> > > >
> > > > However they are used here to convert an unsigned 64bit pattern
> > > > to a signed to a 32/64bit signed number.
> > > > To avoid any confusion use plain min()/max() and explicitely cast
> > > > the u64 expression to the correct signed value.
> > > >
> > > > Use a simple max() for the max_pkt_offset calulation and delete the
> > > > comment about why the cast to u32 is safe.
> > > >
> > > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > > ---
> > > >  kernel/bpf/verifier.c | 29 +++++++++++------------------
> > > >  1 file changed, 11 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > index ff40e5e65c43..22fa9769fbdb 100644
> > > > --- a/kernel/bpf/verifier.c
> > > > +++ b/kernel/bpf/verifier.c
> > > > @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
> > > >         struct tnum var32_off = tnum_subreg(reg->var_off);
> > > >
> > > >         /* min signed is max(sign bit) | min(other bits) */
> > > > -       reg->s32_min_value = max_t(s32, reg->s32_min_value,
> > > > -                       var32_off.value | (var32_off.mask & S32_MIN));
> > > > +       reg->s32_min_value = max(reg->s32_min_value,
> > > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MIN)));
> > > >         /* max signed is min(sign bit) | max(other bits) */
> > > > -       reg->s32_max_value = min_t(s32, reg->s32_max_value,
> > > > -                       var32_off.value | (var32_off.mask & S32_MAX));
> > > > -       reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
> > > > +       reg->s32_max_value = min(reg->s32_max_value,
> > > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MAX)));  
> > >
> > > Nack.
> > > This is plain ugly for no good reason.
> > > Leave the code as-is.  
> >
> > It is really horrid before.
> > From what i remember var32_off.value (and .mask) are both u64.
> > The pattern actually patches that used a few lines down the file.
> >
> > I've been trying to build allmodconfig with the size test added to min_t()
> > and max_t().
> > The number of real (or potentially real) bugs I've found is stunning.
> > The only fix is to nuke min_t() and max_t() to they can't be used.  
> 
> No. min_t() is going to stay. It's not broken and
> this crusade against it is inappropriate.

I bet to differ...

> > The basic problem is the people have used the type of the target not that
> > of the largest parameter.
> > The might be ok for ulong v uint (on 64bit), but there are plenty of places
> > where u16 and u8 are used - a lot are pretty much buggy.
> >
> > Perhaps the worst ones I've found are with clamp_t(),
> > this is from 2/44:
> > -               (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX));      \
> > +               (raw_inode)->xtime = cpu_to_le32(clamp((ts).tv_sec, S32_MIN, S32_MAX)); \
> > If also found clamp_t(u8, xxx, 0, 255).
> >
> > There are just so many broken examples.  
> 
> clamp_t(u8, xxx, 0, 255) is not wrong. It's silly, but
> it's doing the right thing and one can argue and explicit
> clamp values serve as a documentation.

Not when you look at some of the code that uses it.
The clear intention is to saturate a large value - which isn't what it does.

> clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX)) is indeed incorrect,
> but it's a bug in the implementation of __clamp_once().
> Fix it, instead of spamming people with "_t" removal.

It is too late by the time you get to clamp_once().
The 'type' for all the xxx_t() functions is an input cast, not the type
for the result.
clamp_t(type, v, lo, hi) has always been clamp((type)v, (type)lo, type(hi)).
From a code correctness point of view you pretty much never want those casts.

I've already fixed clamp() so it doesn't complain about comparing s64 against s32.
The next stage is to change pretty much all the xxx_t() to plain xxx().

If you've got some spare time try issuing read calls with a 4GB buffer to all
the subsystems you can find - and see how many loop for ever.
(I think you can do that with readv() and a single buffer.)
The issue there is that a lot use min_t(u32, max_frag_size, xfer_size) to split
operations - and xfer_size is size_t (so I'm pretty sure there are ways to get
4GB in there).

	David

Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[Alexei Starovoitov]

On Sun, Nov 23, 2025 at 10:07 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sun, 23 Nov 2025 08:39:51 -0800
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> > On Fri, Nov 21, 2025 at 2:21 PM David Laight
> > <david.laight.linux@gmail.com> wrote:
> > >
> > > On Fri, 21 Nov 2025 13:40:36 -0800
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > >
> > > > On Wed, Nov 19, 2025 at 2:42 PM <david.laight.linux@gmail.com> wrote:
> > > > >
> > > > > From: David Laight <david.laight.linux@gmail.com>
> > > > >
> > > > > min_t() and max_t() are normally used to change the signedness
> > > > > of a positive value to avoid a signed-v-unsigned compare warning.
> > > > >
> > > > > However they are used here to convert an unsigned 64bit pattern
> > > > > to a signed to a 32/64bit signed number.
> > > > > To avoid any confusion use plain min()/max() and explicitely cast
> > > > > the u64 expression to the correct signed value.
> > > > >
> > > > > Use a simple max() for the max_pkt_offset calulation and delete the
> > > > > comment about why the cast to u32 is safe.
> > > > >
> > > > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > > > ---
> > > > >  kernel/bpf/verifier.c | 29 +++++++++++------------------
> > > > >  1 file changed, 11 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > > index ff40e5e65c43..22fa9769fbdb 100644
> > > > > --- a/kernel/bpf/verifier.c
> > > > > +++ b/kernel/bpf/verifier.c
> > > > > @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
> > > > >         struct tnum var32_off = tnum_subreg(reg->var_off);
> > > > >
> > > > >         /* min signed is max(sign bit) | min(other bits) */
> > > > > -       reg->s32_min_value = max_t(s32, reg->s32_min_value,
> > > > > -                       var32_off.value | (var32_off.mask & S32_MIN));
> > > > > +       reg->s32_min_value = max(reg->s32_min_value,
> > > > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MIN)));
> > > > >         /* max signed is min(sign bit) | max(other bits) */
> > > > > -       reg->s32_max_value = min_t(s32, reg->s32_max_value,
> > > > > -                       var32_off.value | (var32_off.mask & S32_MAX));
> > > > > -       reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
> > > > > +       reg->s32_max_value = min(reg->s32_max_value,
> > > > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MAX)));
> > > >
> > > > Nack.
> > > > This is plain ugly for no good reason.
> > > > Leave the code as-is.
> > >
> > > It is really horrid before.
> > > From what i remember var32_off.value (and .mask) are both u64.
> > > The pattern actually patches that used a few lines down the file.
> > >
> > > I've been trying to build allmodconfig with the size test added to min_t()
> > > and max_t().
> > > The number of real (or potentially real) bugs I've found is stunning.
> > > The only fix is to nuke min_t() and max_t() to they can't be used.
> >
> > No. min_t() is going to stay. It's not broken and
> > this crusade against it is inappropriate.
>
> I bet to differ...
>
> > > The basic problem is the people have used the type of the target not that
> > > of the largest parameter.
> > > The might be ok for ulong v uint (on 64bit), but there are plenty of places
> > > where u16 and u8 are used - a lot are pretty much buggy.
> > >
> > > Perhaps the worst ones I've found are with clamp_t(),
> > > this is from 2/44:
> > > -               (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX));      \
> > > +               (raw_inode)->xtime = cpu_to_le32(clamp((ts).tv_sec, S32_MIN, S32_MAX)); \
> > > If also found clamp_t(u8, xxx, 0, 255).
> > >
> > > There are just so many broken examples.
> >
> > clamp_t(u8, xxx, 0, 255) is not wrong. It's silly, but
> > it's doing the right thing and one can argue and explicit
> > clamp values serve as a documentation.
>
> Not when you look at some of the code that uses it.
> The clear intention is to saturate a large value - which isn't what it does.
>
> > clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX)) is indeed incorrect,
> > but it's a bug in the implementation of __clamp_once().
> > Fix it, instead of spamming people with "_t" removal.
>
> It is too late by the time you get to clamp_once().
> The 'type' for all the xxx_t() functions is an input cast, not the type
> for the result.
> clamp_t(type, v, lo, hi) has always been clamp((type)v, (type)lo, type(hi)).
> From a code correctness point of view you pretty much never want those casts.

Historical behavior doesn't justify a footgun.
You definitely can make clampt_t() to behave like clamp_val() plus
the final cast.

Also note:
git grep -w clamp_val|wc -l
818
git grep -w clamp_t|wc -l
494

a safer macro is already used more often.

> I've already fixed clamp() so it doesn't complain about comparing s64 against s32.
> The next stage is to change pretty much all the xxx_t() to plain xxx().

Nack to that. Fix the problem. Not the symptom.

Re: [PATCH 06/44] bpf: Verifier, remove some unusual uses of min_t() and max_t()

[David Laight]

On Sun, 23 Nov 2025 11:20:03 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Sun, Nov 23, 2025 at 10:07 AM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Sun, 23 Nov 2025 08:39:51 -0800
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >  
> > > On Fri, Nov 21, 2025 at 2:21 PM David Laight
> > > <david.laight.linux@gmail.com> wrote:  
> > > >
> > > > On Fri, 21 Nov 2025 13:40:36 -0800
> > > > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > >  
> > > > > On Wed, Nov 19, 2025 at 2:42 PM <david.laight.linux@gmail.com> wrote:  
> > > > > >
> > > > > > From: David Laight <david.laight.linux@gmail.com>
> > > > > >
> > > > > > min_t() and max_t() are normally used to change the signedness
> > > > > > of a positive value to avoid a signed-v-unsigned compare warning.
> > > > > >
> > > > > > However they are used here to convert an unsigned 64bit pattern
> > > > > > to a signed to a 32/64bit signed number.
> > > > > > To avoid any confusion use plain min()/max() and explicitely cast
> > > > > > the u64 expression to the correct signed value.
> > > > > >
> > > > > > Use a simple max() for the max_pkt_offset calulation and delete the
> > > > > > comment about why the cast to u32 is safe.
> > > > > >
> > > > > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > > > > ---
> > > > > >  kernel/bpf/verifier.c | 29 +++++++++++------------------
> > > > > >  1 file changed, 11 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > > > index ff40e5e65c43..22fa9769fbdb 100644
> > > > > > --- a/kernel/bpf/verifier.c
> > > > > > +++ b/kernel/bpf/verifier.c
> > > > > > @@ -2319,12 +2319,12 @@ static void __update_reg32_bounds(struct bpf_reg_state *reg)
> > > > > >         struct tnum var32_off = tnum_subreg(reg->var_off);
> > > > > >
> > > > > >         /* min signed is max(sign bit) | min(other bits) */
> > > > > > -       reg->s32_min_value = max_t(s32, reg->s32_min_value,
> > > > > > -                       var32_off.value | (var32_off.mask & S32_MIN));
> > > > > > +       reg->s32_min_value = max(reg->s32_min_value,
> > > > > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MIN)));
> > > > > >         /* max signed is min(sign bit) | max(other bits) */
> > > > > > -       reg->s32_max_value = min_t(s32, reg->s32_max_value,
> > > > > > -                       var32_off.value | (var32_off.mask & S32_MAX));
> > > > > > -       reg->u32_min_value = max_t(u32, reg->u32_min_value, (u32)var32_off.value);
> > > > > > +       reg->s32_max_value = min(reg->s32_max_value,
> > > > > > +                       (s32)(var32_off.value | (var32_off.mask & S32_MAX)));  
> > > > >
> > > > > Nack.
> > > > > This is plain ugly for no good reason.
> > > > > Leave the code as-is.  
> > > >
> > > > It is really horrid before.
> > > > From what i remember var32_off.value (and .mask) are both u64.
> > > > The pattern actually patches that used a few lines down the file.
> > > >
> > > > I've been trying to build allmodconfig with the size test added to min_t()
> > > > and max_t().
> > > > The number of real (or potentially real) bugs I've found is stunning.
> > > > The only fix is to nuke min_t() and max_t() to they can't be used.  
> > >
> > > No. min_t() is going to stay. It's not broken and
> > > this crusade against it is inappropriate.  
> >
> > I bet to differ...
> >  
> > > > The basic problem is the people have used the type of the target not that
> > > > of the largest parameter.
> > > > The might be ok for ulong v uint (on 64bit), but there are plenty of places
> > > > where u16 and u8 are used - a lot are pretty much buggy.
> > > >
> > > > Perhaps the worst ones I've found are with clamp_t(),
> > > > this is from 2/44:
> > > > -               (raw_inode)->xtime = cpu_to_le32(clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX));      \
> > > > +               (raw_inode)->xtime = cpu_to_le32(clamp((ts).tv_sec, S32_MIN, S32_MAX)); \
> > > > If also found clamp_t(u8, xxx, 0, 255).
> > > >
> > > > There are just so many broken examples.  
> > >
> > > clamp_t(u8, xxx, 0, 255) is not wrong. It's silly, but
> > > it's doing the right thing and one can argue and explicit
> > > clamp values serve as a documentation.  
> >
> > Not when you look at some of the code that uses it.
> > The clear intention is to saturate a large value - which isn't what it does.
> >  
> > > clamp_t(int32_t, (ts).tv_sec, S32_MIN, S32_MAX)) is indeed incorrect,
> > > but it's a bug in the implementation of __clamp_once().
> > > Fix it, instead of spamming people with "_t" removal.  
> >
> > It is too late by the time you get to clamp_once().
> > The 'type' for all the xxx_t() functions is an input cast, not the type
> > for the result.
> > clamp_t(type, v, lo, hi) has always been clamp((type)v, (type)lo, type(hi)).
> > From a code correctness point of view you pretty much never want those casts.  
> 
> Historical behavior doesn't justify a footgun.
> You definitely can make clampt_t() to behave like clamp_val() plus
> the final cast.

clamp_val() is actually the worst of the lot.

> Also note:
> git grep -w clamp_val|wc -l
> 818
> git grep -w clamp_t|wc -l
> 494
> 
> a safer macro is already used more often.

clamp_val() is worse than clamp_t() ...

Nope...
The problem is that clamp() requires all three parameters have the same type.
Coders are lazy and want to write clamp(variable, 1, 10).
This was fine if 'variable' had type 'int', but if it was 'unsigned int' you
had to write clamp(variable, 1u, 10u), worse if it is 'u8' you to either cast
both constants clamp(variable, (u8)1, (u8)10) or the variable
clamp((int)variable, 1, 10).
It the types/values aren't immediately obvious then any of those casts can
discard high bits.

A lot of the clamp_val() are actually for u8 structure members.
One thing to remember about C is it doesn't have any maths operators
for u8, the values are always promoted to 'int' before anything happens.
So if you write (foo->u8_member > 4 ? foo->u8_member : 4) the comparison
is done as an integer one.
Add some casts ((u8)f->m > (u8)x ? (u8)f->m : (u8)x) then the values are
all masked to 8 bits, promoted to 32 and then compared.
Even the ?: operator promotes its arguments and has a result type of 'int'.

Consider clamp_val(f->u8_m, LO, HI);
If HI is 255 it is fine, make HI 256 (perhaps it is sizeof() and something
got changed) and you suddenly have clamp(f->u8_m, LO, 0).
It is all just so fragile.

Maybe you are trying to find the 'chunk size' for a transfer of some kind.
If the transfer size is 'small' it might be in a 'u32', you want to limit it
to the size of the hardware's PCIe window - so do:
	copy_size = min(transfer_size, hardware_window_size);
But the hardware_window_size is a size_t (so 64bit).
The old min() would complain about the type mismatch, since the
hardware_window_size might actually be 4GB (that is true) casting to u32
is broken - you have to use the larger type.
But it might be the other way around, transfer_size is u64 and
hardware_window_size is u32, you still have to cast to u64 - but this
time it is the size of the other parameter.
The trouble is people have a habit of using the type they want for the
result, u32 in both the above and wrong twice.
But it was only the type check in min() that caused a problem.
Without the casts the compiler generates the right code, the only problem
is when a signed variable might contain a negative value that gets promoted
to a large negative value.
The current implementations of min/max/clamp only generate an error if they
can't prove that negative values won't be promoted to large unsigned values.

This is all fine provided the variable/expressions have the correct type
for the value they contain - and they usually do.
But for this bpf code the type of 'var32_off.value | (var32_off.mask & S32_MIN)'
is actually u64, it really does need an explicit cast to s32.

	David

> 
> > I've already fixed clamp() so it doesn't complain about comparing s64 against s32.
> > The next stage is to change pretty much all the xxx_t() to plain xxx().  
> 
> Nack to that. Fix the problem. Not the symptom.

[PATCH 34/44] bpf: use min() instead of min_t()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.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 the 'unsigned long' value is small enough that the result
is ok.

Detected by an extra check added to min_t().

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 kernel/bpf/core.c | 4 ++--
 kernel/bpf/log.c  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index d595fe512498..4f9808ea51fb 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1081,7 +1081,7 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
 	bpf_fill_ill_insns(hdr, size);
 
 	hdr->size = size;
-	hole = min_t(unsigned int, size - (proglen + sizeof(*hdr)),
+	hole = min(size - (proglen + sizeof(*hdr)),
 		     PAGE_SIZE - sizeof(*hdr));
 	start = get_random_u32_below(hole) & ~(alignment - 1);
 
@@ -1142,7 +1142,7 @@ bpf_jit_binary_pack_alloc(unsigned int proglen, u8 **image_ptr,
 	bpf_fill_ill_insns(*rw_header, size);
 	(*rw_header)->size = size;
 
-	hole = min_t(unsigned int, size - (proglen + sizeof(*ro_header)),
+	hole = min(size - (proglen + sizeof(*ro_header)),
 		     BPF_PROG_CHUNK_SIZE - sizeof(*ro_header));
 	start = get_random_u32_below(hole) & ~(alignment - 1);
 
diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index f50533169cc3..01dc13aaa785 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -79,7 +79,7 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
 		/* check if we have at least something to put into user buf */
 		new_n = 0;
 		if (log->end_pos < log->len_total) {
-			new_n = min_t(u32, log->len_total - log->end_pos, n);
+			new_n = min(log->len_total - log->end_pos, n);
 			log->kbuf[new_n - 1] = '\0';
 		}
 
-- 
2.39.5

[PATCH 35/44] bpf: use min() instead of min_t()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.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 the 'unsigned long' value is small enough that the result
is ok.

Detected by an extra check added to min_t().

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 kernel/trace/bpf_trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4f87c16d915a..ee3152df767c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1515,7 +1515,7 @@ BPF_CALL_4(bpf_read_branch_records, struct bpf_perf_event_data_kern *, ctx,
 	if (!buf || (size % br_entry_size != 0))
 		return -EINVAL;
 
-	to_copy = min_t(u32, br_stack->nr * br_entry_size, size);
+	to_copy = min(br_stack->nr * br_entry_size, size);
 	memcpy(buf, br_stack->entries, to_copy);
 
 	return to_copy;
-- 
2.39.5

[PATCH 41/44] net/core: Change loop conditions so min() can be used

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Loops like:
	int copied = ...;
	...
	while (copied) {
		use = min_t(type, copied, PAGE_SIZE - offset);
		...
		copied -= 0;
	}
can be converted to a plain min() if the comparison is changed to:
	while (copied > 0) {
This removes any chance of high bits being discded by min_t().
(In the case above PAGE_SIZE is 64bits so the 'int' cast is safe,
but there are plenty of cases where the check shows up bugs.)

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 net/core/datagram.c | 6 +++---
 net/core/skmsg.c    | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index c285c6465923..555f38b89729 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -664,8 +664,8 @@ int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
 		head = compound_head(pages[n]);
 		order = compound_order(head);
 
-		for (refs = 0; copied != 0; start = 0) {
-			int size = min_t(int, copied, PAGE_SIZE - start);
+		for (refs = 0; copied > 0; start = 0) {
+			int size = min(copied, PAGE_SIZE - start);
 
 			if (pages[n] - head > (1UL << order) - 1) {
 				head = compound_head(pages[n]);
@@ -783,7 +783,7 @@ EXPORT_SYMBOL(__zerocopy_sg_from_iter);
  */
 int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
 {
-	int copy = min_t(int, skb_headlen(skb), iov_iter_count(from));
+	int copy = min(skb_headlen(skb), iov_iter_count(from));
 
 	/* copy up to skb headlen */
 	if (skb_copy_datagram_from_iter(skb, 0, from, copy))
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 2ac7731e1e0a..b58e319f4e2e 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -335,8 +335,8 @@ int sk_msg_zerocopy_from_iter(struct sock *sk, struct iov_iter *from,
 		bytes -= copied;
 		msg->sg.size += copied;
 
-		while (copied) {
-			use = min_t(int, copied, PAGE_SIZE - offset);
+		while (copied > 0) {
+			use = min(copied, PAGE_SIZE - offset);
 			sg_set_page(&msg->sg.data[msg->sg.end],
 				    pages[i], use, offset);
 			sg_unmark_end(&msg->sg.data[msg->sg.end]);
-- 
2.39.5

Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits

[Jakub Kicinski]

On Wed, 19 Nov 2025 22:40:56 +0000 david.laight.linux@gmail.com wrote:
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.

Please split the networking (9?) patches out to a separate series.
It will help you with the CC list, and help us to get this applied..

Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits

[Lorenzo Stoakes]

On Wed, Nov 19, 2025 at 10:40:56PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> 	uint_var = min_t(uint, uint_var, uint64_expression);
>
> Casts to u8 and u16 are very likely to discard significant bits.
>
> These can be detected at compile time by changing min_t(), for example:
> #define CHECK_SIZE(fn, type, val) \
> 	BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
> 		!statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
> 		fn "() significant bits of '" #val "' may be discarded")
>
> #define min_t(type, x, y) ({ \
> 	CHECK_SIZE("min_t", type, x); \
> 	CHECK_SIZE("min_t", type, y); \
> 	__cmp_once(min, type, x, y); })
>
> (and similar changes to max_t() and clamp_t().)

Have we made sure that the introduction of these don't cause a combinatorial
explosion like previous min()/max() changes did?

>
> This shows up some real bugs, some unlikely bugs and some false positives.
> In most cases both arguments are unsigned type (just different ones)
> and min_t() can just be replaced by min().
>
> The patches are all independant and are most of the ones needed to
> get the x86-64 kernel I build to compile.
> I've not tried building an allyesconfig or allmodconfig kernel.

Well I have a beefy box at my disposal so tried thiese for you :)

Both allyesconfig & allmodconfig works fine for x86-64 (I tried both for good
measure)

> I've also not included the patch to minmax.h itself.
>
> I've tried to put the patches that actually fix things first.
> The last one is 0009.
>
> I gave up on fixing sched/fair.c - it is too broken for a single patch!
> The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
> needs multiple/larger changes to make it 'sane'.

I guess this isn't broken per se there just retain min_t()/max_t() right?

>
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.
>
> David Laight (44):
>   x86/asm/bitops: Change the return type of variable__ffs() to unsigned
>     int
>   ext4: Fix saturation of 64bit inode times for old filesystems
>   perf: Fix branch stack callchain limit
>   io_uring/net: Change some dubious min_t()
>   ipc/msg: Fix saturation of percpu counts in msgctl_info()
>   bpf: Verifier, remove some unusual uses of min_t() and max_t()
>   net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
>   net: ethtool: Use min3() instead of nested min_t(u16,...)
>   ipv6: __ip6_append_data() don't abuse max_t() casts
>   x86/crypto: ctr_crypt() use min() instead of min_t()
>   arch/x96/kvm: use min() instead of min_t()
>   block: use min() instead of min_t()
>   drivers/acpi: use min() instead of min_t()
>   drivers/char/hw_random: use min3() instead of nested min_t()
>   drivers/char/tpm: use min() instead of min_t()
>   drivers/crypto/ccp: use min() instead of min_t()
>   drivers/cxl: use min() instead of min_t()
>   drivers/gpio: use min() instead of min_t()
>   drivers/gpu/drm/amd: use min() instead of min_t()
>   drivers/i2c/busses: use min() instead of min_t()
>   drivers/net/ethernet/realtek: use min() instead of min_t()
>   drivers/nvme: use min() instead of min_t()
>   arch/x86/mm: use min() instead of min_t()
>   drivers/nvmem: use min() instead of min_t()
>   drivers/pci: use min() instead of min_t()
>   drivers/scsi: use min() instead of min_t()
>   drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
>     limits
>   drivers/usb/storage: use min() instead of min_t()
>   drivers/xen: use min() instead of min_t()
>   fs: use min() or umin() instead of min_t()
>   block: bvec.h: use min() instead of min_t()
>   nodemask: use min() instead of min_t()
>   ipc: use min() instead of min_t()
>   bpf: use min() instead of min_t()
>   bpf_trace: use min() instead of min_t()
>   lib/bucket_locks: use min() instead of min_t()
>   lib/crypto/mpi: use min() instead of min_t()
>   lib/dynamic_queue_limits: use max() instead of max_t()
>   mm: use min() instead of min_t()
>   net: Don't pass bitfields to max_t()
>   net/core: Change loop conditions so min() can be used
>   net: use min() instead of min_t()
>   net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
>   net/mptcp: Change some dubious min_t(int, ...) to min()
>
>  arch/x86/crypto/aesni-intel_glue.c            |  3 +-
>  arch/x86/include/asm/bitops.h                 | 18 +++++-------
>  arch/x86/kvm/emulate.c                        |  3 +-
>  arch/x86/kvm/lapic.c                          |  2 +-
>  arch/x86/kvm/mmu/mmu.c                        |  2 +-
>  arch/x86/mm/pat/set_memory.c                  | 12 ++++----
>  block/blk-iocost.c                            |  6 ++--
>  block/blk-settings.c                          |  2 +-
>  block/partitions/efi.c                        |  3 +-
>  drivers/acpi/property.c                       |  2 +-
>  drivers/char/hw_random/core.c                 |  2 +-
>  drivers/char/tpm/tpm1-cmd.c                   |  2 +-
>  drivers/char/tpm/tpm_tis_core.c               |  4 +--
>  drivers/crypto/ccp/ccp-dev.c                  |  2 +-
>  drivers/cxl/core/mbox.c                       |  2 +-
>  drivers/gpio/gpiolib-acpi-core.c              |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c  |  4 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  2 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
>  drivers/i2c/busses/i2c-designware-master.c    |  2 +-
>  drivers/net/ethernet/realtek/r8169_main.c     |  3 +-
>  drivers/nvme/host/pci.c                       |  3 +-
>  drivers/nvme/host/zns.c                       |  3 +-
>  drivers/nvmem/core.c                          |  2 +-
>  drivers/pci/probe.c                           |  3 +-
>  drivers/scsi/hosts.c                          |  2 +-
>  drivers/tty/vt/selection.c                    |  9 +++---
>  drivers/usb/storage/protocol.c                |  3 +-
>  drivers/xen/grant-table.c                     |  2 +-
>  fs/buffer.c                                   |  2 +-
>  fs/exec.c                                     |  2 +-
>  fs/ext4/ext4.h                                |  2 +-
>  fs/ext4/mballoc.c                             |  3 +-
>  fs/ext4/resize.c                              |  2 +-
>  fs/ext4/super.c                               |  2 +-
>  fs/fat/dir.c                                  |  4 +--
>  fs/fat/file.c                                 |  3 +-
>  fs/fuse/dev.c                                 |  2 +-
>  fs/fuse/file.c                                |  8 ++---
>  fs/splice.c                                   |  2 +-
>  include/linux/bvec.h                          |  3 +-
>  include/linux/nodemask.h                      |  9 +++---
>  include/linux/perf_event.h                    |  2 +-
>  include/net/tcp_ecn.h                         |  5 ++--
>  io_uring/net.c                                |  6 ++--
>  ipc/mqueue.c                                  |  4 +--
>  ipc/msg.c                                     |  6 ++--
>  kernel/bpf/core.c                             |  4 +--
>  kernel/bpf/log.c                              |  2 +-
>  kernel/bpf/verifier.c                         | 29 +++++++------------
>  kernel/trace/bpf_trace.c                      |  2 +-
>  lib/bucket_locks.c                            |  2 +-
>  lib/crypto/mpi/mpicoder.c                     |  2 +-
>  lib/dynamic_queue_limits.c                    |  2 +-
>  mm/gup.c                                      |  4 +--
>  mm/memblock.c                                 |  2 +-
>  mm/memory.c                                   |  2 +-
>  mm/percpu.c                                   |  2 +-
>  mm/truncate.c                                 |  3 +-
>  mm/vmscan.c                                   |  2 +-
>  net/core/datagram.c                           |  6 ++--
>  net/core/flow_dissector.c                     |  7 ++---
>  net/core/net-sysfs.c                          |  3 +-
>  net/core/skmsg.c                              |  4 +--
>  net/ethtool/cmis_cdb.c                        |  7 ++---
>  net/ipv4/fib_trie.c                           |  2 +-
>  net/ipv4/tcp_input.c                          |  4 +--
>  net/ipv4/tcp_output.c                         |  5 ++--
>  net/ipv4/tcp_timer.c                          |  4 +--
>  net/ipv6/addrconf.c                           |  8 ++---
>  net/ipv6/ip6_output.c                         |  7 +++--
>  net/ipv6/ndisc.c                              |  5 ++--
>  net/mptcp/protocol.c                          |  8 ++---
>  net/netlink/genetlink.c                       |  9 +++---
>  net/packet/af_packet.c                        |  2 +-
>  net/unix/af_unix.c                            |  4 +--
>  76 files changed, 141 insertions(+), 176 deletions(-)
>
> --
> 2.39.5
>

Re: (subset) [PATCH 00/44] Change a lot of min_t() that might mask high bits

[Jens Axboe]

On Wed, 19 Nov 2025 22:40:56 +0000, david.laight.linux@gmail.com wrote:
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> 	uint_var = min_t(uint, uint_var, uint64_expression);
> 
> Casts to u8 and u16 are very likely to discard significant bits.
> 
> [...]

Applied, thanks!

[12/44] block: use min() instead of min_t()
        commit: 9420e720ad192c53c8d2803c5a2313b2d586adbd

Best regards,
-- 
Jens Axboe

Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits

[Herbert Xu]

On Wed, Nov 19, 2025 at 10:40:56PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> 	uint_var = min_t(uint, uint_var, uint64_expression);
> 
> Casts to u8 and u16 are very likely to discard significant bits.
> 
> These can be detected at compile time by changing min_t(), for example:
> #define CHECK_SIZE(fn, type, val) \
> 	BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
> 		!statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
> 		fn "() significant bits of '" #val "' may be discarded")
> 
> #define min_t(type, x, y) ({ \
> 	CHECK_SIZE("min_t", type, x); \
> 	CHECK_SIZE("min_t", type, y); \
> 	__cmp_once(min, type, x, y); })
> 
> (and similar changes to max_t() and clamp_t().)
> 
> This shows up some real bugs, some unlikely bugs and some false positives.
> In most cases both arguments are unsigned type (just different ones)
> and min_t() can just be replaced by min().
> 
> The patches are all independant and are most of the ones needed to
> get the x86-64 kernel I build to compile.
> I've not tried building an allyesconfig or allmodconfig kernel.
> I've also not included the patch to minmax.h itself.
> 
> I've tried to put the patches that actually fix things first.
> The last one is 0009.
> 
> I gave up on fixing sched/fair.c - it is too broken for a single patch!
> The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
> needs multiple/larger changes to make it 'sane'.
> 
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.
> 
> David Laight (44):
>   x86/asm/bitops: Change the return type of variable__ffs() to unsigned
>     int
>   ext4: Fix saturation of 64bit inode times for old filesystems
>   perf: Fix branch stack callchain limit
>   io_uring/net: Change some dubious min_t()
>   ipc/msg: Fix saturation of percpu counts in msgctl_info()
>   bpf: Verifier, remove some unusual uses of min_t() and max_t()
>   net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
>   net: ethtool: Use min3() instead of nested min_t(u16,...)
>   ipv6: __ip6_append_data() don't abuse max_t() casts
>   x86/crypto: ctr_crypt() use min() instead of min_t()
>   arch/x96/kvm: use min() instead of min_t()
>   block: use min() instead of min_t()
>   drivers/acpi: use min() instead of min_t()
>   drivers/char/hw_random: use min3() instead of nested min_t()
>   drivers/char/tpm: use min() instead of min_t()
>   drivers/crypto/ccp: use min() instead of min_t()
>   drivers/cxl: use min() instead of min_t()
>   drivers/gpio: use min() instead of min_t()
>   drivers/gpu/drm/amd: use min() instead of min_t()
>   drivers/i2c/busses: use min() instead of min_t()
>   drivers/net/ethernet/realtek: use min() instead of min_t()
>   drivers/nvme: use min() instead of min_t()
>   arch/x86/mm: use min() instead of min_t()
>   drivers/nvmem: use min() instead of min_t()
>   drivers/pci: use min() instead of min_t()
>   drivers/scsi: use min() instead of min_t()
>   drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
>     limits
>   drivers/usb/storage: use min() instead of min_t()
>   drivers/xen: use min() instead of min_t()
>   fs: use min() or umin() instead of min_t()
>   block: bvec.h: use min() instead of min_t()
>   nodemask: use min() instead of min_t()
>   ipc: use min() instead of min_t()
>   bpf: use min() instead of min_t()
>   bpf_trace: use min() instead of min_t()
>   lib/bucket_locks: use min() instead of min_t()
>   lib/crypto/mpi: use min() instead of min_t()
>   lib/dynamic_queue_limits: use max() instead of max_t()
>   mm: use min() instead of min_t()
>   net: Don't pass bitfields to max_t()
>   net/core: Change loop conditions so min() can be used
>   net: use min() instead of min_t()
>   net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
>   net/mptcp: Change some dubious min_t(int, ...) to min()
> 
>  arch/x86/crypto/aesni-intel_glue.c            |  3 +-
>  arch/x86/include/asm/bitops.h                 | 18 +++++-------
>  arch/x86/kvm/emulate.c                        |  3 +-
>  arch/x86/kvm/lapic.c                          |  2 +-
>  arch/x86/kvm/mmu/mmu.c                        |  2 +-
>  arch/x86/mm/pat/set_memory.c                  | 12 ++++----
>  block/blk-iocost.c                            |  6 ++--
>  block/blk-settings.c                          |  2 +-
>  block/partitions/efi.c                        |  3 +-
>  drivers/acpi/property.c                       |  2 +-
>  drivers/char/hw_random/core.c                 |  2 +-
>  drivers/char/tpm/tpm1-cmd.c                   |  2 +-
>  drivers/char/tpm/tpm_tis_core.c               |  4 +--
>  drivers/crypto/ccp/ccp-dev.c                  |  2 +-
>  drivers/cxl/core/mbox.c                       |  2 +-
>  drivers/gpio/gpiolib-acpi-core.c              |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c  |  4 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  2 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
>  drivers/i2c/busses/i2c-designware-master.c    |  2 +-
>  drivers/net/ethernet/realtek/r8169_main.c     |  3 +-
>  drivers/nvme/host/pci.c                       |  3 +-
>  drivers/nvme/host/zns.c                       |  3 +-
>  drivers/nvmem/core.c                          |  2 +-
>  drivers/pci/probe.c                           |  3 +-
>  drivers/scsi/hosts.c                          |  2 +-
>  drivers/tty/vt/selection.c                    |  9 +++---
>  drivers/usb/storage/protocol.c                |  3 +-
>  drivers/xen/grant-table.c                     |  2 +-
>  fs/buffer.c                                   |  2 +-
>  fs/exec.c                                     |  2 +-
>  fs/ext4/ext4.h                                |  2 +-
>  fs/ext4/mballoc.c                             |  3 +-
>  fs/ext4/resize.c                              |  2 +-
>  fs/ext4/super.c                               |  2 +-
>  fs/fat/dir.c                                  |  4 +--
>  fs/fat/file.c                                 |  3 +-
>  fs/fuse/dev.c                                 |  2 +-
>  fs/fuse/file.c                                |  8 ++---
>  fs/splice.c                                   |  2 +-
>  include/linux/bvec.h                          |  3 +-
>  include/linux/nodemask.h                      |  9 +++---
>  include/linux/perf_event.h                    |  2 +-
>  include/net/tcp_ecn.h                         |  5 ++--
>  io_uring/net.c                                |  6 ++--
>  ipc/mqueue.c                                  |  4 +--
>  ipc/msg.c                                     |  6 ++--
>  kernel/bpf/core.c                             |  4 +--
>  kernel/bpf/log.c                              |  2 +-
>  kernel/bpf/verifier.c                         | 29 +++++++------------
>  kernel/trace/bpf_trace.c                      |  2 +-
>  lib/bucket_locks.c                            |  2 +-
>  lib/crypto/mpi/mpicoder.c                     |  2 +-
>  lib/dynamic_queue_limits.c                    |  2 +-
>  mm/gup.c                                      |  4 +--
>  mm/memblock.c                                 |  2 +-
>  mm/memory.c                                   |  2 +-
>  mm/percpu.c                                   |  2 +-
>  mm/truncate.c                                 |  3 +-
>  mm/vmscan.c                                   |  2 +-
>  net/core/datagram.c                           |  6 ++--
>  net/core/flow_dissector.c                     |  7 ++---
>  net/core/net-sysfs.c                          |  3 +-
>  net/core/skmsg.c                              |  4 +--
>  net/ethtool/cmis_cdb.c                        |  7 ++---
>  net/ipv4/fib_trie.c                           |  2 +-
>  net/ipv4/tcp_input.c                          |  4 +--
>  net/ipv4/tcp_output.c                         |  5 ++--
>  net/ipv4/tcp_timer.c                          |  4 +--
>  net/ipv6/addrconf.c                           |  8 ++---
>  net/ipv6/ip6_output.c                         |  7 +++--
>  net/ipv6/ndisc.c                              |  5 ++--
>  net/mptcp/protocol.c                          |  8 ++---
>  net/netlink/genetlink.c                       |  9 +++---
>  net/packet/af_packet.c                        |  2 +-
>  net/unix/af_unix.c                            |  4 +--
>  76 files changed, 141 insertions(+), 176 deletions(-)

Patches 10,14,16,37 applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt