From: Kumar Kartikeya Dwivedi <[email protected]>
commit 13c6a37d409db9abc9c0bfc6d0a2f07bf0fff60e upstream.
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 <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -108,6 +108,25 @@
.errstr = "R0 min value is outside of the allowed memory range",
},
{
+ "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: overlapping caller/callee",
.insns = {
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 0),
On Fri, Apr 29, 2022 at 6:47 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> From: Kumar Kartikeya Dwivedi <[email protected]>
>
> commit 13c6a37d409db9abc9c0bfc6d0a2f07bf0fff60e upstream.
>
> 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 <[email protected]>
> Signed-off-by: Alexei Starovoitov <[email protected]>
> Link: https://lore.kernel.org/bpf/[email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> --- a/tools/testing/selftests/bpf/verifier/calls.c
> +++ b/tools/testing/selftests/bpf/verifier/calls.c
> @@ -108,6 +108,25 @@
> .errstr = "R0 min value is outside of the allowed memory range",
> },
> {
> + "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: overlapping caller/callee",
> .insns = {
> BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 0),
>
>
Hello Greg,
When I tried to build the bpf selftest from 5.15.49 source tree on a
Ubuntu Jammy instance running with 5.15.49-051549-generic, I got the
following error message:
In file included from
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/tests.h:21,
from test_verifier.c:432:
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:10:
error: ‘struct bpf_test’ has no member named ‘fixup_kfunc_btf_id’
124 | .fixup_kfunc_btf_id = {
| ^~~~~~~~~~~~~~~~~~
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
warning: braces around scalar initializer
124 | .fixup_kfunc_btf_id = {
| ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
note: (near initialization for ‘tests[150].errstr_unpriv’)
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:17:
warning: braces around scalar initializer
125 | { "bpf_kfunc_call_test_acquire", 3 },
| ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:17:
note: (near initialization for ‘tests[150].errstr_unpriv’)
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:50:
warning: excess elements in scalar initializer
125 | { "bpf_kfunc_call_test_acquire", 3 },
| ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:50:
note: (near initialization for ‘tests[150].errstr_unpriv’)
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
warning: braces around scalar initializer
126 | { "bpf_kfunc_call_test_release", 5 },
| ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
note: (near initialization for ‘tests[150].errstr_unpriv’)
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:50:
warning: excess elements in scalar initializer
126 | { "bpf_kfunc_call_test_release", 5 },
| ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:50:
note: (near initialization for ‘tests[150].errstr_unpriv’)
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
warning: excess elements in scalar initializer
126 | { "bpf_kfunc_call_test_release", 5 },
| ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
note: (near initialization for ‘tests[150].errstr_unpriv’)
make: *** [Makefile:508:
/home/ubuntu/linux/tools/testing/selftests/bpf/test_verifier] Error 1
Which is introduced by this commit f59e6886c "selftests/bpf: Add test
for reg2btf_ids out of bounds access" on 5.15. With this commit
reverted, there will be another error in progs/timer_crash.c like in
5.10 [1]:
progs/timer_crash.c:8:19: error: field has incomplete type 'struct bpf_timer'
struct bpf_timer timer;
^
/home/ubuntu/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helper_defs.h:39:8:
note: forward declaration of 'struct bpf_timer'
struct bpf_timer;
^
1 error generated.
Maybe commit "selftests/bpf: Add test for bpf_timer overwriting crash"
should be reverted on 5.15 as well.
Thanks
[1] https://www.spinics.net/lists/stable/msg542618.html
On Fri, Jun 24, 2022 at 06:33:57PM +0800, Po-Hsu Lin wrote:
> On Fri, Apr 29, 2022 at 6:47 PM Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > From: Kumar Kartikeya Dwivedi <[email protected]>
> >
> > commit 13c6a37d409db9abc9c0bfc6d0a2f07bf0fff60e upstream.
> >
> > 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 <[email protected]>
> > Signed-off-by: Alexei Starovoitov <[email protected]>
> > Link: https://lore.kernel.org/bpf/[email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
> > tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > --- a/tools/testing/selftests/bpf/verifier/calls.c
> > +++ b/tools/testing/selftests/bpf/verifier/calls.c
> > @@ -108,6 +108,25 @@
> > .errstr = "R0 min value is outside of the allowed memory range",
> > },
> > {
> > + "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: overlapping caller/callee",
> > .insns = {
> > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 0),
> >
> >
>
> Hello Greg,
>
> When I tried to build the bpf selftest from 5.15.49 source tree on a
> Ubuntu Jammy instance running with 5.15.49-051549-generic, I got the
> following error message:
>
> In file included from
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/tests.h:21,
> from test_verifier.c:432:
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:10:
> error: ‘struct bpf_test’ has no member named ‘fixup_kfunc_btf_id’
> 124 | .fixup_kfunc_btf_id = {
> | ^~~~~~~~~~~~~~~~~~
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
> warning: braces around scalar initializer
> 124 | .fixup_kfunc_btf_id = {
> | ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
> note: (near initialization for ‘tests[150].errstr_unpriv’)
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:17:
> warning: braces around scalar initializer
> 125 | { "bpf_kfunc_call_test_acquire", 3 },
> | ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:17:
> note: (near initialization for ‘tests[150].errstr_unpriv’)
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:50:
> warning: excess elements in scalar initializer
> 125 | { "bpf_kfunc_call_test_acquire", 3 },
> | ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:50:
> note: (near initialization for ‘tests[150].errstr_unpriv’)
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> warning: braces around scalar initializer
> 126 | { "bpf_kfunc_call_test_release", 5 },
> | ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> note: (near initialization for ‘tests[150].errstr_unpriv’)
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:50:
> warning: excess elements in scalar initializer
> 126 | { "bpf_kfunc_call_test_release", 5 },
> | ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:50:
> note: (near initialization for ‘tests[150].errstr_unpriv’)
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> warning: excess elements in scalar initializer
> 126 | { "bpf_kfunc_call_test_release", 5 },
> | ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> note: (near initialization for ‘tests[150].errstr_unpriv’)
> make: *** [Makefile:508:
> /home/ubuntu/linux/tools/testing/selftests/bpf/test_verifier] Error 1
>
> Which is introduced by this commit f59e6886c "selftests/bpf: Add test
> for reg2btf_ids out of bounds access" on 5.15. With this commit
> reverted, there will be another error in progs/timer_crash.c like in
> 5.10 [1]:
>
> progs/timer_crash.c:8:19: error: field has incomplete type 'struct bpf_timer'
> struct bpf_timer timer;
> ^
> /home/ubuntu/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helper_defs.h:39:8:
> note: forward declaration of 'struct bpf_timer'
> struct bpf_timer;
> ^
> 1 error generated.
>
> Maybe commit "selftests/bpf: Add test for bpf_timer overwriting crash"
> should be reverted on 5.15 as well.
Should the test be fixed instead?
I'll take patches for either, thanks.
greg k-h
On Fri, Jun 24, 2022 at 7:44 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Fri, Jun 24, 2022 at 06:33:57PM +0800, Po-Hsu Lin wrote:
> > On Fri, Apr 29, 2022 at 6:47 PM Greg Kroah-Hartman
> > <[email protected]> wrote:
> > >
> > > From: Kumar Kartikeya Dwivedi <[email protected]>
> > >
> > > commit 13c6a37d409db9abc9c0bfc6d0a2f07bf0fff60e upstream.
> > >
> > > 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 <[email protected]>
> > > Signed-off-by: Alexei Starovoitov <[email protected]>
> > > Link: https://lore.kernel.org/bpf/[email protected]
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > ---
> > > tools/testing/selftests/bpf/verifier/calls.c | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > --- a/tools/testing/selftests/bpf/verifier/calls.c
> > > +++ b/tools/testing/selftests/bpf/verifier/calls.c
> > > @@ -108,6 +108,25 @@
> > > .errstr = "R0 min value is outside of the allowed memory range",
> > > },
> > > {
> > > + "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: overlapping caller/callee",
> > > .insns = {
> > > BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 0),
> > >
> > >
> >
> > Hello Greg,
> >
> > When I tried to build the bpf selftest from 5.15.49 source tree on a
> > Ubuntu Jammy instance running with 5.15.49-051549-generic, I got the
> > following error message:
> >
> > In file included from
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/tests.h:21,
> > from test_verifier.c:432:
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:10:
> > error: ‘struct bpf_test’ has no member named ‘fixup_kfunc_btf_id’
> > 124 | .fixup_kfunc_btf_id = {
> > | ^~~~~~~~~~~~~~~~~~
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
> > warning: braces around scalar initializer
> > 124 | .fixup_kfunc_btf_id = {
> > | ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
> > note: (near initialization for ‘tests[150].errstr_unpriv’)
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:17:
> > warning: braces around scalar initializer
> > 125 | { "bpf_kfunc_call_test_acquire", 3 },
> > | ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:17:
> > note: (near initialization for ‘tests[150].errstr_unpriv’)
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:50:
> > warning: excess elements in scalar initializer
> > 125 | { "bpf_kfunc_call_test_acquire", 3 },
> > | ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:125:50:
> > note: (near initialization for ‘tests[150].errstr_unpriv’)
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> > warning: braces around scalar initializer
> > 126 | { "bpf_kfunc_call_test_release", 5 },
> > | ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> > note: (near initialization for ‘tests[150].errstr_unpriv’)
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:50:
> > warning: excess elements in scalar initializer
> > 126 | { "bpf_kfunc_call_test_release", 5 },
> > | ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:50:
> > note: (near initialization for ‘tests[150].errstr_unpriv’)
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> > warning: excess elements in scalar initializer
> > 126 | { "bpf_kfunc_call_test_release", 5 },
> > | ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:126:17:
> > note: (near initialization for ‘tests[150].errstr_unpriv’)
> > make: *** [Makefile:508:
> > /home/ubuntu/linux/tools/testing/selftests/bpf/test_verifier] Error 1
> >
> > Which is introduced by this commit f59e6886c "selftests/bpf: Add test
> > for reg2btf_ids out of bounds access" on 5.15. With this commit
> > reverted, there will be another error in progs/timer_crash.c like in
> > 5.10 [1]:
> >
> > progs/timer_crash.c:8:19: error: field has incomplete type 'struct bpf_timer'
> > struct bpf_timer timer;
> > ^
> > /home/ubuntu/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helper_defs.h:39:8:
> > note: forward declaration of 'struct bpf_timer'
> > struct bpf_timer;
> > ^
> > 1 error generated.
> >
> > Maybe commit "selftests/bpf: Add test for bpf_timer overwriting crash"
> > should be reverted on 5.15 as well.
>
> Should the test be fixed instead?
>
OK!
I will try if I can backport the fix. Otherwise will just go for the revert.
Thanks!
> I'll take patches for either, thanks.
>
> greg k-h