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

On Wed, Jul 29, 2026 at 1:36 PM Puranjay Mohan <[email protected]> wrote:
>
> 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 <[email protected]>
> ---
>  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.
>          */

this is really Claude-style verboseness. let's just say that num iter
maintains an invariant that either s->cur < s->end or s->cur == s->end
== 0, in both cases s->cur + 1 can't overflow int.


> -       if ((s64)(s->cur + 1) >= s->end) {
> +       if (s->cur + 1 >= s->end) {
>                 s->cur = s->end = 0;
>                 return NULL;
>         }
> --
> 2.53.0-Meta
>