| Message ID | 20171222181235.158636-1-jannh@google.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: selftest for late caller stack size increase | expand |
On Fri, Dec 22, 2017 at 07:12:35PM +0100, Jann Horn wrote: > This checks that it is not possible to bypass the total stack size check in > update_stack_depth() by calling a function that uses a large amount of > stack memory *before* using a large amount of stack memory in the caller. > > Currently, the first added testcase causes a rejection as expected, but > the second testcase is (AFAICS incorrectly) accepted: > > [...] > #483/p calls: stack overflow using two frames (post-call access) FAIL > Unexpected success to load! > 0: (85) call pc+2 > caller: > R10=fp0,call_-1 > callee: > frame1: R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0 > 3: (72) *(u8 *)(r10 -300) = 0 > 4: (b7) r0 = 0 > 5: (95) exit > returning from callee: > frame1: R0_w=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0 > to caller at 1: > R0_w=inv0 R10=fp0,call_-1 > > from 5 to 1: R0=inv0 R10=fp0,call_-1 > 1: (72) *(u8 *)(r10 -300) = 0 > 2: (95) exit > processed 6 insns, stack depth 300+300 got it. thanks for the test! working on a fix.
On 12/22/2017 07:12 PM, Jann Horn wrote: > This checks that it is not possible to bypass the total stack size check in > update_stack_depth() by calling a function that uses a large amount of > stack memory *before* using a large amount of stack memory in the caller. > > Currently, the first added testcase causes a rejection as expected, but > the second testcase is (AFAICS incorrectly) accepted: > > [...] > #483/p calls: stack overflow using two frames (post-call access) FAIL > Unexpected success to load! > 0: (85) call pc+2 > caller: > R10=fp0,call_-1 > callee: > frame1: R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0 > 3: (72) *(u8 *)(r10 -300) = 0 > 4: (b7) r0 = 0 > 5: (95) exit > returning from callee: > frame1: R0_w=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0 > to caller at 1: > R0_w=inv0 R10=fp0,call_-1 > > from 5 to 1: R0=inv0 R10=fp0,call_-1 > 1: (72) *(u8 *)(r10 -300) = 0 > 2: (95) exit > processed 6 insns, stack depth 300+300 > [...] > Summary: 704 PASSED, 1 FAILED > > AFAICS the JIT-generated code for the second testcase shows that this > really causes the stack pointer to be decremented by 300+300: > > first function: > 00000000 55 push rbp > 00000001 4889E5 mov rbp,rsp > 00000004 4881EC58010000 sub rsp,0x158 > 0000000B 4883ED28 sub rbp,byte +0x28 > [...] > 00000025 E89AB3AFE5 call 0xffffffffe5afb3c4 > 0000002A C685D4FEFFFF00 mov byte [rbp-0x12c],0x0 > [...] > 00000041 4883C528 add rbp,byte +0x28 > 00000045 C9 leave > 00000046 C3 ret > > second function: > 00000000 55 push rbp > 00000001 4889E5 mov rbp,rsp > 00000004 4881EC58010000 sub rsp,0x158 > 0000000B 4883ED28 sub rbp,byte +0x28 > [...] > 00000025 C685D4FEFFFF00 mov byte [rbp-0x12c],0x0 > [...] > 0000003E 4883C528 add rbp,byte +0x28 > 00000042 C9 leave > 00000043 C3 ret > > Signed-off-by: Jann Horn <jannh@google.com> Applied to bpf-next, thanks a lot Jann!
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 3bacff0d6f91..71fb0be81b78 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -8729,6 +8729,40 @@ static struct bpf_test tests[] = { .prog_type = BPF_PROG_TYPE_XDP, .result = ACCEPT, }, + { + "calls: stack overflow using two frames (pre-call access)", + .insns = { + /* prog 1 */ + BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0), + BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 1), + BPF_EXIT_INSN(), + + /* prog 2 */ + BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_XDP, + .errstr = "combined stack size", + .result = REJECT, + }, + { + "calls: stack overflow using two frames (post-call access)", + .insns = { + /* prog 1 */ + BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 2), + BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0), + BPF_EXIT_INSN(), + + /* prog 2 */ + BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_XDP, + .errstr = "combined stack size", + .result = REJECT, + }, { "calls: spill into caller stack frame", .insns = {
This checks that it is not possible to bypass the total stack size check in update_stack_depth() by calling a function that uses a large amount of stack memory *before* using a large amount of stack memory in the caller. Currently, the first added testcase causes a rejection as expected, but the second testcase is (AFAICS incorrectly) accepted: [...] #483/p calls: stack overflow using two frames (post-call access) FAIL Unexpected success to load! 0: (85) call pc+2 caller: R10=fp0,call_-1 callee: frame1: R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0 3: (72) *(u8 *)(r10 -300) = 0 4: (b7) r0 = 0 5: (95) exit returning from callee: frame1: R0_w=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0 to caller at 1: R0_w=inv0 R10=fp0,call_-1 from 5 to 1: R0=inv0 R10=fp0,call_-1 1: (72) *(u8 *)(r10 -300) = 0 2: (95) exit processed 6 insns, stack depth 300+300 [...] Summary: 704 PASSED, 1 FAILED AFAICS the JIT-generated code for the second testcase shows that this really causes the stack pointer to be decremented by 300+300: first function: 00000000 55 push rbp 00000001 4889E5 mov rbp,rsp 00000004 4881EC58010000 sub rsp,0x158 0000000B 4883ED28 sub rbp,byte +0x28 [...] 00000025 E89AB3AFE5 call 0xffffffffe5afb3c4 0000002A C685D4FEFFFF00 mov byte [rbp-0x12c],0x0 [...] 00000041 4883C528 add rbp,byte +0x28 00000045 C9 leave 00000046 C3 ret second function: 00000000 55 push rbp 00000001 4889E5 mov rbp,rsp 00000004 4881EC58010000 sub rsp,0x158 0000000B 4883ED28 sub rbp,byte +0x28 [...] 00000025 C685D4FEFFFF00 mov byte [rbp-0x12c],0x0 [...] 0000003E 4883C528 add rbp,byte +0x28 00000042 C9 leave 00000043 C3 ret Signed-off-by: Jann Horn <jannh@google.com> --- tools/testing/selftests/bpf/test_verifier.c | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)