Re: [PATCH bpf-next v2 1/2] bpf: fix arg tracking for imprecise/multi-offset BPF_ST/STX

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2026-04-13 at 15:42 -0700, Alexei Starovoitov wrote:
> On Mon, Apr 13, 2026 at 2:58 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > BPF_STX through ARG_IMPRECISE dst should be recognized as a local
> > spill and join at_stack with the written value. For example,
> > consider the following situation:
> > 
> >    // r1 = ARG_IMPRECISE{mask=BIT(0)|BIT(1)}
> >    *(u64 *)(r1 + 0) = r8
> > 
> > Here the analysis should produce an equivalent of
> > 
> >   at_stack[*] = join(old, r8)
> > 
> > BPF_ST through multi-offset or imprecise dst should join at_stack with
> > none instead of overwriting the slots. For example, consider the
> > following situation:
> > 
> >    // r1 = ARG_IMPRECISE{mask=BIT(0)|BIT(1)}
> >    *(u64 *)(r1 + 0) = 0
> > 
> > Here the analysis should produce an equivalent of
> > 
> >   at_stack[*r1] = join(old, none).
> > 
> > Move the definition of the clear_overlapping_stack_slots() in order to
> > have __arg_track_join() visible. Remove the OFF_IMPRECISE constant to
> > avoid having two ways to express imprecise offset. Only
> > 'offset-imprecise {frame=N, cnt=0}' remains.
> > 
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> >  kernel/bpf/liveness.c | 110 ++++++++++++++++++++++++++++----------------------
> >  1 file changed, 61 insertions(+), 49 deletions(-)
> > 
> > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> > index 1fb4c511db5a..23d19147f123 100644
> > --- a/kernel/bpf/liveness.c
> > +++ b/kernel/bpf/liveness.c
> > @@ -574,7 +574,7 @@ static int print_instances(struct bpf_verifier_env *env)
> >   *
> >   *   precise {frame=N, off=V}      -- known absolute frame index and byte offset
> >   *        |
> > - *   offset-imprecise {frame=N, off=OFF_IMPRECISE}
> > + *   offset-imprecise {frame=N, cnt=0}
> >   *        |                        -- known frame identity, unknown offset
> >   *   fully-imprecise {frame=ARG_IMPRECISE, mask=bitmask}
> >   *                                 -- unknown frame identity; .mask is a
> > @@ -607,8 +607,6 @@ enum arg_track_state {
> >         ARG_IMPRECISE   = -3,   /* lost identity; .mask is arg bitmask */
> >  };
> > 
> > -#define OFF_IMPRECISE  S16_MIN /* arg identity known but offset unknown */
> > -
> >  /* Track callee stack slots fp-8 through fp-512 (64 slots of 8 bytes each) */
> >  #define MAX_ARG_SPILL_SLOTS 64
> > 
> > @@ -622,28 +620,6 @@ static bool arg_is_fp(const struct arg_track *at)
> >         return at->frame >= 0 || at->frame == ARG_IMPRECISE;
> >  }
> > 
> > -/*
> > - * Clear all tracked callee stack slots overlapping the byte range
> > - * [off, off+sz-1] where off is a negative FP-relative offset.
> > - */
> > -static void clear_overlapping_stack_slots(struct arg_track *at_stack, s16 off, u32 sz)
> > -{
> > -       struct arg_track none = { .frame = ARG_NONE };
> > -
> > -       if (off == OFF_IMPRECISE) {
> > -               for (int i = 0; i < MAX_ARG_SPILL_SLOTS; i++)
> > -                       at_stack[i] = none;
> > -               return;
> > -       }
> > -       for (int i = 0; i < MAX_ARG_SPILL_SLOTS; i++) {
> > -               int slot_start = -((i + 1) * 8);
> > -               int slot_end = slot_start + 8;
> > -
> > -               if (slot_start < off + (int)sz && slot_end > off)
> > -                       at_stack[i] = none;
> > -       }
> > -}
> > -
> >  static void verbose_arg_track(struct bpf_verifier_env *env, struct arg_track *at)
> >  {
> >         int i;
> > @@ -863,16 +839,15 @@ static void arg_track_alu64(struct arg_track *dst, const struct arg_track *src)
> >         *dst = arg_join_imprecise(*dst, *src);
> >  }
> > 
> > -static s16 arg_add(s16 off, s64 delta)
> > +static bool arg_add(s16 off, s64 delta, s16 *out)
> >  {
> >         s64 res;
> > 
> > -       if (off == OFF_IMPRECISE)
> > -               return OFF_IMPRECISE;
> >         res = (s64)off + delta;
> > -       if (res < S16_MIN + 1 || res > S16_MAX)
> > -               return OFF_IMPRECISE;
> > -       return res;
> > +       if (res < S16_MIN || res > S16_MAX)
> > +               return true;
> > +       *out = res;
> > +       return false;
> 
> Nice. It's almost check_add_overflow().
> May be something like:
>   s16 d = delta;
>   if (d != delta)
>     return true;
>   return check_add_overflow(off, d, out);

I modeled api after the check_add_overflow() but using it directly
didn't occur to me for some reason.

I don't think the s16 cast is necessary, check_add_overflow results in
a call to __builtin_add_overflow(), which is documented as:

  > Built-in Function: bool __builtin_add_overflow (type1 a, type2 b, type3 *res)
  >   ...
  >   These built-in functions promote the first two operands into
  >   infinite precision signed type and perform addition on those
  >   promoted operands. The result is then cast to the type the third
  >   pointer argument points to and stored there. If the stored result is
  >   equal to the infinite precision result, the built-in functions
  >   return false, otherwise they return true. As the addition is
  >   performed in infinite signed precision, these built-in functions
  >   have fully defined behavior for all argument values.

> I'm curious whether asm will be better.

Disasm looks identical.
I'll respin with check_add_overflow(), fixes tag and a test nit from sashiko.