Skip to content

Commit 2d347d1

Browse files
hbathinimaddy-kerneldev
authored andcommitted
powerpc64/bpf: remove BPF redzone protection in trampoline stack
Since bpf2bpf tailcall support is enabled for 64-bit powerpc with kernel commit 2ed2d8f ("powerpc64/bpf: Support tailcalls with subprogs"), 'tailcalls/tailcall_bpf2bpf_hierarchy_fexit' BPF selftest is triggering "corrupted stack end detected inside scheduler" with the config option CONFIG_SCHED_STACK_END_CHECK enabled. While reviewing the stack layout for BPF trampoline, observed that the dummy frame is trying to protect the redzone of BPF program. This is because tail call info and NVRs save area are in redzone at the time of tailcall as the current BPF program stack frame is teared down before the tailcall. But saving this redzone in the dummy frame of trampoline is unnecessary because of the follow reasons: 1) Firstly, trampoline can be attached to BPF entry/main program or subprog. But prologue part of the BPF entry/main program, where the trampoline attachpoint is, is skipped during tailcall. So, protecting the redzone does not arise when the trampoline is not even triggered in this scenario. 2) In case of subprog, the caller's stackframe is already setup and the subprog's stackframe is yet to be setup. So, nothing on the redzone to be protected. Also, using dummy frame in BPF trampoline, wastes critically scarce kernel stack space, especially in tailcall sequence, for marginal benefit in stack unwinding. So, drop setting up the dummy frame. Instead, save return address in bpf trampoline frame and use it as appropriate. Pruning this unnecessary stack usage mitigates the likelihood of stack overflow in scenarios where bpf2bpf tailcalls and fexit programs are mixed. Reported-by: Saket Kumar Bhaskar <[email protected]> Fixes: 2ed2d8f ("powerpc64/bpf: Support tailcalls with subprogs") Tested-by: Venkat Rao Bagalkote <[email protected]> Signed-off-by: Hari Bathini <[email protected]> Signed-off-by: Madhavan Srinivasan <[email protected]> Link: https://patch.msgid.link/[email protected]
1 parent 3727d6e commit 2d347d1

1 file changed

Lines changed: 33 additions & 56 deletions

File tree

arch/powerpc/net/bpf_jit_comp.c

