Re: [PATCH 44/44] net/mptcp: Change some dubious min_t(int, ...) to min()
From: David Laight <hidden>
Date: 2025-12-18 22:00:00
Also in:
lkml, mptcp
On Thu, 18 Dec 2025 18:33:26 +0100 Matthieu Baerts [off-list ref] wrote:
Hi David, On 19/11/2025 23:41, david.laight.linux@gmail.com wrote:quoted
From: David Laight <redacted> 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