On Wed, Nov 29, 2023 at 8:48 AM Andrei Matei <andreimatei1@xxxxxxxxx> wrote:
>
> [...]
>
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index af2819d5c8ee..f9546dd73f3c 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -1685,10 +1685,12 @@ static int resize_reference_state(struct bpf_func_state *state, size_t n)
> > > return 0;
> > > }
> > >
> > > -static int grow_stack_state(struct bpf_func_state *state, int size)
> > > +/* Possibly update state->allocated_stack to be at least size bytes. Also
> > > + * possibly update the function's high-water mark in its bpf_subprog_info.
> > > + */
> > > +static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state *state, int size)
> > > {
> > > size_t old_n = state->allocated_stack / BPF_REG_SIZE, n = size / BPF_REG_SIZE;
> >
> > shouldn't this be rounding up? (size + BPF_REG_SIZE - 1) / BPF_REG_SIZE?
>
> You're saying this was always broken, regardless of the current patch, right? I
I think so, yes...
> think you're right, but that seems like a bug that should have been
> caught somehow; I'm surprised no programs crashed the verifier. Perhaps in
> practice all stack accesses are 8-byte aligned, so the rounding doesn't matter?
>
> I'll spend a bit of time reading code and come back.
Thanks!
>
> [...]
