> On Mon, 2022-06-13 at 08:48 -0700, Song Liu wrote: > > +static int optimize_bpf_loop(struct bpf_verifier_env *env) > > + new_prog = inline_bpf_loop(env, > > + i + delta, > > + -(stack_depth + stack_depth_extra), > > + inline_state->callback_subprogno, > > + &cnt); > > + if (!new_prog) > > + return -ENOMEM; > > We do not fail over for -ENOMEM, which is reasonable. (It is also reasonable if > we do fail the program with -ENOMEM. However, if we don't fail the program, > we need to update stack_depth properly before returning, right? > Ouch, you are correct! Sorry, this was really sloppy on my side. The behavior here should be the same as in `do_misc_fixups` which does fail in case of -ENOMEM. In order to do the same `optimize_bpf_loop` should remain as is but the following part of the patch has to be updated: > @@ -15031,6 +15193,9 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr) > ret = check_max_stack_depth(env); > > /* instruction rewrites happen after this point */ > + if (ret == 0) > + optimize_bpf_loop(env); > + It should be as follows: + if (ret == 0) + ret = optimize_bpf_loop(env); // added `ret` assignment! Not sure if there is a reasonable way to write a test for this case. I will add this change and produce the v7 today. Do you see anything else that should be updated? Thanks, Eduard
