Re: [PATCH bpf-next v7 21/22] selftests/bpf: Cover more accepted .bpf_cleanup exception shapes

[Yonghong Song <yonghong.song@linux.dev>]

On 9/28/26 5:52 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/exceptions_cleanup.h b/tools/testing/selftests/bpf/exceptions_cleanup.h
>> index d1d40314035e..616b479238c7 100644
>> --- a/tools/testing/selftests/bpf/exceptions_cleanup.h
>> +++ b/tools/testing/selftests/bpf/exceptions_cleanup.h
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
>> index 255f88d35aad..c06ec10359b9 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
>> @@ -4,6 +4,7 @@
>>   #include "exceptions_cleanup.h"
>>   #include "exceptions_cleanup.skel.h"
>>   #include "exceptions_cleanup_fail.skel.h"
>> +#include "exceptions_cleanup_shapes.skel.h"
>>
>>   /* foo3 unwound: every frame that has a pad ran it. */
>>   #define PADS_FOO3_UNWOUND \
>> @@ -82,4 +83,5 @@ void test_exceptions_cleanup(void)
>>   	exceptions_cleanup__destroy(skel);
>>
>>   	RUN_TESTS(exceptions_cleanup_fail);
>> +	RUN_TESTS(exceptions_cleanup_shapes);
>>   }
> A subsystem pattern flags this as potentially concerning: The only skip
> gate in test_exceptions_cleanup() is the -EOPNOTSUPP check on the main
> exceptions_cleanup skeleton, which tests bpf_jit_supports_cleanup_pads().
>
> RUN_TESTS(exceptions_cleanup_shapes) then loads entry_callx_region, which
> also needs indirect-call JIT support. progs/exceptions_cleanup_shapes.c
> guards that program only on the compiler and arch:
>
>      /* gcc has no indirect calls, and only these JITs emit them */
>      #if defined(__clang__) && \
>      	(defined(__TARGET_ARCH_x86) || defined(__TARGET_ARCH_arm64))
>
> On x86 the two capabilities are independent:
>
> arch/x86/net/bpf_jit_comp.c:
>
>      bool bpf_jit_supports_callx(void)
>      {
>      	return cfi_mode != CFI_FINEIBT;
>      }
>
>      bool bpf_jit_supports_cleanup_pads(void)
>      {
>      	return IS_ENABLED(CONFIG_UNWINDER_ORC);
>      }
>
> With a kCFI kernel on IBT hardware (cfi=auto resolves to CFI_FINEIBT in
> arch/x86/kernel/alternative.c), the main skeleton loads, so the skip
> branch is not taken. The verifier's require_callx_jit() then refuses
> entry_callx_region with -EOPNOTSUPP ("JIT doesn't support callx").
> test_loader's run_subtest() treats any load error on a __success program
> as ASSERT_OK(err, "unexpected_load_failure"), so this subtest fails
> instead of skipping, even though the host simply lacks the feature.
>
> Could the test check for callx support separately and skip on FineIBT
> hosts? The existing callx selftests (RUN_JITED(verifier_callx) in
> prog_tests/verifier.c and callx_func_ptr_map.c) do not skip on FineIBT
> either; they check only is_jit_enabled(). So this shape follows the
> existing convention, and the failure is limited to x86 FineIBT hosts
> (clang kCFI build plus IBT-capable CPU).

This really depends on the config. Currently CI config does not have
CONFIG_CFI so the test should be okay. This is just a CI test and we
can adjust later if config changed.

