On Mon, 2023-10-30 at 22:03 -0700, Andrii Nakryiko wrote: > Instead of always forcing STACK_ZERO slots to STACK_MISC, preserve it in > situations where this is possible. E.g., when spilling register as > 1/2/4-byte subslots on the stack, all the remaining bytes in the stack > slot do not automatically become unknown. If we knew they contained > zeroes, we can preserve those STACK_ZERO markers. > > Add a helper mark_stack_slot_misc(), similar to scrub_spilled_slot(), > but that doesn't overwrite either STACK_INVALID nor STACK_ZERO. Note > that we need to take into account possibility of being in unprivileged > mode, in which case STACK_INVALID is forced to STACK_MISC for correctness, > as treating STACK_INVALID as equivalent STACK_MISC is only enabled in > privileged mode. > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> Could you please add a test case? Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx> [...] > @@ -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"? [...]
