Re: [PATCH bpf-next v10 15/24] bpf, arm64: Dispatch exception cleanup pads at run time

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

On 10/9/26 6:30 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] arm64/bpf: PAC modifier mismatch for bpf_throw's C frame causes FPAC panic
> --
>
> commit 2f751c03b321c9d53d97196968f653cf6c3fa289
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf, arm64: Dispatch exception cleanup pads at run time
>
> This commit adds arm64 support for unwinding BPF frames and redirecting
> their return addresses to cleanup pads. It signs the modified return
> addresses using PAC when address authentication is enabled.
>
>> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
>> index 3ebcf8c53fb04..56d1310eca323 100644
>> --- a/arch/arm64/kernel/stacktrace.c
>> +++ b/arch/arm64/kernel/stacktrace.c
> [ ... ]
>
>> +/* PACIASP's modifier is the entry sp, record + 16 for a BPF prologue. */
>> +static void bpf_unwind_store_ra(unsigned long record, u64 ra)
>> +{
>> +	struct frame_record *rec = (struct frame_record *)record;
>> +
>> +	/*
>> +	 * Whether the slot holds a signed address is a property of the build,
>> +	 * not one to be read off the value: a PAC can come out equal to the
>> +	 * bits stripping puts back, and a signed address would then be taken
>> +	 * for an unsigned one. What signs is CONFIG_ARM64_PTR_AUTH_KERNEL --
>> +	 * the prologue here, and -mbranch-protection for everything the
>> +	 * compiler emits.
>> +	 */
>> +	if (IS_ENABLED(CONFIG_ARM64_PTR_AUTH_KERNEL) &&
>> +	    system_supports_address_auth())
>> +		ra = bpf_unwind_sign_ra(ra, record + sizeof(struct frame_record));
> [Severity: High]
> Will this hardcoded PAC modifier cause an FPAC panic when returning from
> bpf_throw()?

bpf_throw() cannot go together with bpf_unwind().

>
> The unwinder uses bpf_unwind_store_ra() to redirect the return address for
> the first BPF frame. The caller of this first BPF frame is the C function
> bpf_throw().
>
> While record + sizeof(struct frame_record) correctly assumes a strict BPF
> JIT prologue where the entry SP is FP + 16, bpf_throw() is a compiled C
> function. Its actual frame size is compiler-dependent and may be larger
> than 16 bytes due to local variables.
>
> When bpf_throw() returns, its epilogue will authenticate the return address
> using its actual entry SP. If we sign it here using FP + 16 instead of the
> actual entry SP, could authentication fail and trigger a kernel panic on
> PAC-enabled kernels?
>
>> +	WRITE_ONCE(rec->lr, ra);
>> +}
> [ ... ]
>