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

On Tue, Aug 4, 2026 at 3:47=E2=80=AFPM <[email protected]> wrote:
>
> > diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> > index f5eaeb2493d4a..b235e117e206a 100644
> > --- a/kernel/bpf/bpf_iter.c
> > +++ b/kernel/bpf/bpf_iter.c
> > @@ -802,12 +802,11 @@ __bpf_kfunc int *bpf_iter_num_next(struct bpf_ite=
r_num* it)
> >  {
> >       struct bpf_iter_num_kern *s =3D (void *)it;
> >
> > -     /* check failed initialization or if we are done (same behavior);
> > -      * need to be careful about overflow, so convert to s64 for check=
s,
> > -      * e.g., if s->cur =3D=3D s->end =3D=3D INT_MAX, we can't just do
> > -      * s->cur + 1 >=3D s->end
> > +     /*
> > +      * s->cur < s->end while iterating, else s->cur =3D=3D s->end =3D=
=3D 0; the signed
> > +      * s->cur + 1 >=3D s->end holds even when s->cur + 1 wraps (start=
 =3D=3D INT_MIN).
>                                   ^^^^^
> >        */
> > -     if ((s64)(s->cur + 1) >=3D s->end) {
> > +     if (s->cur + 1 >=3D s->end) {
> >               s->cur =3D s->end =3D 0;
> >               return NULL;
> >       }
>
> The replacement comment says "s->cur + 1 >=3D s->end holds even when
> s->cur + 1 wraps (start =3D=3D INT_MIN)". Doesn't the inequality evaluate=
 to
> false in the wrap case, not true?
>
> Looking at the code: bpf_iter_num_new() sets s->cur =3D start - 1, so whe=
n
> start =3D=3D INT_MIN, s->cur wraps to INT_MAX. On the first call to
> bpf_iter_num_next(), s->cur + 1 wraps back to INT_MIN. For a non-empty
> range like the selftest bpf_for(i, INT_MIN, INT_MIN + 2) at
> tools/testing/selftests/bpf/progs/iters_num.c:101, the test becomes
> INT_MIN >=3D INT_MIN + 2, which is false, so the function returns the fir=
st
> element.
>
> The commit message describes this correctly ("the wrapping s->cur + 1
> brings it back to start"), but "X >=3D Y holds" reads as "the inequality =
is
> true," when what seems to be meant is that the comparison mechanism remai=
ns
> correct under wraparound.
>
> Also, the first clause says "s->cur < s->end while iterating, else
> s->cur =3D=3D s->end =3D=3D 0". This doesn't cover the post-bpf_iter_num_=
new(),
> pre-first-next() state for start =3D=3D INT_MIN, where s->cur =3D=3D INT_=
MAX and
> s->end =3D=3D INT_MIN + 2 (neither s->cur < s->end nor s->cur =3D=3D s->e=
nd =3D=3D 0).
>
> Since the sole purpose of this patch is to make the comment accurate, cou=
ld
> the wording be adjusted to avoid asserting a condition that is false in t=
he
> cited case?

Andrii do you have a preference for the comment here? I will copy
whatever you say verbatim to the next version.

Thanks,
Puranjay