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/20250603213232.339242-1-luis.gerhorst@fau.de/

Fixes: d6f1c85f2253 ("bpf: Fall back to nospec for Spectre v1")
Reported-by: syzbot+b5eb72a560b8149a1885@syzkaller.appspotmail.com
Reported-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/all/38862a832b91382cddb083dddd92643bed0723b8.camel@gmail.com/
Signed-off-by: Luis Gerhorst <luis.gerhorst@fau.de>
---
 kernel/bpf/verifier.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b1f797616f20..d3bff0385a55 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14229,7 +14229,7 @@ static int sanitize_err(struct bpf_verifier_env *env,
 	case REASON_STACK:
 		verbose(env, "R%d could not be pushed for speculative verification, %s\n",
 			dst, err);
-		break;
+		return -ENOMEM;
 	default:
 		verbose(env, "verifier internal error: unknown reason (%d)\n",
 			reason);
@@ -19753,7 +19753,7 @@ static int do_check(struct bpf_verifier_env *env)
 			goto process_bpf_exit;
 
 		err = do_check_insn(env, &do_print_state);
-		if (state->speculative && error_recoverable_with_nospec(err)) {
+		if (error_recoverable_with_nospec(err) && state->speculative) {
 			/* Prevent this speculative path from ever reaching the
 			 * insn that would have been unsafe to execute.
 			 */

base-commit: 2d72dd14d77f31a7caa619fe0b889304844e612e
-- 
2.49.0

On Wed, 2025-06-11 at 23:07 +0200, Luis Gerhorst wrote:

[...]

> Signed-off-by: Luis Gerhorst <luis.gerhorst@fau.de>

I reproduced the error locally and this patch fixes it.
Also double-checked places where free_verifier_state is called
and error codes used in error_recoverable_with_nospec() are used.
Looks like env->cur_state should be always ok if
error_recoverable_with_nospec() recovers, env internal structures
in healthy state.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

> ---
>  kernel/bpf/verifier.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index b1f797616f20..d3bff0385a55 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14229,7 +14229,7 @@ static int sanitize_err(struct bpf_verifier_env *env,
>  	case REASON_STACK:
>  		verbose(env, "R%d could not be pushed for speculative verification, %s\n",
>  			dst, err);
> -		break;
> +		return -ENOMEM;

Good catch, I would have probably missed it.

>  	default:
>  		verbose(env, "verifier internal error: unknown reason (%d)\n",
>  			reason);
> @@ -19753,7 +19753,7 @@ static int do_check(struct bpf_verifier_env *env)
>  			goto process_bpf_exit;
>  
>  		err = do_check_insn(env, &do_print_state);
> -		if (state->speculative && error_recoverable_with_nospec(err)) {
> +		if (error_recoverable_with_nospec(err) && state->speculative) {
>  			/* Prevent this speculative path from ever reaching the
>  			 * insn that would have been unsafe to execute.
>  			 */
> 
> base-commit: 2d72dd14d77f31a7caa619fe0b889304844e612e