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

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.

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

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.