bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access
@ 2022-02-20  2:31 Kumar Kartikeya Dwivedi
  2022-02-22  2:23 ` Song Liu
  2022-02-22  4:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-02-20  2:31 UTC (permalink / raw)
  To: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko

This test tries to pass a PTR_TO_BTF_ID_OR_NULL to the release function,
which would trigger a out of bounds access without the fix in commit
45ce4b4f9009 ("bpf: Fix crash due to out of bounds access into reg2btf_ids.")
but after the fix, it should only index using base_type(reg->type),
which should be less than __BPF_REG_TYPE_MAX, and also not permit any
type flags to be set for the reg->type.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index 829be2b9e08e..0a8ea60c2a80 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -96,6 +96,25 @@
 		{ "bpf_kfunc_call_test_mem_len_fail1", 2 },
 	},
 },
+{
+	"calls: trigger reg2btf_ids[reg->type] for reg->type > __BPF_REG_TYPE_MAX",
+	.insns = {
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+	BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = REJECT,
+	.errstr = "arg#0 pointer type STRUCT prog_test_ref_kfunc must point",
+	.fixup_kfunc_btf_id = {
+		{ "bpf_kfunc_call_test_acquire", 3 },
+		{ "bpf_kfunc_call_test_release", 5 },
+	},
+},
 {
 	"calls: basic sanity",
 	.insns = {
--
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access
  2022-02-20  2:31 [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access Kumar Kartikeya Dwivedi
@ 2022-02-22  2:23 ` Song Liu
  2022-02-22  2:35   ` Kumar Kartikeya Dwivedi
  2022-02-22  4:30 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: Song Liu @ 2022-02-22  2:23 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko

On Sat, Feb 19, 2022 at 6:31 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> This test tries to pass a PTR_TO_BTF_ID_OR_NULL to the release function,
> which would trigger a out of bounds access without the fix in commit
> 45ce4b4f9009 ("bpf: Fix crash due to out of bounds access into reg2btf_ids.")
> but after the fix, it should only index using base_type(reg->type),
> which should be less than __BPF_REG_TYPE_MAX, and also not permit any
> type flags to be set for the reg->type.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
> index 829be2b9e08e..0a8ea60c2a80 100644
> --- a/tools/testing/selftests/bpf/verifier/calls.c
> +++ b/tools/testing/selftests/bpf/verifier/calls.c
> @@ -96,6 +96,25 @@
>                 { "bpf_kfunc_call_test_mem_len_fail1", 2 },
>         },
>  },
> +{
> +       "calls: trigger reg2btf_ids[reg->type] for reg->type > __BPF_REG_TYPE_MAX",
> +       .insns = {
> +       BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
> +       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
> +       BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
> +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
> +       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
> +       BPF_EXIT_INSN(),
> +       },
> +       .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> +       .result = REJECT,
> +       .errstr = "arg#0 pointer type STRUCT prog_test_ref_kfunc must point",

Why do we stop errstr at "must point"?

> +       .fixup_kfunc_btf_id = {
> +               { "bpf_kfunc_call_test_acquire", 3 },
> +               { "bpf_kfunc_call_test_release", 5 },
> +       },
> +},
>  {
>         "calls: basic sanity",
>         .insns = {
> --
> 2.35.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access
  2022-02-22  2:23 ` Song Liu
@ 2022-02-22  2:35   ` Kumar Kartikeya Dwivedi
  0 siblings, 0 replies; 4+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-02-22  2:35 UTC (permalink / raw)
  To: Song Liu; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko

On Tue, Feb 22, 2022 at 07:53:33AM IST, Song Liu wrote:
> On Sat, Feb 19, 2022 at 6:31 PM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > This test tries to pass a PTR_TO_BTF_ID_OR_NULL to the release function,
> > which would trigger a out of bounds access without the fix in commit
> > 45ce4b4f9009 ("bpf: Fix crash due to out of bounds access into reg2btf_ids.")
> > but after the fix, it should only index using base_type(reg->type),
> > which should be less than __BPF_REG_TYPE_MAX, and also not permit any
> > type flags to be set for the reg->type.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
> > index 829be2b9e08e..0a8ea60c2a80 100644
> > --- a/tools/testing/selftests/bpf/verifier/calls.c
> > +++ b/tools/testing/selftests/bpf/verifier/calls.c
> > @@ -96,6 +96,25 @@
> >                 { "bpf_kfunc_call_test_mem_len_fail1", 2 },
> >         },
> >  },
> > +{
> > +       "calls: trigger reg2btf_ids[reg->type] for reg->type > __BPF_REG_TYPE_MAX",
> > +       .insns = {
> > +       BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
> > +       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
> > +       BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
> > +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
> > +       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> > +       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
> > +       BPF_EXIT_INSN(),
> > +       },
> > +       .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> > +       .result = REJECT,
> > +       .errstr = "arg#0 pointer type STRUCT prog_test_ref_kfunc must point",
>
> Why do we stop errstr at "must point"?
>

The complete string is too long, I think it matches using strncmp.

> > +       .fixup_kfunc_btf_id = {
> > +               { "bpf_kfunc_call_test_acquire", 3 },
> > +               { "bpf_kfunc_call_test_release", 5 },
> > +       },
> > +},
> >  {
> >         "calls: basic sanity",
> >         .insns = {
> > --
> > 2.35.1
> >

--
Kartikeya

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access
  2022-02-20  2:31 [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access Kumar Kartikeya Dwivedi
  2022-02-22  2:23 ` Song Liu
@ 2022-02-22  4:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-22  4:30 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi; +Cc: bpf, ast, daniel, andrii

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sun, 20 Feb 2022 08:01:38 +0530 you wrote:
> This test tries to pass a PTR_TO_BTF_ID_OR_NULL to the release function,
> which would trigger a out of bounds access without the fix in commit
> 45ce4b4f9009 ("bpf: Fix crash due to out of bounds access into reg2btf_ids.")
> but after the fix, it should only index using base_type(reg->type),
> which should be less than __BPF_REG_TYPE_MAX, and also not permit any
> type flags to be set for the reg->type.
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access
    https://git.kernel.org/bpf/bpf-next/c/13c6a37d409d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-02-22  4:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-20  2:31 [PATCH bpf-next] selftests/bpf: Add test for reg2btf_ids out of bounds access Kumar Kartikeya Dwivedi
2022-02-22  2:23 ` Song Liu
2022-02-22  2:35   ` Kumar Kartikeya Dwivedi
2022-02-22  4:30 ` patchwork-bot+netdevbpf

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).