>
>> diff --git a/tools/testing/selftests/bpf/progs/exceptions_cleanup_shapes.c b/tools/testing/selftests/bpf/progs/exceptions_cleanup_shapes.c
>> new file mode 100644
>> index 000000000000..1978fd1102f0
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/exceptions_cleanup_shapes.c
> [ ... ]
>
>> +/*
>> + * A precision chain crossing a resume: r6 is kept across a call whose only
>> + * way back is the callee's pad, then used as a variable stack offset.
>> + */
>> +static __used __naked __noinline __u64 prec_inner_frame(void)
>> +{
>> +	asm volatile (
>> +"1:"	"call bpf_unwind;"		/* cleanup region */
>> +"2:"
>> +	"r0 = 0;"
>> +	"exit;"
>> +"3:"					/* landing pad */
>> +	"call bpf_unwind_resume;"
>> +	"exit;"
>> +	CLEANUP_REC("1b", "2b", "3b")
>> +	::: __clobber_all);
>> +}
>> +
>> +static __used __naked __noinline __u64 prec_outer_frame(void)
>> +{
>> +	asm volatile (
>> +	"r1 = %[input] ll;"
>> +	"r6 = *(u64 *)(r1 + 0);"
>> +	"r6 &= 0x7;"
>> +	"r0 = 0;"
>> +	"*(u64 *)(r10 - 8) = r0;"
>> +	"*(u64 *)(r10 - 16) = r0;"
>> +	"call prec_inner_frame;"	/* comes back only through the pad */
>> +	"r2 = r10;"
>> +	"r2 += -16;"
>> +	"r2 += r6;"			/* variable stack offset: r6 must be precise */
>> +	"*(u8 *)(r2 + 0) = 1;"
>> +	"r0 = 0;"
>> +	"exit;"
>> +	:
>> +	: __imm_addr(input)
>> +	: __clobber_all);
>> +}
>> +
>> +SEC("?syscall")
>> +__success __set_global(input, 101) __retval(0)
>> +int entry_prec_across_resume(void *ctx)
>> +{
>> +	return prec_outer_frame();
>> +}
> A subsystem pattern flags this as potentially concerning:
> entry_prec_across_resume is meant to cover "a precision chain crossing a
> resume" (per the commit message), but does the verifier actually check the
> code after the call?
>
> prec_inner_frame always unwinds into its own pad. The pad ends in
> bpf_unwind_resume while curframe is 1. The resume handler in
> do_check_insn() ends the path there:
>
> kernel/bpf/verifier.c:
>
>      /*
>       * No need to walk into the caller: its pad was
>       * pushed as a branch at its call, and with no
>       * pad nothing of it runs.
>       */
>      if (env->cur_state->curframe)
>      	return PROCESS_BPF_EXIT;
>
> prec_outer_frame has no CLEANUP_REC over "call prec_inner_frame", so no
> pad branch is pushed there either. The fall-through of bpf_unwind in
> prec_inner_frame is never walked, because process_bpf_unwind() jumps
> straight to the pad. As a result, none of these instructions are ever
> verified:
>
>      "r2 = r10;"
>      "r2 += -16;"
>      "r2 += r6;"
>      "*(u8 *)(r2 + 0) = 1;"
>
> They never run either: bpf_unwind_rewrite() points prec_outer_frame's
> uncovered return at the epilogue.
>
> So the variable-offset stack write never runs mark_chain_precision() on
> r6. The resume-crossing branch in kernel/bpf/backtrack.c (the
> bt_subprog_enter() call made on a resume) is never exercised. Would the
> test still pass if "r6 &= 0x7" were deleted, or if the backtrack.c resume
> handling were broken?
>
> Both the comment "comes back only through the pad" and the header "r6 is
> kept across a call whose only way back is the callee's pad" appear
> inconsistent with this: nothing comes back. Could this shape be adjusted
> to cover the call in prec_outer_frame with a CLEANUP_REC and do the r6
> variable-offset access in prec_outer_frame's own pad, so a pad path
> verifies the use and precision is propagated back through the call?

Okay, will modify the test to have proper precision checking.

>
>> +/*
>> + * The jump_into_pad shape with the branch statically dead, so only a
>> + * speculative walk reaches the pad: a barrier rather than a refusal.
>> + */
>> +static __used __naked __noinline __u64 dead_jump_into_pad_frame(void)
>> +{
>> +	asm volatile (
>> +	"r6 = 0;"
>> +	"if r6 > 7 goto 4f;"		/* never taken: walked speculatively */
>> +"1:"	"call always_unwind;"		/* cleanup region */
>> +"2:"
>> +	"r0 = 0;"
>> +	"exit;"
>> +"3:"					/* landing pad */
>> +	"r7 = r0;"
>> +"4:"					/* ... and its second instruction */
>> +	"call bpf_unwind_resume;"
>> +	"exit;"
>> +	CLEANUP_REC("1b", "2b", "3b")
>> +	::: __clobber_all);
>> +}
>> +
>> +SEC("?syscall")
>> +__success
>> +int dead_jump_into_pad(void *ctx)
>> +{
>> +	return dead_jump_into_pad_frame();
>> +}
> A subsystem pattern flags this as potentially concerning:
> dead_jump_into_pad is meant to cover "a pad reached only by a speculative
> walk" (per the commit message), but does the verifier actually do a
> speculative walk here?
>
> The program has only __success, so test_loader loads it only in privileged
> mode, as root with all capabilities. The verifier sets:
>
>      env->bypass_spec_v1 = bpf_bypass_spec_v1(env->prog->aux->token);
>
> bpf_bypass_spec_v1() is true when bpf_token_capable(token, CAP_PERFMON)
> holds, which root satisfies. In check_cond_jmp_op(), the dead branch of
> "if r6 > 7" (pred == 0) is pushed only when bypass_spec_v1 is false:
>
> kernel/bpf/verifier.c:
>
>      } else if (pred == 0) {
>      	...
>      	if (!env->bypass_spec_v1) {
>      		err = sanitize_speculative_path(env, insn, *insn_idx + insn->off + 1,
>      						*insn_idx);
>
> So label 4 is never walked. bpf_exc_check_insn() never reports "runs both
> inside and outside a landing pad" on a speculative state, and the
> error_recoverable_with_nospec() && state->speculative recovery in
> do_check() never runs. The program loads because the verifier ignores the
> dead branch entirely, not because of the barrier logic. Would the test
> still pass unchanged if that recovery were removed and the error made
> fatal?
>
> The comment "never taken: walked speculatively" and the header "only a
> speculative walk reaches the pad: a barrier rather than a refusal" describe
> something that does not happen under the test's own settings.
>
> Could this also run the program unprivileged with CAP_BPF kept, so it
> loads but CAP_PERFMON is gone and bypass_spec_v1 is false? A syscall
> program is not a perfmon program type, so CAP_BPF is enough to load it:
>
>      SEC("?syscall")
>      __success __caps_unpriv(CAP_BPF) __success_unpriv
>      int dead_jump_into_pad(void *ctx)

Okay, I will try this one "__success __caps_unpriv(CAP_BPF) __success_unpriv".

>
> It could also check the verifier log for the nospec insertion.
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36502848904