* [PATCH bpf-next 0/3] bpf: refine retval for bpf_get_task_stack helper
@ 2021-04-16 2:55 Dave Marchevsky
2021-04-16 2:55 ` [PATCH bpf-next 1/3] " Dave Marchevsky
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Dave Marchevsky @ 2021-04-16 2:55 UTC (permalink / raw)
To: bpf
Cc: kernel-team, Alexei Starovoitov, Daniel Borkmann, Song Liu,
Yonghong Song, Dave Marchevsky
Similarly to the bpf_get_stack helper, bpf_get_task_stack's return value
can be more tightly bound by the verifier - it's the number of bytes
written to a user-supplied buffer, or a negative error value. Currently
the verifier believes bpf_task_get_stack's retval bounds to be unknown,
requiring extraneous bounds checking to remedy.
Adding it to do_refine_retval_range fixes the issue, as evidenced by
new selftests which fail to load if retval bounds are not refined.
Dave Marchevsky (3):
bpf: refine retval for bpf_get_task_stack helper
bpf/selftests: add bpf_get_task_stack retval bounds verifier test
bpf/selftests: add bpf_get_task_stack retval bounds test_prog
kernel/bpf/verifier.c | 1 +
.../selftests/bpf/prog_tests/bpf_iter.c | 1 +
.../selftests/bpf/progs/bpf_iter_task_stack.c | 22 ++++++++++
.../selftests/bpf/verifier/bpf_get_stack.c | 43 +++++++++++++++++++
4 files changed, 67 insertions(+)
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next 1/3] bpf: refine retval for bpf_get_task_stack helper
2021-04-16 2:55 [PATCH bpf-next 0/3] bpf: refine retval for bpf_get_task_stack helper Dave Marchevsky
@ 2021-04-16 2:55 ` Dave Marchevsky
2021-04-16 16:53 ` Song Liu
2021-04-16 2:55 ` [PATCH bpf-next 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test Dave Marchevsky
2021-04-16 2:55 ` [PATCH bpf-next 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog Dave Marchevsky
2 siblings, 1 reply; 7+ messages in thread
From: Dave Marchevsky @ 2021-04-16 2:55 UTC (permalink / raw)
To: bpf
Cc: kernel-team, Alexei Starovoitov, Daniel Borkmann, Song Liu,
Yonghong Song, Dave Marchevsky
Verifier can constrain the min/max bounds of bpf_get_task_stack's return
value more tightly than the default tnum_unknown. Like bpf_get_stack,
return value is num bytes written into a caller-supplied buf, or error,
so do_refine_retval_range will work.
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 852541a435ef..348e97f77003 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5767,6 +5767,7 @@ static void do_refine_retval_range(struct bpf_reg_state *regs, int ret_type,
if (ret_type != RET_INTEGER ||
(func_id != BPF_FUNC_get_stack &&
+ func_id != BPF_FUNC_get_task_stack &&
func_id != BPF_FUNC_probe_read_str &&
func_id != BPF_FUNC_probe_read_kernel_str &&
func_id != BPF_FUNC_probe_read_user_str))
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test
2021-04-16 2:55 [PATCH bpf-next 0/3] bpf: refine retval for bpf_get_task_stack helper Dave Marchevsky
2021-04-16 2:55 ` [PATCH bpf-next 1/3] " Dave Marchevsky
@ 2021-04-16 2:55 ` Dave Marchevsky
2021-04-16 17:09 ` Song Liu
2021-04-16 2:55 ` [PATCH bpf-next 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog Dave Marchevsky
2 siblings, 1 reply; 7+ messages in thread
From: Dave Marchevsky @ 2021-04-16 2:55 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>
---
.../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..0e8299c043d4 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] 7+ messages in thread
* [PATCH bpf-next 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog
2021-04-16 2:55 [PATCH bpf-next 0/3] bpf: refine retval for bpf_get_task_stack helper Dave Marchevsky
2021-04-16 2:55 ` [PATCH bpf-next 1/3] " Dave Marchevsky
2021-04-16 2:55 ` [PATCH bpf-next 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test Dave Marchevsky
@ 2021-04-16 2:55 ` Dave Marchevsky
2021-04-16 17:06 ` Song Liu
2 siblings, 1 reply; 7+ messages in thread
From: Dave Marchevsky @ 2021-04-16 2:55 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>
---
.../selftests/bpf/prog_tests/bpf_iter.c | 1 +
.../selftests/bpf/progs/bpf_iter_task_stack.c | 22 +++++++++++++++++++
2 files changed, 23 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..c60048ed226f 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,25 @@ 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;
+
+ bpf_seq_write(seq, &entries, buf_sz);
+ return 0;
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 1/3] bpf: refine retval for bpf_get_task_stack helper
2021-04-16 2:55 ` [PATCH bpf-next 1/3] " Dave Marchevsky
@ 2021-04-16 16:53 ` Song Liu
0 siblings, 0 replies; 7+ messages in thread
From: Song Liu @ 2021-04-16 16:53 UTC (permalink / raw)
To: Dave Marchevsky
Cc: bpf, Kernel Team, Alexei Starovoitov, Daniel Borkmann, Yonghong Song
> On Apr 15, 2021, at 7:55 PM, Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> Verifier can constrain the min/max bounds of bpf_get_task_stack's return
> value more tightly than the default tnum_unknown. Like bpf_get_stack,
> return value is num bytes written into a caller-supplied buf, or error,
> so do_refine_retval_range will work.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> kernel/bpf/verifier.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 852541a435ef..348e97f77003 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5767,6 +5767,7 @@ static void do_refine_retval_range(struct bpf_reg_state *regs, int ret_type,
>
> if (ret_type != RET_INTEGER ||
> (func_id != BPF_FUNC_get_stack &&
> + func_id != BPF_FUNC_get_task_stack &&
> func_id != BPF_FUNC_probe_read_str &&
> func_id != BPF_FUNC_probe_read_kernel_str &&
> func_id != BPF_FUNC_probe_read_user_str))
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog
2021-04-16 2:55 ` [PATCH bpf-next 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog Dave Marchevsky
@ 2021-04-16 17:06 ` Song Liu
0 siblings, 0 replies; 7+ messages in thread
From: Song Liu @ 2021-04-16 17:06 UTC (permalink / raw)
To: Dave Marchevsky
Cc: bpf, Kernel Team, Alexei Starovoitov, Daniel Borkmann, Yonghong Song
> On Apr 15, 2021, at 7:55 PM, Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> 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>
With one nit below.
> ---
> .../selftests/bpf/prog_tests/bpf_iter.c | 1 +
> .../selftests/bpf/progs/bpf_iter_task_stack.c | 22 +++++++++++++++++++
> 2 files changed, 23 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..c60048ed226f 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,25 @@ 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;
> +
nit: When the test fails because of missing the verifier change, a comment here
would help the debug effort.
> + bpf_seq_write(seq, &entries, buf_sz);
> + return 0;
> +}
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test
2021-04-16 2:55 ` [PATCH bpf-next 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test Dave Marchevsky
@ 2021-04-16 17:09 ` Song Liu
0 siblings, 0 replies; 7+ messages in thread
From: Song Liu @ 2021-04-16 17:09 UTC (permalink / raw)
To: Dave Marchevsky
Cc: bpf, Kernel Team, Alexei Starovoitov, Daniel Borkmann, Yonghong Song
> On Apr 15, 2021, at 7:55 PM, Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> 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..0e8299c043d4 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 [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-16 17:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 2:55 [PATCH bpf-next 0/3] bpf: refine retval for bpf_get_task_stack helper Dave Marchevsky
2021-04-16 2:55 ` [PATCH bpf-next 1/3] " Dave Marchevsky
2021-04-16 16:53 ` Song Liu
2021-04-16 2:55 ` [PATCH bpf-next 2/3] bpf/selftests: add bpf_get_task_stack retval bounds verifier test Dave Marchevsky
2021-04-16 17:09 ` Song Liu
2021-04-16 2:55 ` [PATCH bpf-next 3/3] bpf/selftests: add bpf_get_task_stack retval bounds test_prog Dave Marchevsky
2021-04-16 17:06 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).