* [PATCH bpf-next v2 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test
2021-04-16 20:47 [PATCH bpf-next v2 0/3] bpf: refine retval for bpf_get_task_stack helper Dave Marchevsky
2021-04-16 20:47 ` [PATCH bpf-next v2 1/3] " Dave Marchevsky
@ 2021-04-16 20:47 ` Dave Marchevsky
2021-04-16 20:47 ` [PATCH bpf-next v2 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog Dave Marchevsky
2021-04-20 2:20 ` [PATCH bpf-next v2 0/3] bpf: refine retval for bpf_get_task_stack helper Alexei Starovoitov
3 siblings, 0 replies; 5+ messages in thread
From: Dave Marchevsky @ 2021-04-16 20:47 UTC (permalink / raw)
To: bpf
Cc: kernel-team, Alexei Starovoitov, Daniel Borkmann, Song Liu,
Yonghong Song, Dave Marchevsky
Add a bpf_iter test which feeds bpf_get_task_stack's return value into
seq_write after confirming it's positive. No attempt to bound the value
from above is made.
Load will fail if verifier does not refine retval range based on
buf sz input to bpf_get_task_stack.
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
.../selftests/bpf/verifier/bpf_get_stack.c | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/tools/testing/selftests/bpf/verifier/bpf_get_stack.c b/tools/testing/selftests/bpf/verifier/bpf_get_stack.c
index 69b048cf46d9..3e024c891178 100644
--- a/tools/testing/selftests/bpf/verifier/bpf_get_stack.c
+++ b/tools/testing/selftests/bpf/verifier/bpf_get_stack.c
@@ -42,3 +42,46 @@
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
},
+{
+ "bpf_get_task_stack return R0 range is refined",
+ .insns = {
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_6, 0), // ctx->meta->seq
+ BPF_LDX_MEM(BPF_DW, BPF_REG_7, BPF_REG_1, 8), // ctx->task
+ BPF_LD_MAP_FD(BPF_REG_1, 0), // fixup_map_array_48b
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_9, BPF_REG_0), // keep buf for seq_write
+ BPF_MOV64_IMM(BPF_REG_3, 48),
+ BPF_MOV64_IMM(BPF_REG_4, 0),
+ BPF_EMIT_CALL(BPF_FUNC_get_task_stack),
+ BPF_JMP_IMM(BPF_JSGT, BPF_REG_0, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_9),
+ BPF_MOV64_REG(BPF_REG_3, BPF_REG_0),
+ BPF_EMIT_CALL(BPF_FUNC_seq_write),
+
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_TRACING,
+ .expected_attach_type = BPF_TRACE_ITER,
+ .kfunc = "task",
+ .runs = -1, // Don't run, just load
+ .fixup_map_array_48b = { 3 },
+},
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH bpf-next v2 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog
2021-04-16 20:47 [PATCH bpf-next v2 0/3] bpf: refine retval for bpf_get_task_stack helper Dave Marchevsky
2021-04-16 20:47 ` [PATCH bpf-next v2 1/3] " Dave Marchevsky
2021-04-16 20:47 ` [PATCH bpf-next v2 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test Dave Marchevsky
@ 2021-04-16 20:47 ` Dave Marchevsky
2021-04-20 2:20 ` [PATCH bpf-next v2 0/3] bpf: refine retval for bpf_get_task_stack helper Alexei Starovoitov
3 siblings, 0 replies; 5+ messages in thread
From: Dave Marchevsky @ 2021-04-16 20:47 UTC (permalink / raw)
To: bpf
Cc: kernel-team, Alexei Starovoitov, Daniel Borkmann, Song Liu,
Yonghong Song, Dave Marchevsky
Add a libbpf test prog which feeds bpf_get_task_stack's return value
into seq_write after confirming it's positive. No attempt to bound the
value from above is made.
Load will fail if verifier does not refine retval range based on buf sz
input to bpf_get_task_stack.
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
.../selftests/bpf/prog_tests/bpf_iter.c | 1 +
.../selftests/bpf/progs/bpf_iter_task_stack.c | 27 +++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 74c45d557a2b..2d3590cfb5e1 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -147,6 +147,7 @@ static void test_task_stack(void)
return;
do_dummy_read(skel->progs.dump_task_stack);
+ do_dummy_read(skel->progs.get_task_user_stacks);
bpf_iter_task_stack__destroy(skel);
}
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_task_stack.c b/tools/testing/selftests/bpf/progs/bpf_iter_task_stack.c
index 50e59a2e142e..43c36f5f7649 100644
--- a/tools/testing/selftests/bpf/progs/bpf_iter_task_stack.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_task_stack.c
@@ -35,3 +35,30 @@ int dump_task_stack(struct bpf_iter__task *ctx)
return 0;
}
+
+SEC("iter/task")
+int get_task_user_stacks(struct bpf_iter__task *ctx)
+{
+ struct seq_file *seq = ctx->meta->seq;
+ struct task_struct *task = ctx->task;
+ uint64_t buf_sz = 0;
+ int64_t res;
+
+ if (task == (void *)0)
+ return 0;
+
+ res = bpf_get_task_stack(task, entries,
+ MAX_STACK_TRACE_DEPTH * SIZE_OF_ULONG, BPF_F_USER_STACK);
+ if (res <= 0)
+ return 0;
+
+ buf_sz += res;
+
+ /* If the verifier doesn't refine bpf_get_task_stack res, and instead
+ * assumes res is entirely unknown, this program will fail to load as
+ * the verifier will believe that max buf_sz value allows reading
+ * past the end of entries in bpf_seq_write call
+ */
+ bpf_seq_write(seq, &entries, buf_sz);
+ return 0;
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread