From: Abhishek Dubey <adubey@linux.ibm.com>
The bpf_throw() function never returns, if it has clobbered
any callee-saved register, those will remain clobbered. The
prologue must take care of saving all callee-saved registers
in the frame of exception boundary program. Later these
additional non volatile registers R14-R25 along with other
NVRs are restored back in the epilogue of exception callback.
To achieve above objective, the frame size is determined
dynamically to accommodate additional non volatile registers
in exception boundary's frame.
For non-exception boundary program, the frame size remains
optimal. The additional instructions to save & restore r14-r25
registers are emitted only during exception boundary and
exception callback program respectively.
Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
---
arch/powerpc/net/bpf_jit_comp64.c | 89 ++++++++++++++++++++++++++++---
1 file changed, 81 insertions(+), 8 deletions(-)
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
@@ -32,21 +32,37 @@
*
* [ prev sp ] <-------------
* [ tail_call_info ] 8 |
- * [ nv gpr save area ] 6*8 |
+ * [ nv gpr save area ] 6*8 + (12*8) |
* [ local_tmp_var ] 24 |
* fp (r31) --> [ ebpf stack space ] upto 512 |
* [ frame header ] 32/112 |
* sp (r1) ---> [ stack pointer ] --------------
+ *
+ * Additional (12*8) in 'nv gpr save area' only in case of
+ * exception boundary.
*/
/* for bpf JIT code internal usage */
#define BPF_PPC_STACK_LOCALS 24
+/*
+ * for additional non volatile registers(r14-r25) to be saved
+ * at exception boundary
+ */
+#define BPF_PPC_EXC_STACK_SAVE (12*8)
+
/* stack frame excluding BPF stack, ensure this is quadword aligned */
#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + \
BPF_PPC_STACK_LOCALS + \
BPF_PPC_STACK_SAVE + \
BPF_PPC_TAILCALL)
+/*
+ * same as BPF_PPC_STACKFRAME with save area for additional
+ * non volatile registers saved at exception boundary.
+ * This is quad-word aligned.
+ */
+#define BPF_PPC_EXC_STACKFRAME (BPF_PPC_STACKFRAME + BPF_PPC_EXC_STACK_SAVE)
+
/* BPF register usage */
#define TMP_REG_1 (MAX_BPF_JIT_REG + 0)
#define TMP_REG_2 (MAX_BPF_JIT_REG + 1)
@@ -88,10 +104,16 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
* - we call other functions (kernel helpers), or
* - the bpf program uses its stack area
* The latter condition is deduced from the usage of BPF_REG_FP
+ *
+ * bpf_throw() leads to exception callback from a BPF (sub)program.
+ * The (sub)program is always marked as SEEN_FUNC, creating a stack
+ * frame. The exception callback uses the frame of the exception
+ * boundary, so the exception boundary program must have a frame.
*/
return ctx->seen & SEEN_FUNC ||
bpf_is_seen_register(ctx, bpf_to_ppc(BPF_REG_FP)) ||
- ctx->exception_cb;
+ ctx->exception_cb ||
+ ctx->exception_boundary;
}
/*
@@ -103,9 +125,12 @@ 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
+ * [ nv gpr save area ] 6*8 + (12*8)
* [ local_tmp_var ] 24
* [ unused red zone ] 224
+ *
+ * Additional (12*8) in 'nv gpr save area' only in case of
+ * exception boundary.
*/
static int bpf_jit_stack_local(struct codegen_context *ctx)
{
@@ -114,7 +139,12 @@ 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 + BPF_PPC_STACK_LOCALS);
+ return -(BPF_PPC_TAILCALL
+ +BPF_PPC_STACK_SAVE
+ +(ctx->exception_boundary || ctx->exception_cb ?
+ BPF_PPC_EXC_STACK_SAVE : 0)
+ +BPF_PPC_STACK_LOCALS
+ );
}
}
@@ -125,9 +155,19 @@ int bpf_jit_stack_tailcallinfo_offset(struct codegen_context *ctx)
static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
{
- if (reg >= BPF_PPC_NVR_MIN && reg < 32)
+ int min_valid_nvreg = BPF_PPC_NVR_MIN;
+ /* Default frame size for all cases except exception boundary */
+ int frame_nvr_size = BPF_PPC_STACKFRAME;
+
+ /* Consider all nv regs for handling exceptions */
+ if (ctx->exception_boundary || ctx->exception_cb) {
+ min_valid_nvreg = _R14;
+ frame_nvr_size = BPF_PPC_EXC_STACKFRAME;
+ }
+
+ if (reg >= min_valid_nvreg && reg < 32)
return (bpf_has_stack_frame(ctx) ?
- (BPF_PPC_STACKFRAME + ctx->stack_size) : 0)
+ (frame_nvr_size + ctx->stack_size) : 0)
- (8 * (32 - reg)) - BPF_PPC_TAILCALL;
pr_err("BPF JIT is asking about unknown registers");
@@ -138,6 +178,17 @@ void bpf_jit_realloc_regs(struct codegen_context *ctx)
{
}
+/*
+ * 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;
+}
+
void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
{
int i;
@@ -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)));
+ }
+
+ /*
+ * 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.
+ */
+ if (ctx->exception_boundary) {
+ for (i = _R14; i <= _R25; i++)
+ EMIT(PPC_RAW_STD(i, _R1, bpf_jit_stack_offsetof(ctx, i)));
}
if (!ctx->exception_cb) {
@@ -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));
+
if (ctx->seen & SEEN_FUNC || ctx->exception_cb) {
EMIT(PPC_RAW_LD(_R0, _R1, PPC_LR_STKOFF));
EMIT(PPC_RAW_MTLR(_R0));
--
2.48.1
> 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?
> +
> if (ctx->seen & SEEN_FUNC || ctx->exception_cb) {
> EMIT(PPC_RAW_LD(_R0, _R1, PPC_LR_STKOFF));
> EMIT(PPC_RAW_MTLR(_R0));
---
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/21311936056
On 24/01/26 1:53 pm, bot+bpf-ci@kernel.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?
>
>> +
>> if (ctx->seen & SEEN_FUNC || ctx->exception_cb) {
>> EMIT(PPC_RAW_LD(_R0, _R1, PPC_LR_STKOFF));
>> EMIT(PPC_RAW_MTLR(_R0));
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
I will post the fix for this issue as a follow-up.
Thanks
Hari
On 24/01/26 1:53 pm, bot+bpf-ci@kernel.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)
- 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)) {
- Hari
On 27/01/26 12:00 am, Hari Bathini wrote:
>
>
> On 24/01/26 1:53 pm, bot+bpf-ci@kernel.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
© 2016 - 2026 Red Hat, Inc.