public inbox for mptcp@lists.linux.dev
 help / color / mirror / Atom feed
* [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 44/44] net/mptcp: Change some dubious min_t(int, ...) to min() david.laight.linux
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ 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] 9+ 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
@ 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)
  4 siblings, 1 reply; 9+ 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] 9+ 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
  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)
  4 siblings, 0 replies; 9+ 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] 9+ 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
  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 ` [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
  4 siblings, 0 replies; 9+ 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] 9+ 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
                   ` (2 preceding siblings ...)
  2025-11-20  9:38 ` Lorenzo Stoakes
@ 2025-11-20 14:52 ` Jens Axboe
  2025-11-24  9:49 ` Herbert Xu
  4 siblings, 0 replies; 9+ 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] 9+ 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
                   ` (3 preceding siblings ...)
  2025-11-20 14:52 ` (subset) " Jens Axboe
@ 2025-11-24  9:49 ` Herbert Xu
  4 siblings, 0 replies; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread

end of thread, other threads:[~2025-12-19 10:48 UTC | newest]

Thread overview: 9+ 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 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