Lines changed: 33 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -638,15 +638,10 @@ static int invoke_bpf_mod_ret(u32 *image, u32 *ro_image, struct codegen_context
638638
* for the traced function (BPF subprog/callee) to fetch it.
639639
*/
640640
static void bpf_trampoline_setup_tail_call_info(u32 *image, struct codegen_context *ctx,
641-
int func_frame_offset,
642-
int bpf_dummy_frame_size, int r4_off)
641+
int bpf_frame_size, int r4_off)
643642
{
644643
if (IS_ENABLED(CONFIG_PPC64)) {
645-
/*
646-
* func_frame_offset = ...(1)
647-
* bpf_dummy_frame_size + trampoline_frame_size
648-
*/
649-
EMIT(PPC_RAW_LD(_R4, _R1, func_frame_offset));
644+
EMIT(PPC_RAW_LD(_R4, _R1, bpf_frame_size));
650645
/* Refer to trampoline's Generated stack layout */
651646
EMIT(PPC_RAW_LD(_R3, _R4, -BPF_PPC_TAILCALL));
652647

@@ -657,29 +652,21 @@ static void bpf_trampoline_setup_tail_call_info(u32 *image, struct codegen_conte
657652
EMIT(PPC_RAW_CMPLWI(_R3, MAX_TAIL_CALL_CNT));
658653
PPC_BCC_CONST_SHORT(COND_GT, 8);
659654
EMIT(PPC_RAW_ADDI(_R3, _R4, -BPF_PPC_TAILCALL));
655+
660656
/*
661-
* From ...(1) above:
662-
* trampoline_frame_bottom = ...(2)
663-
* func_frame_offset - bpf_dummy_frame_size
664-
*
665-
* Using ...(2) derived above:
666-
* trampoline_tail_call_info_offset = ...(3)
667-
* trampoline_frame_bottom - BPF_PPC_TAILCALL
668-
*
669-
* From ...(3):
670-
* Use trampoline_tail_call_info_offset to write reference of main's
671-
* tail_call_info in trampoline frame.
657+
* Trampoline's tail_call_info is at the same offset, as that of
658+
* any bpf program, with reference to previous frame. Update the
659+
* address of main's tail_call_info in trampoline frame.
672660
*/
673-
EMIT(PPC_RAW_STL(_R3, _R1, (func_frame_offset - bpf_dummy_frame_size)
674-
- BPF_PPC_TAILCALL));
661+
EMIT(PPC_RAW_STL(_R3, _R1, bpf_frame_size - BPF_PPC_TAILCALL));
675662
} else {
676663
/* See bpf_jit_stack_offsetof() and BPF_PPC_TC */
677664
EMIT(PPC_RAW_LL(_R4, _R1, r4_off));
678665
}
679666
}
680667

681668
static void bpf_trampoline_restore_tail_call_cnt(u32 *image, struct codegen_context *ctx,
682-
int func_frame_offset, int r4_off)
669+
int bpf_frame_size, int r4_off)
683670
{
684671
if (IS_ENABLED(CONFIG_PPC32)) {
685672
/*
@@ -690,12 +677,12 @@ static void bpf_trampoline_restore_tail_call_cnt(u32 *image, struct codegen_cont
690677
}
691678
}
692679

693-
static void bpf_trampoline_save_args(u32 *image, struct codegen_context *ctx, int func_frame_offset,
694-
int nr_regs, int regs_off)
680+
static void bpf_trampoline_save_args(u32 *image, struct codegen_context *ctx,
681+
int bpf_frame_size, int nr_regs, int regs_off)
695682
{
696683
int param_save_area_offset;
697684

698-
param_save_area_offset = func_frame_offset; /* the two frames we alloted */
685+
param_save_area_offset = bpf_frame_size;
699686
param_save_area_offset += STACK_FRAME_MIN_SIZE; /* param save area is past frame header */
700687

701688
for (int i = 0; i < nr_regs; i++) {
@@ -718,11 +705,11 @@ static void bpf_trampoline_restore_args_regs(u32 *image, struct codegen_context
718705

719706
/* Used when we call into the traced function. Replicate parameter save area */
720707
static void bpf_trampoline_restore_args_stack(u32 *image, struct codegen_context *ctx,
721-
int func_frame_offset, int nr_regs, int regs_off)
708+
int bpf_frame_size, int nr_regs, int regs_off)
722709
{
723710
int param_save_area_offset;
724711

725-
param_save_area_offset = func_frame_offset; /* the two frames we alloted */
712+
param_save_area_offset = bpf_frame_size;
726713
param_save_area_offset += STACK_FRAME_MIN_SIZE; /* param save area is past frame header */
727714

728715
for (int i = 8; i < nr_regs; i++) {
@@ -739,10 +726,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
739726
void *func_addr)
740727
{
741728
int regs_off, nregs_off, ip_off, run_ctx_off, retval_off, nvr_off, alt_lr_off, r4_off = 0;
742-
int i, ret, nr_regs, bpf_frame_size = 0, bpf_dummy_frame_size = 0, func_frame_offset;
743729
struct bpf_tramp_links *fmod_ret = &tlinks[BPF_TRAMP_MODIFY_RETURN];
744730
struct bpf_tramp_links *fentry = &tlinks[BPF_TRAMP_FENTRY];
745731
struct bpf_tramp_links *fexit = &tlinks[BPF_TRAMP_FEXIT];
732+
int i, ret, nr_regs, retaddr_off, bpf_frame_size = 0;
746733
struct codegen_context codegen_ctx, *ctx;
747734
u32 *image = (u32 *)rw_image;
748735
ppc_inst_t branch_insn;
@@ -768,16 +755,11 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
768755
* Generated stack layout:
769756
*
770757
* func prev back chain [ back chain ]
771-
* [ ]
772-
* bpf prog redzone/tailcallcnt [ ... ] 64 bytes (64-bit powerpc)
773-
* [ ] --
774-
* LR save area [ r0 save (64-bit) ] | header
775-
* [ r0 save (32-bit) ] |
776-
* dummy frame for unwind [ back chain 1 ] --
777758
* [ tail_call_info ] optional - 64-bit powerpc
778759
* [ padding ] align stack frame
779760
* r4_off [ r4 (tailcallcnt) ] optional - 32-bit powerpc
780761
* alt_lr_off [ real lr (ool stub)] optional - actual lr
762+
* retaddr_off [ return address ]
781763
* [ r26 ]
782764
* nvr_off [ r25 ] nvr save area
783765
* retval_off [ return value ]
@@ -841,6 +823,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
841823
nvr_off = bpf_frame_size;
842824
bpf_frame_size += 2 * SZL;
843825

826+
/* Save area for return address */
827+
retaddr_off = bpf_frame_size;
828+
bpf_frame_size += SZL;
829+
844830
/* Optional save area for actual LR in case of ool ftrace */
845831
if (IS_ENABLED(CONFIG_PPC_FTRACE_OUT_OF_LINE)) {
846832
alt_lr_off = bpf_frame_size;
@@ -867,16 +853,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
867853
/* Padding to align stack frame, if any */
868854
bpf_frame_size = round_up(bpf_frame_size, SZL * 2);
869855

870-
/* Dummy frame size for proper unwind - includes 64-bytes red zone for 64-bit powerpc */
871-
bpf_dummy_frame_size = STACK_FRAME_MIN_SIZE + 64;
872-
873-
/* Offset to the traced function's stack frame */
874-
func_frame_offset = bpf_dummy_frame_size + bpf_frame_size;
875-
876-
/* Create dummy frame for unwind, store original return value */
856+
/* Store original return value */
877857
EMIT(PPC_RAW_STL(_R0, _R1, PPC_LR_STKOFF));
878-
/* Protect red zone where tail call count goes */
879-
EMIT(PPC_RAW_STLU(_R1, _R1, -bpf_dummy_frame_size));
880858

881859
/* Create our stack frame */
882860
EMIT(PPC_RAW_STLU(_R1, _R1, -bpf_frame_size));
@@ -891,14 +869,14 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
891869
if (IS_ENABLED(CONFIG_PPC32) && nr_regs < 2)
892870
EMIT(PPC_RAW_STL(_R4, _R1, r4_off));
893871

894-
bpf_trampoline_save_args(image, ctx, func_frame_offset, nr_regs, regs_off);
872+
bpf_trampoline_save_args(image, ctx, bpf_frame_size, nr_regs, regs_off);
895873

896874
/* Save our LR/return address */
897875
EMIT(PPC_RAW_MFLR(_R3));
898876
if (IS_ENABLED(CONFIG_PPC_FTRACE_OUT_OF_LINE))
899877
EMIT(PPC_RAW_STL(_R3, _R1, alt_lr_off));
900878
else
901-
EMIT(PPC_RAW_STL(_R3, _R1, bpf_frame_size + PPC_LR_STKOFF));
879+
EMIT(PPC_RAW_STL(_R3, _R1, retaddr_off));
902880

903881
/*
904882
* Derive IP address of the traced function.
@@ -925,9 +903,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
925903
EMIT(PPC_RAW_STL(_R3, _R1, ip_off));
926904

927905
if (IS_ENABLED(CONFIG_PPC_FTRACE_OUT_OF_LINE)) {
928-
/* Fake our LR for unwind */
906+
/* Fake our LR for BPF_TRAMP_F_CALL_ORIG case */
929907
EMIT(PPC_RAW_ADDI(_R3, _R3, 4));
930-
EMIT(PPC_RAW_STL(_R3, _R1, bpf_frame_size + PPC_LR_STKOFF));
908+
EMIT(PPC_RAW_STL(_R3, _R1, retaddr_off));
931909
}
932910

933911
/* Save function arg count -- see bpf_get_func_arg_cnt() */
@@ -966,20 +944,19 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
966944
/* Call the traced function */
967945
if (flags & BPF_TRAMP_F_CALL_ORIG) {
968946
/*
969-
* The address in LR save area points to the correct point in the original function
947+
* retaddr on trampoline stack points to the correct point in the original function
970948
* with both PPC_FTRACE_OUT_OF_LINE as well as with traditional ftrace instruction
971949
* sequence
972950
*/
973-
EMIT(PPC_RAW_LL(_R3, _R1, bpf_frame_size + PPC_LR_STKOFF));
951+
EMIT(PPC_RAW_LL(_R3, _R1, retaddr_off));
974952
EMIT(PPC_RAW_MTCTR(_R3));
975953

976954
/* Replicate tail_call_cnt before calling the original BPF prog */
977955
if (flags & BPF_TRAMP_F_TAIL_CALL_CTX)
978-
bpf_trampoline_setup_tail_call_info(image, ctx, func_frame_offset,
979-
bpf_dummy_frame_size, r4_off);
956+
bpf_trampoline_setup_tail_call_info(image, ctx, bpf_frame_size, r4_off);
980957

981958
/* Restore args */
982-
bpf_trampoline_restore_args_stack(image, ctx, func_frame_offset, nr_regs, regs_off);
959+
bpf_trampoline_restore_args_stack(image, ctx, bpf_frame_size, nr_regs, regs_off);
983960

984961
/* Restore TOC for 64-bit */
985962
if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && !IS_ENABLED(CONFIG_PPC_KERNEL_PCREL))
@@ -993,7 +970,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
993970

994971
/* Restore updated tail_call_cnt */
995972
if (flags & BPF_TRAMP_F_TAIL_CALL_CTX)
996-
bpf_trampoline_restore_tail_call_cnt(image, ctx, func_frame_offset, r4_off);
973+
bpf_trampoline_restore_tail_call_cnt(image, ctx, bpf_frame_size, r4_off);
997974

998975
/* Reserve space to patch branch instruction to skip fexit progs */
999976
if (ro_image) /* image is NULL for dummy pass */
@@ -1045,21 +1022,21 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
10451022
EMIT(PPC_RAW_LD(_R2, _R1, 24));
10461023
if (flags & BPF_TRAMP_F_SKIP_FRAME) {
10471024
/* Skip the traced function and return to parent */
1048-
EMIT(PPC_RAW_ADDI(_R1, _R1, func_frame_offset));
1025+
EMIT(PPC_RAW_ADDI(_R1, _R1, bpf_frame_size));
10491026
EMIT(PPC_RAW_LL(_R0, _R1, PPC_LR_STKOFF));
10501027
EMIT(PPC_RAW_MTLR(_R0));
10511028
EMIT(PPC_RAW_BLR());
10521029
} else {
10531030
if (IS_ENABLED(CONFIG_PPC_FTRACE_OUT_OF_LINE)) {
10541031
EMIT(PPC_RAW_LL(_R0, _R1, alt_lr_off));
10551032
EMIT(PPC_RAW_MTLR(_R0));
1056-
EMIT(PPC_RAW_ADDI(_R1, _R1, func_frame_offset));
1033+
EMIT(PPC_RAW_ADDI(_R1, _R1, bpf_frame_size));
10571034
EMIT(PPC_RAW_LL(_R0, _R1, PPC_LR_STKOFF));
10581035
EMIT(PPC_RAW_BLR());
10591036
} else {
1060-
EMIT(PPC_RAW_LL(_R0, _R1, bpf_frame_size + PPC_LR_STKOFF));
1037+
EMIT(PPC_RAW_LL(_R0, _R1, retaddr_off));
10611038
EMIT(PPC_RAW_MTCTR(_R0));
1062-
EMIT(PPC_RAW_ADDI(_R1, _R1, func_frame_offset));
1039+
EMIT(PPC_RAW_ADDI(_R1, _R1, bpf_frame_size));
10631040
EMIT(PPC_RAW_LL(_R0, _R1, PPC_LR_STKOFF));
10641041
EMIT(PPC_RAW_MTLR(_R0));
10651042
EMIT(PPC_RAW_BCTR());

0 commit comments

Comments
 (0)