* [PATCH 00/44] Change a lot of min_t() that might mask high bits
@ 2025-11-19 22:40 david.laight.linux
2025-11-19 22:41 ` [PATCH 07/44] net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value david.laight.linux
` (12 more replies)
0 siblings, 13 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:40 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Alexander Viro, Alexei Starovoitov, Andi Shyti,
Andreas Dilger, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Andy Shevchenko, Ard Biesheuvel, Arnaldo Carvalho de Melo,
Bjorn Helgaas, Borislav Petkov, Christian Brauner,
Christian König, Christoph Hellwig, Daniel Borkmann,
Dan Williams, Dave Hansen, Dave Jiang, David Ahern,
David Hildenbrand, Davidlohr Bueso, David S. Miller, Dennis Zhou,
Eric Dumazet, Greg Kroah-Hartman, Herbert Xu, Ingo Molnar,
Jakub Kicinski, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle), Mika Westerberg, Mike Rapoport,
Miklos Szeredi, Namhyung Kim, Neal Cardwell, nic_swsd,
OGAWA Hirofumi, Olivia Mackall, Paolo Abeni, Paolo Bonzini,
Peter Huewe, Peter Zijlstra, Rafael J. Wysocki,
Sean Christopherson, Srinivas Kandagatla, Stefano Stabellini,
Steven Rostedt, Tejun Heo, Theodore Ts'o, Thomas Gleixner,
Tom Lendacky, Willem de Bruijn, x86, Yury Norov, amd-gfx, bpf,
cgroups, dri-devel, io-uring, kvm, linux-acpi, linux-block,
linux-crypto, linux-cxl, linux-efi, linux-ext4, linux-fsdevel,
linux-gpio, linux-i2c, linux-integrity, linux-mm, linux-nvme,
linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage,
David Laight
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 07/44] net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 08/44] net: ethtool: Use min3() instead of nested min_t(u16,...) david.laight.linux
` (11 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Laight
From: David Laight <david.laight.linux@gmail.com>
There are some dodgy clamp_t(u16, ...) and min_t(u16, ...).
__skb_flow_dissect() tries to cap its return value with:
key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
however this casts skb->len to u16 before the comparison.
While both nboff and hlen are 'small', skb->len could be 0x10001 which
gets converted to 1 by the cast.
This gives an invalid (small) value for thoff for valid packets.
bpf_flow_dissect() used clamp_t(u16, ...) to set both flow_keys->nhoff
and flow_keys->thoff.
While I think these can't lose significant bits the casts are unnecessary
plain clamp(...) works fine.
Fixes: d0c081b49137c ("flow_dissector: properly cap thoff field")
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
net/core/flow_dissector.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 1b61bb25ba0e..e362160bb73d 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1023,9 +1023,8 @@ u32 bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
result = bpf_prog_run_pin_on_cpu(prog, ctx);
- flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);
- flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
- flow_keys->nhoff, hlen);
+ flow_keys->nhoff = clamp(flow_keys->nhoff, nhoff, hlen);
+ flow_keys->thoff = clamp(flow_keys->thoff, flow_keys->nhoff, hlen);
return result;
}
@@ -1687,7 +1686,7 @@ bool __skb_flow_dissect(const struct net *net,
ret = true;
out:
- key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
+ key_control->thoff = umin(nhoff, skb ? skb->len : hlen);
key_basic->n_proto = proto;
key_basic->ip_proto = ip_proto;
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 08/44] net: ethtool: Use min3() instead of nested min_t(u16,...)
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 07/44] net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts david.laight.linux
` (10 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, David Laight
From: David Laight <david.laight.linux@gmail.com>
In ethtool_cmis_cdb_execute_epl_cmd() change space_left and
bytes_to_write from u16 to u32.
Although the values may fit in 16 bits, 32bit variables will generate
better code.
Replace the nested min_t(u16, bytes_left, min_t(u16, space_left, x))
with a call to min3().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
net/ethtool/cmis_cdb.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/ethtool/cmis_cdb.c b/net/ethtool/cmis_cdb.c
index 3057576bc81e..1406205e047e 100644
--- a/net/ethtool/cmis_cdb.c
+++ b/net/ethtool/cmis_cdb.c
@@ -573,12 +573,11 @@ ethtool_cmis_cdb_execute_epl_cmd(struct net_device *dev,
while (offset <= CMIS_CDB_EPL_FW_BLOCK_OFFSET_END &&
bytes_written < epl_len) {
u32 bytes_left = epl_len - bytes_written;
- u16 space_left, bytes_to_write;
+ u32 space_left, bytes_to_write;
space_left = CMIS_CDB_EPL_FW_BLOCK_OFFSET_END - offset + 1;
- bytes_to_write = min_t(u16, bytes_left,
- min_t(u16, space_left,
- args->read_write_len_ext));
+ bytes_to_write = min3(bytes_left, space_left,
+ args->read_write_len_ext);
err = __ethtool_cmis_cdb_execute_cmd(dev, page_data,
page, offset,
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 07/44] net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value david.laight.linux
2025-11-19 22:41 ` [PATCH 08/44] net: ethtool: Use min3() instead of nested min_t(u16,...) david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-20 0:32 ` bot+bpf-ci
2025-11-19 22:41 ` [PATCH 21/44] drivers/net/ethernet/realtek: use min() instead of min_t() david.laight.linux
` (9 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: David Ahern, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, David Laight
From: David Laight <david.laight.linux@gmail.com>
The implicit casts done by max_t() should only really be used to
convert positive values to signed or unsigned types.
In the EMSGSIZE error path
pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
is being used to convert a large unsigned value to a signed negative one.
Rework using a signed temporary variable and max(pmtu, 0), as well as
casting sizeof() to (int) - which is where the unsignedness comes from.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
net/ipv6/ip6_output.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index f904739e99b9..6fecf2f2cc9a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
struct sk_buff *skb, *skb_prev = NULL;
struct inet_cork *cork = &cork_full->base;
struct flowi6 *fl6 = &cork_full->fl.u.ip6;
- unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
+ unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;
struct ubuf_info *uarg = NULL;
int exthdrlen = 0;
int dst_exthdrlen = 0;
@@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
maxnonfragsize = mtu;
if (cork->length + length > maxnonfragsize - headersize) {
+ int pmtu;
emsgsize:
- pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
- ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
+ pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
+ ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
return -EMSGSIZE;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 21/44] drivers/net/ethernet/realtek: use min() instead of min_t()
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (2 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 40/44] net: Don't pass bitfields to max_t() david.laight.linux
` (8 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit,
Jakub Kicinski, nic_swsd, Paolo Abeni, David Laight
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>
---
drivers/net/ethernet/realtek/r8169_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index d18734fe12e4..3e636983df4a 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4238,8 +4238,7 @@ static unsigned int rtl8125_quirk_udp_padto(struct rtl8169_private *tp,
}
if (trans_data_len < sizeof(struct udphdr))
- padto = max_t(unsigned int, padto,
- len + sizeof(struct udphdr) - trans_data_len);
+ padto = max(padto, len + sizeof(struct udphdr) - trans_data_len);
}
return padto;
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 40/44] net: Don't pass bitfields to max_t()
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (3 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 21/44] drivers/net/ethernet/realtek: use min() instead of min_t() david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 41/44] net/core: Change loop conditions so min() can be used david.laight.linux
` (7 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Laight
From: David Laight <david.laight.linux@gmail.com>
It is invalid to use sizeof() or typeof() in bitfields which stops
them being passed to max().
This has been fixed by using max_t().
I want to add some checks to max_t() to detect cases where the cast
discards non-zero high bits - which uses sizeof().
So add 0 to the bitfield (converting it to int) then use max().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
include/net/tcp_ecn.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/net/tcp_ecn.h b/include/net/tcp_ecn.h
index f13e5cd2b1ac..14c00404a95f 100644
--- a/include/net/tcp_ecn.h
+++ b/include/net/tcp_ecn.h
@@ -196,7 +196,7 @@ static inline void tcp_accecn_opt_demand_min(struct sock *sk,
struct tcp_sock *tp = tcp_sk(sk);
u8 opt_demand;
- opt_demand = max_t(u8, opt_demand_min, tp->accecn_opt_demand);
+ opt_demand = max(opt_demand_min, tp->accecn_opt_demand + 0);
tp->accecn_opt_demand = opt_demand;
}
@@ -303,8 +303,7 @@ static inline void tcp_ecn_received_counters(struct sock *sk,
u32 bytes_mask = GENMASK_U32(31, 22);
tp->received_ecn_bytes[ecnfield - 1] += len;
- tp->accecn_minlen = max_t(u8, tp->accecn_minlen,
- minlen);
+ tp->accecn_minlen = max(tp->accecn_minlen + 0, minlen);
/* Send AccECN option at least once per 2^22-byte
* increase in any ECN byte counter.
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 41/44] net/core: Change loop conditions so min() can be used
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (4 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 40/44] net: Don't pass bitfields to max_t() david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 42/44] net: use min() instead of min_t() david.laight.linux
` (6 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, bpf, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Jakub Sitnicki,
John Fastabend, Paolo Abeni, David Laight
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 42/44] net: use min() instead of min_t()
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (5 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 41/44] net/core: Change loop conditions so min() can be used david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 43/44] net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits david.laight.linux
` (5 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: David Ahern, David S. Miller, Eric Dumazet, Jakub Kicinski,
Kuniyuki Iwashima, Neal Cardwell, Paolo Abeni, Willem de Bruijn,
David Laight
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>
---
net/core/net-sysfs.c | 3 +--
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/ndisc.c | 5 ++---
net/packet/af_packet.c | 2 +-
net/unix/af_unix.c | 4 ++--
9 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index ca878525ad7c..8aaeed38be0b 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -985,8 +985,7 @@ static int netdev_rx_queue_set_rps_mask(struct netdev_rx_queue *queue,
struct rps_map *old_map, *map;
int cpu, i;
- map = kzalloc(max_t(unsigned int,
- RPS_MAP_SIZE(cpumask_weight(mask)), L1_CACHE_BYTES),
+ map = kzalloc(max(RPS_MAP_SIZE(cpumask_weight(mask)), L1_CACHE_BYTES),
GFP_KERNEL);
if (!map)
return -ENOMEM;
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 59a6f0a9638f..e85441717222 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -710,7 +710,7 @@ static unsigned char update_suffix(struct key_vector *tn)
* tn->pos + tn->bits, the second highest node will have a suffix
* length at most of tn->pos + tn->bits - 1
*/
- slen_max = min_t(unsigned char, tn->pos + tn->bits - 1, tn->slen);
+ slen_max = min(tn->pos + tn->bits - 1, tn->slen);
/* search though the list of children looking for nodes that might
* have a suffix greater than the one we currently have. This is
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e4a979b75cc6..8c9eb91190ae 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2870,7 +2870,7 @@ static void tcp_mtup_probe_success(struct sock *sk)
val = (u64)tcp_snd_cwnd(tp) * tcp_mss_to_mtu(sk, tp->mss_cache);
do_div(val, icsk->icsk_mtup.probe_size);
DEBUG_NET_WARN_ON_ONCE((u32)val != val);
- tcp_snd_cwnd_set(tp, max_t(u32, 1U, val));
+ tcp_snd_cwnd_set(tp, max(1, val));
tp->snd_cwnd_cnt = 0;
tp->snd_cwnd_stamp = tcp_jiffies32;
@@ -3323,7 +3323,7 @@ void tcp_rearm_rto(struct sock *sk)
/* delta_us may not be positive if the socket is locked
* when the retrans timer fires and is rescheduled.
*/
- rto = usecs_to_jiffies(max_t(int, delta_us, 1));
+ rto = usecs_to_jiffies(max(delta_us, 1));
}
tcp_reset_xmit_timer(sk, ICSK_TIME_RETRANS, rto, true);
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b94efb3050d2..516ea138993d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3076,7 +3076,7 @@ bool tcp_schedule_loss_probe(struct sock *sk, bool advancing_rto)
jiffies_to_usecs(inet_csk(sk)->icsk_rto) :
tcp_rto_delta_us(sk); /* How far in future is RTO? */
if (rto_delta_us > 0)
- timeout = min_t(u32, timeout, usecs_to_jiffies(rto_delta_us));
+ timeout = min(timeout, usecs_to_jiffies(rto_delta_us));
tcp_reset_xmit_timer(sk, ICSK_TIME_LOSS_PROBE, timeout, true);
return true;
@@ -4382,8 +4382,7 @@ void tcp_send_delayed_ack(struct sock *sk)
* directly.
*/
if (tp->srtt_us) {
- int rtt = max_t(int, usecs_to_jiffies(tp->srtt_us >> 3),
- TCP_DELACK_MIN);
+ int rtt = max(usecs_to_jiffies(tp->srtt_us >> 3), TCP_DELACK_MIN);
if (rtt < max_ato)
max_ato = rtt;
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 2dd73a4e8e51..9d5fc405e76a 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -43,7 +43,7 @@ static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
if (remaining <= 0)
return 1; /* user timeout has passed; fire ASAP */
- return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(remaining));
+ return min(icsk->icsk_rto, msecs_to_jiffies(remaining));
}
u32 tcp_clamp_probe0_to_user_timeout(const struct sock *sk, u32 when)
@@ -504,7 +504,7 @@ static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
*/
if (rtx_delta > user_timeout)
return true;
- timeout = min_t(u32, timeout, msecs_to_jiffies(user_timeout));
+ timeout = umin(timeout, msecs_to_jiffies(user_timeout));
}
/* Note: timer interrupt might have been delayed by at least one jiffy,
* and tp->rcv_tstamp might very well have been written recently.
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 40e9c336f6c5..930e34af4331 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1422,11 +1422,11 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
if_public_preferred_lft = ifp->prefered_lft;
memset(&cfg, 0, sizeof(cfg));
- cfg.valid_lft = min_t(__u32, ifp->valid_lft,
- READ_ONCE(idev->cnf.temp_valid_lft) + age);
+ cfg.valid_lft = min(ifp->valid_lft,
+ READ_ONCE(idev->cnf.temp_valid_lft) + age);
cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor;
- cfg.preferred_lft = min_t(__u32, if_public_preferred_lft, cfg.preferred_lft);
- cfg.preferred_lft = min_t(__u32, cfg.valid_lft, cfg.preferred_lft);
+ cfg.preferred_lft = min(if_public_preferred_lft, cfg.preferred_lft);
+ cfg.preferred_lft = min(cfg.valid_lft, cfg.preferred_lft);
cfg.plen = ifp->prefix_len;
tmp_tstamp = ifp->tstamp;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f427e41e9c49..b3bcbf0d864b 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1731,9 +1731,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
neigh_release(neigh);
}
- rd_len = min_t(unsigned int,
- IPV6_MIN_MTU - sizeof(struct ipv6hdr) - sizeof(*msg) - optlen,
- skb->len + 8);
+ rd_len = min(IPV6_MIN_MTU - sizeof(struct ipv6hdr) - sizeof(*msg) - optlen,
+ skb->len + 8);
rd_len &= ~0x7;
optlen += rd_len;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 173e6edda08f..af0c74f7b4d4 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -3015,7 +3015,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
hlen = LL_RESERVED_SPACE(dev);
tlen = dev->needed_tailroom;
linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
- linear = max(linear, min_t(int, len, dev->hard_header_len));
+ linear = max(linear, min(len, dev->hard_header_len));
skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, linear,
msg->msg_flags & MSG_DONTWAIT, &err);
if (skb == NULL)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 768098dec231..e573fcb21a01 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2448,7 +2448,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
/* allow fallback to order-0 allocations */
size = min_t(int, size, SKB_MAX_HEAD(0) + UNIX_SKB_FRAGS_SZ);
- data_len = max_t(int, 0, size - SKB_MAX_HEAD(0));
+ data_len = max(0, size - (int)SKB_MAX_HEAD(0));
data_len = min_t(size_t, size, PAGE_ALIGN(data_len));
@@ -3054,7 +3054,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
sunaddr = NULL;
}
- chunk = min_t(unsigned int, unix_skb_len(skb) - skip, size);
+ chunk = min(unix_skb_len(skb) - skip, size);
chunk = state->recv_actor(skb, skip, chunk, state);
if (chunk < 0) {
if (copied == 0)
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 43/44] net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (6 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 42/44] net: use min() instead of min_t() david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-19 22:41 ` [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min() david.laight.linux
` (4 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
David Laight
From: David Laight <david.laight.linux@gmail.com>
The scan limit in genl_allocate_reserve_groups() is:
min_t(int, id + n_groups, mc_groups_longs * BITS_PER_LONG);
While 'id' and 'n_groups' are both 'int', 'mc_groups_longs' is
'unsigned long' (BITS_PER_LONG is 'int').
These inconsistent types (all the values are small and non-negative)
means that a simple min() fails.
When checks for masking high bits are added to min_t() that also fails.
Instead use umin() so safely convert all the values to unsigned.
Move the limit calculation outside the loop for efficiency and
readability.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
net/netlink/genetlink.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 978c129c6095..a802dd8ead2d 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -395,10 +395,11 @@ static unsigned int genl_op_iter_idx(struct genl_op_iter *iter)
return iter->cmd_idx;
}
-static int genl_allocate_reserve_groups(int n_groups, int *first_id)
+static noinline_for_stack int genl_allocate_reserve_groups(int n_groups, int *first_id)
{
unsigned long *new_groups;
int start = 0;
+ int limit;
int i;
int id;
bool fits;
@@ -414,10 +415,8 @@ static int genl_allocate_reserve_groups(int n_groups, int *first_id)
start);
fits = true;
- for (i = id;
- i < min_t(int, id + n_groups,
- mc_groups_longs * BITS_PER_LONG);
- i++) {
+ limit = umin(id + n_groups, mc_groups_longs * BITS_PER_LONG);
+ for (i = id; i < limit; i++) {
if (test_bit(i, mc_groups)) {
start = i;
fits = false;
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min()
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (7 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 43/44] net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-12-18 17:33 ` Matthieu Baerts
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
` (3 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, mptcp, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Mat Martineau,
Matthieu Baerts, Paolo Abeni, David Laight
From: David Laight <david.laight.linux@gmail.com>
There are two:
min_t(int, xxx, mptcp_wnd_end(msk) - msk->snd_nxt);
Both mptcp_wnd_end(msk) and msk->snd_nxt are u64, their difference
(aka the window size) might be limited to 32 bits - but that isn't
knowable from this code.
So checks being added to min_t() detect the potential discard of
significant bits.
Provided the 'avail_size' and return of mptcp_check_allowed_size()
are changed to an unsigned type (size_t matches the type the caller
uses) both min_t() can be changed to min().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
net/mptcp/protocol.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 2d6b8de35c44..d57c3659462f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1123,8 +1123,8 @@ struct mptcp_sendmsg_info {
bool data_lock_held;
};
-static int mptcp_check_allowed_size(const struct mptcp_sock *msk, struct sock *ssk,
- u64 data_seq, int avail_size)
+static size_t mptcp_check_allowed_size(const struct mptcp_sock *msk, struct sock *ssk,
+ u64 data_seq, size_t avail_size)
{
u64 window_end = mptcp_wnd_end(msk);
u64 mptcp_snd_wnd;
@@ -1133,7 +1133,7 @@ static int mptcp_check_allowed_size(const struct mptcp_sock *msk, struct sock *s
return avail_size;
mptcp_snd_wnd = window_end - data_seq;
- avail_size = min_t(unsigned int, mptcp_snd_wnd, avail_size);
+ avail_size = min(mptcp_snd_wnd, avail_size);
if (unlikely(tcp_sk(ssk)->snd_wnd < mptcp_snd_wnd)) {
tcp_sk(ssk)->snd_wnd = min_t(u64, U32_MAX, mptcp_snd_wnd);
@@ -1477,7 +1477,7 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
if (!ssk || !sk_stream_memory_free(ssk))
return NULL;
- burst = min_t(int, MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt);
+ burst = min(MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt);
wmem = READ_ONCE(ssk->sk_wmem_queued);
if (!burst)
return ssk;
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
2025-11-19 22:41 ` [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts david.laight.linux
@ 2025-11-20 0:32 ` bot+bpf-ci
2025-11-20 11:16 ` David Laight
0 siblings, 1 reply; 20+ messages in thread
From: bot+bpf-ci @ 2025-11-20 0:32 UTC (permalink / raw)
To: david.laight.linux, linux-kernel, netdev
Cc: dsahern, davem, edumazet, kuba, pabeni, david.laight.linux, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index f904739e9..6fecf2f2c 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
> struct sk_buff *skb, *skb_prev = NULL;
> struct inet_cork *cork = &cork_full->base;
> struct flowi6 *fl6 = &cork_full->fl.u.ip6;
> - unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
> + unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;
[ ... ]
> @@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
> maxnonfragsize = mtu;
>
> if (cork->length + length > maxnonfragsize - headersize) {
> + int pmtu;
> emsgsize:
> - pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
> - ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
> + pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
> + ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
> return -EMSGSIZE;
> }
Can the variable pmtu be used uninitialized here?
When reaching the emsgsize label via goto statements from earlier in
__ip6_append_data() (the checks at lines around mtu <= fragheaderlen,
headersize + transhdrlen > mtu, and the dontfrag check), the int pmtu
declaration is skipped. This leaves pmtu uninitialized when it's used
in the calculation that's passed to ipv6_local_error().
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19519124312
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (8 preceding siblings ...)
2025-11-19 22:41 ` [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min() david.laight.linux
@ 2025-11-20 1:47 ` Jakub Kicinski
2025-11-20 9:38 ` Lorenzo Stoakes
` (2 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2025-11-20 1:47 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Herbert Xu,
Ingo Molnar, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle), Mika Westerberg, Mike Rapoport,
Miklos Szeredi, Namhyung Kim, Neal Cardwell, nic_swsd,
OGAWA Hirofumi, Olivia Mackall, Paolo Abeni, Paolo Bonzini,
Peter Huewe, Peter Zijlstra, Rafael J. Wysocki,
Sean Christopherson, Srinivas Kandagatla, Stefano Stabellini,
Steven Rostedt, Tejun Heo, Theodore Ts'o, Thomas Gleixner,
Tom Lendacky, Willem de Bruijn, x86, Yury Norov, amd-gfx, bpf,
cgroups, dri-devel, io-uring, kvm, linux-acpi, linux-block,
linux-crypto, linux-cxl, linux-efi, linux-ext4, linux-fsdevel,
linux-gpio, linux-i2c, linux-integrity, linux-mm, linux-nvme,
linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage
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..
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (9 preceding siblings ...)
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
@ 2025-11-20 9:38 ` Lorenzo Stoakes
2025-11-20 14:52 ` (subset) " Jens Axboe
2025-11-24 9:49 ` Herbert Xu
12 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-11-20 9:38 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Herbert Xu,
Ingo Molnar, Jakub Kicinski, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle), Mika Westerberg, Mike Rapoport,
Miklos Szeredi, Namhyung Kim, Neal Cardwell, nic_swsd,
OGAWA Hirofumi, Olivia Mackall, Paolo Abeni, Paolo Bonzini,
Peter Huewe, Peter Zijlstra, Rafael J. Wysocki,
Sean Christopherson, Srinivas Kandagatla, Stefano Stabellini,
Steven Rostedt, Tejun Heo, Theodore Ts'o, Thomas Gleixner,
Tom Lendacky, Willem de Bruijn, x86, Yury Norov, amd-gfx, bpf,
cgroups, dri-devel, io-uring, kvm, linux-acpi, linux-block,
linux-crypto, linux-cxl, linux-efi, linux-ext4, linux-fsdevel,
linux-gpio, linux-i2c, linux-integrity, linux-mm, linux-nvme,
linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage
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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
2025-11-20 0:32 ` bot+bpf-ci
@ 2025-11-20 11:16 ` David Laight
2025-11-20 13:50 ` Chris Mason
0 siblings, 1 reply; 20+ messages in thread
From: David Laight @ 2025-11-20 11:16 UTC (permalink / raw)
To: bot+bpf-ci
Cc: linux-kernel, netdev, dsahern, davem, edumazet, kuba, pabeni, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
On Thu, 20 Nov 2025 00:32:34 +0000 (UTC)
bot+bpf-ci@kernel.org wrote:
> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index f904739e9..6fecf2f2c 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
> > struct sk_buff *skb, *skb_prev = NULL;
> > struct inet_cork *cork = &cork_full->base;
> > struct flowi6 *fl6 = &cork_full->fl.u.ip6;
> > - unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
> > + unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;
>
> [ ... ]
>
> > @@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
> > maxnonfragsize = mtu;
> >
> > if (cork->length + length > maxnonfragsize - headersize) {
> > + int pmtu;
> > emsgsize:
> > - pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
> > - ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
> > + pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
> > + ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
> > return -EMSGSIZE;
> > }
>
> Can the variable pmtu be used uninitialized here?
If your AI bot doesn't understand code don't run it.
David
>
> When reaching the emsgsize label via goto statements from earlier in
> __ip6_append_data() (the checks at lines around mtu <= fragheaderlen,
> headersize + transhdrlen > mtu, and the dontfrag check), the int pmtu
> declaration is skipped. This leaves pmtu uninitialized when it's used
> in the calculation that's passed to ipv6_local_error().
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19519124312
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
2025-11-20 11:16 ` David Laight
@ 2025-11-20 13:50 ` Chris Mason
0 siblings, 0 replies; 20+ messages in thread
From: Chris Mason @ 2025-11-20 13:50 UTC (permalink / raw)
To: David Laight, bot+bpf-ci
Cc: linux-kernel, netdev, dsahern, davem, edumazet, kuba, pabeni, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, ihor.solodrai
On 11/20/25 6:16 AM, David Laight wrote:
> On Thu, 20 Nov 2025 00:32:34 +0000 (UTC)
> bot+bpf-ci@kernel.org wrote:
>
>>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>>> index f904739e9..6fecf2f2c 100644
>>> --- a/net/ipv6/ip6_output.c
>>> +++ b/net/ipv6/ip6_output.c
>>> @@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
>>> struct sk_buff *skb, *skb_prev = NULL;
>>> struct inet_cork *cork = &cork_full->base;
>>> struct flowi6 *fl6 = &cork_full->fl.u.ip6;
>>> - unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
>>> + unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;
>>
>> [ ... ]
>>
>>> @@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
>>> maxnonfragsize = mtu;
>>>
>>> if (cork->length + length > maxnonfragsize - headersize) {
>>> + int pmtu;
>>> emsgsize:
>>> - pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
>>> - ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
>>> + pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
>>> + ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
>>> return -EMSGSIZE;
>>> }
>>
>> Can the variable pmtu be used uninitialized here?
>
> If your AI bot doesn't understand code don't run it.
I ran this locally three more times and this false positive didn't
reproduce, but I'll see if the CI has enough logs to figure out where it
got confused.
Regardless, I'm doing periodic checks for patterns of false positives
and fine tuning the prompts.
-chris
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (subset) [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (10 preceding siblings ...)
2025-11-20 9:38 ` Lorenzo Stoakes
@ 2025-11-20 14:52 ` Jens Axboe
2025-11-24 9:49 ` Herbert Xu
12 siblings, 0 replies; 20+ messages in thread
From: Jens Axboe @ 2025-11-20 14:52 UTC (permalink / raw)
To: linux-kernel, david.laight.linux
Cc: Alan Stern, Alexander Viro, Alexei Starovoitov, Andi Shyti,
Andreas Dilger, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Andy Shevchenko, Ard Biesheuvel, Arnaldo Carvalho de Melo,
Bjorn Helgaas, Borislav Petkov, Christian Brauner,
Christian König, Christoph Hellwig, Daniel Borkmann,
Dan Williams, Dave Hansen, Dave Jiang, David Ahern,
Davidlohr Bueso, David S. Miller, Dennis Zhou, Eric Dumazet,
Greg Kroah-Hartman, Herbert Xu, Ingo Molnar, Jakub Kicinski,
Jakub Sitnicki, James E.J. Bottomley, Jarkko Sakkinen,
Jason A. Donenfeld, Jiri Slaby, Johannes Weiner, John Allen,
Jonathan Cameron, Juergen Gross, Kees Cook, KP Singh,
Linus Walleij, Martin K. Petersen, Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage,
David Hildenbrand
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (11 preceding siblings ...)
2025-11-20 14:52 ` (subset) " Jens Axboe
@ 2025-11-24 9:49 ` Herbert Xu
12 siblings, 0 replies; 20+ messages in thread
From: Herbert Xu @ 2025-11-24 9:49 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Ingo Molnar,
Jakub Kicinski, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle), Mika Westerberg, Mike Rapoport,
Miklos Szeredi, Namhyung Kim, Neal Cardwell, nic_swsd,
OGAWA Hirofumi, Olivia Mackall, Paolo Abeni, Paolo Bonzini,
Peter Huewe, Peter Zijlstra, Rafael J. Wysocki,
Sean Christopherson, Srinivas Kandagatla, Stefano Stabellini,
Steven Rostedt, Tejun Heo, Theodore Ts'o, Thomas Gleixner,
Tom Lendacky, Willem de Bruijn, x86, Yury Norov, amd-gfx, bpf,
cgroups, dri-devel, io-uring, kvm, linux-acpi, linux-block,
linux-crypto, linux-cxl, linux-efi, linux-ext4, linux-fsdevel,
linux-gpio, linux-i2c, linux-integrity, linux-mm, linux-nvme,
linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min()
2025-11-19 22:41 ` [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min() david.laight.linux
@ 2025-12-18 17:33 ` Matthieu Baerts
2025-12-18 20:15 ` David Laight
0 siblings, 1 reply; 20+ messages in thread
From: Matthieu Baerts @ 2025-12-18 17:33 UTC (permalink / raw)
To: david.laight.linux, linux-kernel, mptcp, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Mat Martineau,
Paolo Abeni
Hi David,
On 19/11/2025 23:41, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> There are two:
> min_t(int, xxx, mptcp_wnd_end(msk) - msk->snd_nxt);
> Both mptcp_wnd_end(msk) and msk->snd_nxt are u64, their difference
> (aka the window size) might be limited to 32 bits - but that isn't
> knowable from this code.
> So checks being added to min_t() detect the potential discard of
> significant bits.
>
> Provided the 'avail_size' and return of mptcp_check_allowed_size()
> are changed to an unsigned type (size_t matches the type the caller
> uses) both min_t() can be changed to min().
Thank you for the patch!
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
I'm not sure what the status on your side: I don't know if you still
plan to send a specific series for all the modifications in the net, but
just in case, I just applied your patch in the MPTCP tree. I removed the
"net/" prefix from the subject. I will send this patch with others for
including in the net-next tree later on if you didn't do that in between.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min()
2025-12-18 17:33 ` Matthieu Baerts
@ 2025-12-18 20:15 ` David Laight
2025-12-19 10:48 ` Matthieu Baerts
0 siblings, 1 reply; 20+ messages in thread
From: David Laight @ 2025-12-18 20:15 UTC (permalink / raw)
To: Matthieu Baerts
Cc: linux-kernel, mptcp, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Mat Martineau, Paolo Abeni
On Thu, 18 Dec 2025 18:33:26 +0100
Matthieu Baerts <matttbe@kernel.org> wrote:
> Hi David,
>
> On 19/11/2025 23:41, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > There are two:
> > min_t(int, xxx, mptcp_wnd_end(msk) - msk->snd_nxt);
> > Both mptcp_wnd_end(msk) and msk->snd_nxt are u64, their difference
> > (aka the window size) might be limited to 32 bits - but that isn't
> > knowable from this code.
> > So checks being added to min_t() detect the potential discard of
> > significant bits.
> >
> > Provided the 'avail_size' and return of mptcp_check_allowed_size()
> > are changed to an unsigned type (size_t matches the type the caller
> > uses) both min_t() can be changed to min().
>
> Thank you for the patch!
>
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
> I'm not sure what the status on your side: I don't know if you still
> plan to send a specific series for all the modifications in the net, but
> just in case, I just applied your patch in the MPTCP tree. I removed the
> "net/" prefix from the subject. I will send this patch with others for
> including in the net-next tree later on if you didn't do that in between.
I'll go through them again at some point.
I'll check against 'next' (but probably not net-next).
I actually need to look at the ones that seemed like real bugs when I
did an allmodconfig build - that got to over 200 patches to get 'clean'.
It would be nice to get rid of a lot of the min_t(), but I might try
to attack the dubious ones rather than the ones that appear to make
no difference.
I might propose some extra checks in minmax.h that would break W=1 builds.
Detecting things like min_t(u8, u32_value, 0xff) where the cast makes the
comparison always succeed.
In reality any calls with casts to u8 and u16 are 'dubious'.
That and changing checkpatch.pl to not suggest min_t() at all, and
to reject the more dubious uses.
After all with:
min(x, (int)y)
it is clear to the reader that 'y' is being possibly truncated and converted
to a signed value, but with:
min_t(int, x, y)
you don't know which value needed the cast (and the line isn't even shorter).
But what I've found all to often is actually:
a = min_t(typeof(a), x, y);
and the similar:
x = min_t(typeof(x), x, y);
where the type of the result is used and high bits get discarded.
I've just been trying to build with #define clamp_val clamp.
That requires a few minor changes and I'm pretty sure shows up
a real bug.
David
>
> Cheers,
> Matt
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min()
2025-12-18 20:15 ` David Laight
@ 2025-12-19 10:48 ` Matthieu Baerts
0 siblings, 0 replies; 20+ messages in thread
From: Matthieu Baerts @ 2025-12-19 10:48 UTC (permalink / raw)
To: David Laight
Cc: linux-kernel, mptcp, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Mat Martineau, Paolo Abeni
Hi David,
Thank you for your reply!
On 18/12/2025 21:15, David Laight wrote:
> On Thu, 18 Dec 2025 18:33:26 +0100
> Matthieu Baerts <matttbe@kernel.org> wrote:
>
>> Hi David,
>>
>> On 19/11/2025 23:41, david.laight.linux@gmail.com wrote:
>>> From: David Laight <david.laight.linux@gmail.com>
>>>
>>> There are two:
>>> min_t(int, xxx, mptcp_wnd_end(msk) - msk->snd_nxt);
>>> Both mptcp_wnd_end(msk) and msk->snd_nxt are u64, their difference
>>> (aka the window size) might be limited to 32 bits - but that isn't
>>> knowable from this code.
>>> So checks being added to min_t() detect the potential discard of
>>> significant bits.
>>>
>>> Provided the 'avail_size' and return of mptcp_check_allowed_size()
>>> are changed to an unsigned type (size_t matches the type the caller
>>> uses) both min_t() can be changed to min().
>>
>> Thank you for the patch!
>>
>> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>
>> I'm not sure what the status on your side: I don't know if you still
>> plan to send a specific series for all the modifications in the net, but
>> just in case, I just applied your patch in the MPTCP tree. I removed the
>> "net/" prefix from the subject. I will send this patch with others for
>> including in the net-next tree later on if you didn't do that in between.
>
> I'll go through them again at some point.
Great, thank you!
> I'll check against 'next' (but probably not net-next).
net-next is in linux-next, so that should be fine.
> I actually need to look at the ones that seemed like real bugs when I
> did an allmodconfig build - that got to over 200 patches to get 'clean'.
>
> It would be nice to get rid of a lot of the min_t(), but I might try
> to attack the dubious ones rather than the ones that appear to make
> no difference.
>
> I might propose some extra checks in minmax.h that would break W=1 builds.
> Detecting things like min_t(u8, u32_value, 0xff) where the cast makes the
> comparison always succeed.
> In reality any calls with casts to u8 and u16 are 'dubious'.
>
> That and changing checkpatch.pl to not suggest min_t() at all, and
> to reject the more dubious uses.
> After all with:
> min(x, (int)y)
> it is clear to the reader that 'y' is being possibly truncated and converted
> to a signed value, but with:
> min_t(int, x, y)
> you don't know which value needed the cast (and the line isn't even shorter).
> But what I've found all to often is actually:
> a = min_t(typeof(a), x, y);
> and the similar:
> x = min_t(typeof(x), x, y);
> where the type of the result is used and high bits get discarded.
Good idea to add extra checks and prevent future issues!
> I've just been trying to build with #define clamp_val clamp.
> That requires a few minor changes and I'm pretty sure shows up
> a real bug.
Thank you for looking at that!
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-12-19 10:48 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 07/44] net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value david.laight.linux
2025-11-19 22:41 ` [PATCH 08/44] net: ethtool: Use min3() instead of nested min_t(u16,...) david.laight.linux
2025-11-19 22:41 ` [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts david.laight.linux
2025-11-20 0:32 ` bot+bpf-ci
2025-11-20 11:16 ` David Laight
2025-11-20 13:50 ` Chris Mason
2025-11-19 22:41 ` [PATCH 21/44] drivers/net/ethernet/realtek: use min() instead of min_t() david.laight.linux
2025-11-19 22:41 ` [PATCH 40/44] net: Don't pass bitfields to max_t() david.laight.linux
2025-11-19 22:41 ` [PATCH 41/44] net/core: Change loop conditions so min() can be used david.laight.linux
2025-11-19 22:41 ` [PATCH 42/44] net: use min() instead of min_t() david.laight.linux
2025-11-19 22:41 ` [PATCH 43/44] net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min() david.laight.linux
2025-12-18 17:33 ` Matthieu Baerts
2025-12-18 20:15 ` David Laight
2025-12-19 10:48 ` Matthieu Baerts
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
2025-11-20 9:38 ` Lorenzo Stoakes
2025-11-20 14:52 ` (subset) " Jens Axboe
2025-11-24 9:49 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).