On Mon, 2024-08-12 at 10:50 -0700, Alexei Starovoitov wrote:
> On Mon, Aug 12, 2024 at 10:47 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
> >
> > On Mon, 2024-08-12 at 10:44 -0700, Alexei Starovoitov wrote:
> >
> > [...]
> >
> > > Should we move the check up instead?
> > >
> > > if (i >= cur->allocated_stack)
> > > return false;
> > >
> > > Checking it twice looks odd.
> >
> > A few checks before that, namely:
> >
> > if (!(old->stack[spi].spilled_ptr.live & REG_LIVE_READ)
> > && exact == NOT_EXACT) {
> > i += BPF_REG_SIZE - 1;
> > /* explored state didn't use this */
> > continue;
> > }
> >
> > if (old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_INVALID)
> > continue;
> >
> > if (env->allow_uninit_stack &&
> > old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_MISC)
> > continue;
> >
> > Should be done regardless cur->allocated_stack.
>
> Right, but then let's sink old->slot_type != cur->slot_type down?
It does not seem correct to swap the order for these two checks:
if (exact != NOT_EXACT && i < cur->allocated_stack &&
old->stack[spi].slot_type[i % BPF_REG_SIZE] !=
cur->stack[spi].slot_type[i % BPF_REG_SIZE])
return false;
if (!(old->stack[spi].spilled_ptr.live & REG_LIVE_READ)
&& exact == NOT_EXACT) {
i += BPF_REG_SIZE - 1;
/* explored state didn't use this */
continue;
}
if we do, 'slot_type' won't be checked for 'cur' when 'old' register is not marked live.
