Skip to content

Commit 1c66f4a

Browse files
luisgerhorstAlexei Starovoitov
authored andcommitted
bpf: Fix state use-after-free on push_stack() err
Without this, `state->speculative` is used after the cleanup cycles in push_stack() or push_async_cb() freed `env->cur_state` (i.e., `state`). Avoid this by relying on the short-circuit logic to only access `state` if the error is recoverable (and make sure it never is after push_*() failed). push_*() callers must always return an error for which error_recoverable_with_nospec(err) is false if push_*() returns NULL, otherwise we try to recover and access the stale `state`. This is only violated by sanitize_ptr_alu(), thus also fix this case to return -ENOMEM. state->speculative does not make sense if the error path of push_*() ran. In that case, `state->speculative && error_recoverable_with_nospec(err)` as a whole should already never evaluate to true (because all cases where push_stack() fails must return -ENOMEM/-EFAULT). As mentioned, this is only violated by the push_stack() call in sanitize_speculative_path() which returns -EACCES without [1] (through REASON_STACK in sanitize_err() after sanitize_ptr_alu()). To fix this, return -ENOMEM for REASON_STACK (which is also the behavior we will have after [1]). Checked that it fixes the syzbot reproducer as expected. [1] https://lore.kernel.org/all/[email protected]/ Fixes: d6f1c85 ("bpf: Fall back to nospec for Spectre v1") Reported-by: [email protected] Reported-by: Eduard Zingerman <[email protected]> Link: https://lore.kernel.org/all/[email protected]/ Signed-off-by: Luis Gerhorst <[email protected]> Acked-by: Eduard Zingerman <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 05e2249 commit 1c66f4a

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

kernel/bpf/verifier.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14293,7 +14293,7 @@ static int sanitize_err(struct bpf_verifier_env *env,
1429314293
case REASON_STACK:
1429414294
verbose(env, "R%d could not be pushed for speculative verification, %s\n",
1429514295
dst, err);
14296-
break;
14296+
return -ENOMEM;
1429714297
default:
1429814298
verbose(env, "verifier internal error: unknown reason (%d)\n",
1429914299
reason);
@@ -19926,7 +19926,7 @@ static int do_check(struct bpf_verifier_env *env)
1992619926
goto process_bpf_exit;
1992719927

1992819928
err = do_check_insn(env, &do_print_state);
19929-
if (state->speculative && error_recoverable_with_nospec(err)) {
19929+
if (error_recoverable_with_nospec(err) && state->speculative) {
1993019930
/* Prevent this speculative path from ever reaching the
1993119931
* insn that would have been unsafe to execute.
1993219932
*/

0 commit comments

Comments
 (0)