Re: [PATCH bpf-next 6/7] bpf: preserve constant zero when doing partial register restore

Eduard Zingerman
2023-11-09T19:34:56Z

On Thu, 2023-11-09 at 09:41 -0800, Andrii Nakryiko wrote:
> On Thu, Nov 9, 2023 at 7:21 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Mon, 2023-10-30 at 22:03 -0700, Andrii Nakryiko wrote:
> > > Similar to special handling of STACK_ZERO, when reading 1/2/4 bytes from
> > > stack from slot that has register spilled into it and that register has
> > > a constant value zero, preserve that zero and mark spilled register as
> > > precise for that. This makes spilled const zero register and STACK_ZERO
> > > cases equivalent in their behavior.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > Could you please add a test case?
> >
>
> There is already at least one test case that relies on this behavior
> :) But yep, I'll add a dedicated test.

Thank you. Having a dedicated test always helps with debugging, should
something go wrong.

[...]

> > Condition for this branch is (off % BPF_REG_SIZE != 0) || size != spill_size,
> > is it necessary to check for some unusual offsets, e.g. off % BPF_REG_SIZE == 7
> > or something like that?
>
> I don't think so. We rely on all bytes we are reading to be either
> spills (and thus spill_cnt == size), in which case verifier logic
> makes sure we have spill at slot boundary (off % BPF_REG_SIZE == 0).
> Or it's all STACK_ZERO, and then zero_cnt == size, in which case we
> know it's zero.
>
> Unless I missed something else?

False alarm, 'slot' is derived from 'off' and the loop checks
'type = stype[(slot - i) % BPF_REG_SIZE];', sorry for the noise.