bpf: Remove redundant free_verifier_state()/pop_stack()
authorLuis Gerhorst <luis.gerhorst@fau.de>
Fri, 13 Jun 2025 09:01:58 +0000 (11:01 +0200)
committerAlexei Starovoitov <ast@kernel.org>
Fri, 13 Jun 2025 21:59:30 +0000 (14:59 -0700)
commitf66b4aaff2548bed5eedded0f47ae3a9ac933cec
tree2c7d0331841ea052dbfa63e1b222b5667202c6b4
parent4a4b84ba9e453295c746d81cb245c0c5d80050f0
bpf: Remove redundant free_verifier_state()/pop_stack()

This patch removes duplicated code.

Eduard points out [1]:

    Same cleanup cycles are done in push_stack() and push_async_cb(),
    both functions are only reachable from do_check_common() via
    do_check() -> do_check_insn().

    Hence, I think that cur state should not be freed in push_*()
    functions and pop_stack() loop there is not needed.

This would also fix the 'symptom' for [2], but the issue also has a
simpler fix which was sent separately. This fix also makes sure the
push_*() callers always return an error for which
error_recoverable_with_nospec(err) is false. This is required because
otherwise we try to recover and access the stale `state`.

Moving free_verifier_state() and pop_stack(..., pop_log=false) to happen
after the bpf_vlog_reset() call in do_check_common() is fine because the
pop_stack() call that is moved does not call bpf_vlog_reset() with the
pop_log=false parameter.

[1] https://lore.kernel.org/all/b6931bd0dd72327c55287862f821ca6c4c3eb69a.camel@gmail.com/
[2] https://lore.kernel.org/all/68497853.050a0220.33aa0e.036a.GAE@google.com/

Reported-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/all/b6931bd0dd72327c55287862f821ca6c4c3eb69a.camel@gmail.com/
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Luis Gerhorst <luis.gerhorst@fau.de>
Link: https://lore.kernel.org/r/20250613090157.568349-2-luis.gerhorst@fau.de
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
kernel/bpf/verifier.c