| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6873d674-316e-4bc2-a39d-52ba4239e395@linux.ibm.com>
Date: Tue, 27 Jan 2026 08:02:29 +0530
From: Hari Bathini <hbathini@...ux.ibm.com>
To: bot+bpf-ci@...nel.org, adubey@...ux.ibm.com, bpf@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: sachinpb@...ux.ibm.com, venkat88@...ux.ibm.com, andrii@...nel.org,
eddyz87@...il.com, mykolal@...com, ast@...nel.org,
daniel@...earbox.net, martin.lau@...ux.dev, song@...nel.org,
yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org,
christophe.leroy@...roup.eu, naveen@...nel.org, maddy@...ux.ibm.com,
mpe@...erman.id.au, npiggin@...il.com, memxor@...il.com,
iii@...ux.ibm.com, shuah@...nel.org, martin.lau@...nel.org,
clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH v5 6/6] powerpc64/bpf: Additional NVR handling for
bpf_throw
On 27/01/26 12:00 am, Hari Bathini wrote:
>
>
> On 24/01/26 1:53 pm, bot+bpf-ci@...nel.org wrote:
>>> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/
>>> bpf_jit_comp64.c
>>> index db121b1404fe..17de8b53a962 100644
>>> --- a/arch/powerpc/net/bpf_jit_comp64.c
>>> +++ b/arch/powerpc/net/bpf_jit_comp64.c
>>
>> [ ... ]
>>
>>> +/*
>>> + * For exception boundary & exception_cb progs:
>>> + * return increased size to accommodate additional NVRs.
>>> + */
>>> +static int bpf_jit_stack_size(struct codegen_context *ctx)
>>> +{
>>> + return ctx->exception_boundary || ctx->exception_cb ?
>>> + BPF_PPC_EXC_STACKFRAME :
>>> + BPF_PPC_STACKFRAME;
>>> +}
>>
>> [ ... ]
>>
>>> @@ -198,7 +249,19 @@ void bpf_jit_build_prologue(u32 *image, struct
>>> codegen_context *ctx)
>>> EMIT(PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF));
>>> }
>>>
>>> - EMIT(PPC_RAW_STDU(_R1, _R1, -(BPF_PPC_STACKFRAME + ctx-
>>> >stack_size)));
>>> + EMIT(PPC_RAW_STDU(_R1, _R1,
>>> + -(bpf_jit_stack_size(ctx) + ctx->stack_size)));
>>> + }
>>
>> [ ... ]
>>
>>> @@ -248,9 +311,19 @@ static void bpf_jit_emit_common_epilogue(u32
>>> *image, struct codegen_context *ctx)
>>> EMIT(PPC_RAW_LD(bpf_to_ppc(ARENA_VM_START), _R1,
>>> bpf_jit_stack_offsetof(ctx,
>>> bpf_to_ppc(ARENA_VM_START))));
>>>
>>> + if (ctx->exception_cb) {
>>> + /*
>>> + * Recover additionally saved non volatile registers from stack
>>> + * frame of exception boundary program.
>>> + */
>>> + for (i = _R14; i <= _R25; i++)
>>> + EMIT(PPC_RAW_LD(i, _R1, bpf_jit_stack_offsetof(ctx, i)));
>>> + }
>>> +
>>> /* Tear down our stack frame */
>>> if (bpf_has_stack_frame(ctx)) {
>>> - EMIT(PPC_RAW_ADDI(_R1, _R1, BPF_PPC_STACKFRAME + ctx-
>>> >stack_size));
>>> + EMIT(PPC_RAW_ADDI(_R1, _R1, bpf_jit_stack_size(ctx) + ctx-
>>> >stack_size));
>>
>> For the exception callback case, doesn't this use the wrong stack_size?
>>
>> In bpf_jit_build_prologue(), the exception callback does not create
>> its own
>> frame - it sets R1 to the boundary's frame pointer via
>> EMIT(PPC_RAW_MR(_R1,
>> _R5)). However here in the epilogue, the frame teardown uses ctx-
>> >stack_size
>> which is the callback's stack depth, not the boundary's.
>>
>> The boundary program creates its frame with:
>> STDU R1, R1, -(bpf_jit_stack_size(boundary_ctx) +
>> boundary_stack_size)
>>
>> But the callback tears it down with:
>> ADDI R1, R1, bpf_jit_stack_size(callback_ctx) + callback_stack_size
>>
>> Since each BPF subprogram can have a different stack_depth (set per-
>> subprog
>> in kernel/bpf/verifier.c via func[i]->aux->stack_depth), if the exception
>> callback has a different stack depth than the boundary, R1 will end up at
>> the wrong location after this ADDI.
>>
>> After the wrong R1 adjustment, the LR restore below loads from R1+16
>> which
>> is now pointing to the wrong memory location, potentially causing a
>> return
>> to an invalid address.
>>
>> Would it be correct to store the boundary's stack_size somewhere
>> accessible
>> to the callback, or ensure the callback uses the same stack layout as the
>> boundary?
>
> That is a good catch! Jitting the rollback of the stack frame of
> exception boundary program in exception callback program is tricky
> as stack_depth of exception boundary program is not readily available
> while jitting exception callback program...
>
> "ld r1, 0(r1)" instruction can be used instead of "addi r1, r1, size"
> form for tearing down the stack.
>
> The other issue is calculating the offset of non volatile registers
> to restore them from the stack. To reliably restore, move one frame
> above exception boundary frame and restore NVRs with redzone offset
> without relying on stack_depth of exception boundary program.
>
> Also, as exception callback may also use stack, setup frame for
> exception callback and restore BPF JIT NVRs in its epilogue.
>
> In essence, something like the below is needed on top of your patch..
>
> ---
>
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/
> bpf_jit_comp64.c
> index b1a3945ccc9f..15342d76f96f 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -32,7 +32,8 @@
> *
> * [ prev sp ] <-------------
> * [ tail_call_info ] 8 |
> - * [ nv gpr save area ] 6*8 + (12*8) |
> + * [ nv gpr save area ] 6*8 |
> + * [ addl. nv gpr save area] (12*8) | <--- only in exception
> boundary program
> * [ local_tmp_var ] 24 |
> * fp (r31) --> [ ebpf stack space ] upto 512 |
> * [ frame header ] 32/112 |
> @@ -125,7 +126,8 @@ static inline bool bpf_has_stack_frame(struct
> codegen_context *ctx)
> * [ ... ] |
> * sp (r1) ---> [ stack pointer ] --------------
> * [ tail_call_info ] 8
> - * [ nv gpr save area ] 6*8 + (12*8)
> + * [ nv gpr save area ] 6*8
> + * [ addl. nv gpr save area] (12*8) <--- Only in case of
> exception boundary
> * [ local_tmp_var ] 24
> * [ unused red zone ] 224
> *
> @@ -139,12 +141,9 @@ static int bpf_jit_stack_local(struct
> codegen_context *ctx)
> return STACK_FRAME_MIN_SIZE + ctx->stack_size;
> } else {
> /* Stack layout with redzone */
> - return -(BPF_PPC_TAILCALL
> - +BPF_PPC_STACK_SAVE
> - +(ctx->exception_boundary || ctx->exception_cb ?
> - BPF_PPC_EXC_STACK_SAVE : 0)
> - +BPF_PPC_STACK_LOCALS
> - );
> + return -(BPF_PPC_TAILCALL + BPF_PPC_STACK_SAVE +
> + (ctx->exception_boundary ? BPF_PPC_EXC_STACK_SAVE : 0) +
> + BPF_PPC_STACK_LOCALS);
> }
> }
>
> @@ -153,20 +152,27 @@ int bpf_jit_stack_tailcallinfo_offset(struct
> codegen_context *ctx)
> return bpf_jit_stack_local(ctx) + BPF_PPC_STACK_LOCALS +
> BPF_PPC_STACK_SAVE;
> }
>
> -static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
> +static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg,
> bool use_redzone)
> {
> int min_valid_nvreg = BPF_PPC_NVR_MIN;
> /* Default frame size for all cases except exception boundary */
> int frame_nvr_size = BPF_PPC_STACKFRAME;
>
> + /*
> + * When use_redzone is true, return offset of the NVR on redzone
> + * Else, return offset based on whether stack is setup or not.
> + */
> + if (!use_redzone)
> + use_redzone = bpf_has_stack_frame(ctx);
> +
> /* Consider all nv regs for handling exceptions */
> - if (ctx->exception_boundary || ctx->exception_cb) {
> + if (ctx->exception_boundary) {
> min_valid_nvreg = _R14;
> frame_nvr_size = BPF_PPC_EXC_STACKFRAME;
> }
>
> if (reg >= min_valid_nvreg && reg < 32)
On a closely look, min_valid_nvreg has to be _R14 for exception_cb to
satisfy the above reg condition check...
> - return (bpf_has_stack_frame(ctx) ?
> + return (!use_redzone ?
> (frame_nvr_size + ctx->stack_size) : 0)
> - (8 * (32 - reg)) - BPF_PPC_TAILCALL;
>
> @@ -179,14 +185,12 @@ void bpf_jit_realloc_regs(struct codegen_context
> *ctx)
> }
>
> /*
> - * For exception boundary & exception_cb progs:
> - * return increased size to accommodate additional NVRs.
> + * For exception boundary program, return increased size to
> + * accommodate additional NVRs.
> */
> static int bpf_jit_stack_size(struct codegen_context *ctx)
> {
> - return ctx->exception_boundary || ctx->exception_cb ?
> - BPF_PPC_EXC_STACKFRAME :
> - BPF_PPC_STACKFRAME;
> + return ctx->exception_boundary ? BPF_PPC_EXC_STACKFRAME :
> BPF_PPC_STACKFRAME;
> }
>
> void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
> @@ -235,12 +239,30 @@ void bpf_jit_build_prologue(u32 *image, struct
> codegen_context *ctx)
> EMIT(PPC_RAW_STD(bpf_to_ppc(TMP_REG_1), _R1, -
> (BPF_PPC_TAILCALL)));
> }
>
> - if (bpf_has_stack_frame(ctx) && !ctx->exception_cb) {
> + if (ctx->exception_cb) {
> + /*
> + * Exception callback receives frame pointer of exception
> boundary program as
> + * third argument but reliable offset calculation for NVRs is
> only possible
> + * on the redzone because stack_depth of exception boundary is
> not known
> + * while jitting the exception_cb program. So, rollback to an
> sp above
> + * exception boundary frame and restore the additional NVRs
> using redzone
> + * offset. The regular BPF JIT NVRs can be restored now, or in
> the epilogue
> + * of exception_cb like any other bpf prog by reusing the BPF
> JIT NVRs part
> + * of exception_boundary stack frame. The latter is preferred
> as exception
> + * cb may also clobber the BPF JIT NVRs.
> + */
> + EMIT(PPC_RAW_LD(_R1, _R5, 0));
> +
> + /*
> + * Recover additionally saved non volatile registers from stack
> + * frame of exception boundary program.
> + */
> + for (i = _R14; i <= _R25; i++)
> + EMIT(PPC_RAW_LD(i, _R1, bpf_jit_stack_offsetof(ctx, i,
> true)));
> + }
> +
> + if (bpf_has_stack_frame(ctx)) {
> /*
> - * exception_cb uses boundary frame after stack walk.
> - * It can simply use redzone, this optimization reduces
> - * stack walk loop by one level.
> - *
> * We need a stack frame, but we don't necessarily need to
> * save/restore LR unless we call other functions
> */
> @@ -257,11 +279,11 @@ void bpf_jit_build_prologue(u32 *image, struct
> codegen_context *ctx)
> * Program acting as exception boundary pushes R14..R25 in
> addition to
> * BPF callee-saved non volatile registers. Exception callback uses
> * the boundary program's stack frame, recover additionally saved
> - * registers in epilogue of exception callback.
> + * registers in prologue of exception callback.
> */
> if (ctx->exception_boundary) {
> for (i = _R14; i <= _R25; i++)
> - EMIT(PPC_RAW_STD(i, _R1, bpf_jit_stack_offsetof(ctx, i)));
> + EMIT(PPC_RAW_STD(i, _R1, bpf_jit_stack_offsetof(ctx, i,
> false)));
> }
>
> if (!ctx->exception_cb) {
> @@ -273,17 +295,11 @@ void bpf_jit_build_prologue(u32 *image, struct
> codegen_context *ctx)
> for (i = BPF_REG_6; i <= BPF_REG_10; i++)
> if (ctx->exception_boundary || bpf_is_seen_register(ctx,
> bpf_to_ppc(i)))
> EMIT(PPC_RAW_STD(bpf_to_ppc(i), _R1,
> - bpf_jit_stack_offsetof(ctx, bpf_to_ppc(i))));
> + bpf_jit_stack_offsetof(ctx, bpf_to_ppc(i), false)));
>
> if (ctx->exception_boundary || ctx->arena_vm_start)
> EMIT(PPC_RAW_STD(bpf_to_ppc(ARENA_VM_START), _R1,
> - bpf_jit_stack_offsetof(ctx,
> bpf_to_ppc(ARENA_VM_START))));
> - } else {
> - /*
> - * Exception callback receives Frame Pointer of boundary
> - * program(main prog) as third arg
> - */
> - EMIT(PPC_RAW_MR(_R1, _R5));
> + bpf_jit_stack_offsetof(ctx,
> bpf_to_ppc(ARENA_VM_START), false)));
> }
>
> /*
> @@ -305,20 +321,12 @@ static void bpf_jit_emit_common_epilogue(u32
> *image, struct codegen_context *ctx
> /* Restore NVRs */
> for (i = BPF_REG_6; i <= BPF_REG_10; i++)
> if (ctx->exception_cb || bpf_is_seen_register(ctx,
> bpf_to_ppc(i)))
> - EMIT(PPC_RAW_LD(bpf_to_ppc(i), _R1,
> bpf_jit_stack_offsetof(ctx, bpf_to_ppc(i))));
> + EMIT(PPC_RAW_LD(bpf_to_ppc(i), _R1,
> + bpf_jit_stack_offsetof(ctx, bpf_to_ppc(i), false)));
>
> if (ctx->exception_cb || ctx->arena_vm_start)
> EMIT(PPC_RAW_LD(bpf_to_ppc(ARENA_VM_START), _R1,
> - bpf_jit_stack_offsetof(ctx, bpf_to_ppc(ARENA_VM_START))));
> -
> - if (ctx->exception_cb) {
> - /*
> - * Recover additionally saved non volatile registers from stack
> - * frame of exception boundary program.
> - */
> - for (i = _R14; i <= _R25; i++)
> - EMIT(PPC_RAW_LD(i, _R1, bpf_jit_stack_offsetof(ctx, i)));
> - }
> + bpf_jit_stack_offsetof(ctx, bpf_to_ppc(ARENA_VM_START),
> false)));
>
> /* Tear down our stack frame */
> if (bpf_has_stack_frame(ctx)) {
Also, if all exception selftests are passing, it is likely that neither
exception boundary nor exception callback is using BPF stack.
Need to have a test case that tests different BPF stack size for
exception boundary and exception callback..
- Hari
Powered by blists - more mailing lists