Eduard Zingerman <eddyz87@xxxxxxxxx> writes:
> Could you please point me to a location, where exact error code
> returned by updated push_stack() matters?
> I checked push_stack() callgraph (in the attachment), but can't find
> anything.
Only with the final patch 11 ("bpf: Fall back to nospec for spec path
verification") applied, the error code should matter. Then, the error
code either matches `state->speculative &&
error_recoverable_with_nospec(err)` in do_check() if it was EINVAL (in
this case we heuristically avoided nested speculative path verification
but have to add a nospec), or we continue to raise the error (e.g.,
ENOMEM) from do_check().
Or is your question on this part from the commit message of patch 9?
This changes the sanitization-case to returning -ENOMEM. However, this
is more fitting as -EFAULT would indicate a verifier-internal bug.
This was referring to the sanitize_speculative_path() calls in
check_cond_jmp_op(). For that case, the error should also only be used
in do_check() with patch 11 applied. However, regarding this, EFAULT and
ENOMEM are treated the same (they both don't satisfy
error_recoverable_with_nospec()), therefore this change is primarily
made to not complicate the code.
I just became aware that there is some special handling of EFAULT as
discussed in c7a897843224 ("bpf: don't leave partial mangled prog in
jit_subprogs error path"). I will have look into this in detail to make
sure changing push_stack() from EFAULT to ENOMEM is OK.
Hope this answers your question.
Adding some of these details to v2 won't hurt I guess.
