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

Eduard Zingerman
2026-04-13T22:57:11Z

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.