Skip to content

Change a lot of min_t() that might mask high bits #6398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

Change a lot of min_t() that might mask high bits #6398

wants to merge 12 commits into from

Conversation

@kernel-patches-daemon-bpf-rc
Copy link

@kernel-patches-daemon-bpf-rc kernel-patches-daemon-bpf-rc bot commented Nov 19, 2025

Pull request for series with
subject: Change a lot of min_t() that might mask high bits
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1025536

David Laight added 12 commits November 19, 2025 14:49
bpf: Verifier, remove some unusual uses of min_t() and max_t()
ad1d01a 
min_t() and max_t() are normally used to change the signedness
of a positive value to avoid a signed-v-unsigned compare warning.

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

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

Signed-off-by: David Laight <[email protected]>
net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
9c03117 
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: d0c081b ("flow_dissector: properly cap thoff field")
Signed-off-by: David Laight <[email protected]>
net: ethtool: Use min3() instead of nested min_t(u16,...)
3a6cda0 
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 <[email protected]>
ipv6: __ip6_append_data() don't abuse max_t() casts
c8d3c7c 
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 <[email protected]>
drivers/net/ethernet/realtek: use min() instead of min_t()
03969f9 
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 <[email protected]>
bpf: use min() instead of min_t()
9141571 
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 <[email protected]>
bpf: use min() instead of min_t()
cbed533 
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 <[email protected]>
net: Don't pass bitfields to max_t()
54c5a7a 
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 <[email protected]>
net/core: Change loop conditions so min() can be used
4c10477 
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 <[email protected]>
net: use min() instead of min_t()
f57a33b 
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 <[email protected]>
net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
1ce101e 
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 <[email protected]>
net/mptcp: Change some dubious min_t(int, ...) to min()
3eaf408 
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 <[email protected]>
@kernel-patches-daemon-bpf-rc
Copy link
Author

kernel-patches-daemon-bpf-rc bot commented Nov 19, 2025

Upstream branch: d6ec090
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1025536
version: 1

@kernel-patches-daemon-bpf-rc
Copy link
Author

kernel-patches-daemon-bpf-rc bot commented Nov 20, 2025

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1025536 expired. Closing PR.

@kernel-patches-daemon-bpf-rc kernel-patches-daemon-bpf-rc bot deleted the series/1025536=>bpf-next branch November 22, 2025 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bpf-next changes-requested V1-ci-fail V1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant