Re: [PATCH bpf-next 5/7] bpf: preserve STACK_ZERO slots on partial reg spills

Eduard Zingerman
2023-11-09T17:54:31Z

On Thu, 2023-11-09 at 09:37 -0800, Andrii Nakryiko wrote:
[...]
> > > @@ -1355,6 +1355,21 @@ static void scrub_spilled_slot(u8 *stype)
> > > *stype = STACK_MISC;
> > > }
> > >
> > > +/* Mark stack slot as STACK_MISC, unless it is already STACK_INVALID, in which
> > > + * case they are equivalent, or it's STACK_ZERO, in which case we preserve
> > > + * more precise STACK_ZERO.
> > > + * Note, in uprivileged mode leaving STACK_INVALID is wrong, so we take
> > > + * env->allow_ptr_leaks into account and force STACK_MISC, if necessary.
> > > + */
> > > +static void mark_stack_slot_misc(struct bpf_verifier_env *env, u8 *stype)
> >
> > Nitpick: I find this name misleading, maybe something like "remove_spill_mark"?
>
> remove_spill_mark is even more misleading, no? there is also DYNPTR
> and ITER stack slots?

Right, forgot about those...

>
> maybe mark_stack_slot_scalar (though that's a bit misleading as well,
> as can be understood as marking slot as spilled SCALAR_VALUE
> register)? not sure, I think "slot_misc" is close enough as an
> approximation of what it's doing, modulo ZERO/INVALID

maybe_mark_stack_slot_misc?
The other similar function is named 'scrub_spilled_slot'.