[PATCH bpf-next v4 1/6] bpf: Correct the overflow check comment in bpf_iter_num_next()

[Puranjay Mohan <puranjay@kernel.org>]

bpf_iter_num_next() decides whether the iterator is exhausted with:

	if ((s64)(s->cur + 1) >= s->end) {

The comment above it claimed the (s64) cast was needed to be careful
about overflow, "e.g., if s->cur == s->end == INT_MAX, we can't just do
s->cur + 1 >= s->end". That reasoning is wrong: s->cur + 1 is evaluated
in int and wraps *before* the cast, so casting the already-wrapped result
to s64 changes nothing. For s->cur == s->end == INT_MAX the plain
s->cur + 1 >= s->end and the (s64) version both evaluate to false, and
more generally the two are identical for all inputs (sign-extending both
operands of a signed compare never changes its result).

The wraparound of s->cur + 1 is in fact intentional and load-bearing:
bpf_iter_num_new() initializes s->cur to start - 1, which wraps to
INT_MAX when start == INT_MIN, and the wrapping s->cur + 1 recovers
start on the first call. Using real 64-bit arithmetic ((s64)s->cur + 1)
would instead break iterators starting at INT_MIN.

Drop the redundant cast and rewrite the comment to describe what actually
happens. No functional change; the kernel builds with -fno-strict-overflow
so the signed wraparound is well defined.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 kernel/bpf/bpf_iter.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index f5eaeb2493d4a..f190f2b250048 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -802,12 +802,14 @@ __bpf_kfunc int *bpf_iter_num_next(struct bpf_iter_num* it)
 {
 	struct bpf_iter_num_kern *s = (void *)it;
 
-	/* check failed initialization or if we are done (same behavior);
-	 * need to be careful about overflow, so convert to s64 for checks,
-	 * e.g., if s->cur == s->end == INT_MAX, we can't just do
-	 * s->cur + 1 >= s->end
+	/* Detect the end of the range, or a failed/empty iterator: all of these
+	 * leave s->cur + 1 >= s->end. bpf_iter_num_new() set s->cur to start - 1
+	 * (which wraps to INT_MAX when start == INT_MIN), so the s->cur + 1 below
+	 * is a deliberate 32-bit wraparound that recovers start. As s->cur and
+	 * s->end are int, this is an ordinary signed 32-bit compare, exactly what
+	 * the inlined bpf_iter_num_next() emits.
 	 */
-	if ((s64)(s->cur + 1) >= s->end) {
+	if (s->cur + 1 >= s->end) {
 		s->cur = s->end = 0;
 		return NULL;
 	}
-- 
2.53.0-Meta