Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220429104052.345760505@linuxfoundation.org> <20220429104053.296096344@linuxfoundation.org>
In-Reply-To: <20220429104053.296096344@linuxfoundation.org>
From:   Po-Hsu Lin <po-hsu.lin@canonical.com>
Date:   Fri, 24 Jun 2022 18:33:57 +0800
Message-ID: <CAMy_GT9YbgqsoLbCDqhHpXNW6EejgK+YaE4YPxpxcmer+qn-1g@mail.gmail.com>
Subject: Re: [PATCH 5.15 33/33] selftests/bpf: Add test for reg2btf_ids out of
 bounds access
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Apr 29, 2022 at 6:47 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> 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_id=
s.")
> 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>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Link: https://lore.kernel.org/bpf/20220220023138.2224652-1-memxor@gmail.c=
om
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  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 =3D "R0 min value is outside of the allowed memory range"=
,
>  },
>  {
> +       "calls: trigger reg2btf_ids[reg->type] for reg->type > __BPF_REG_=
TYPE_MAX",
> +       .insns =3D {
> +       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 =3D BPF_PROG_TYPE_SCHED_CLS,
> +       .result =3D REJECT,
> +       .errstr =3D "arg#0 pointer type STRUCT prog_test_ref_kfunc must p=
oint",
> +       .fixup_kfunc_btf_id =3D {
> +               { "bpf_kfunc_call_test_acquire", 3 },
> +               { "bpf_kfunc_call_test_release", 5 },
> +       },
> +},
> +{
>         "calls: overlapping caller/callee",
>         .insns =3D {
>         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: =E2=80=98struct bpf_test=E2=80=99 has no member named =E2=80=98fixup=
_kfunc_btf_id=E2=80=99
  124 |         .fixup_kfunc_btf_id =3D {
      |          ^~~~~~~~~~~~~~~~~~
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
warning: braces around scalar initializer
  124 |         .fixup_kfunc_btf_id =3D {
      |         ^
/home/ubuntu/linux/tools/testing/selftests/bpf/verifier/calls.c:124:9:
note: (near initialization for =E2=80=98tests[150].errstr_unpriv=E2=80=99)
/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 =E2=80=98tests[150].errstr_unpriv=E2=80=99)
/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 =E2=80=98tests[150].errstr_unpriv=E2=80=99)
/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 =E2=80=98tests[150].errstr_unpriv=E2=80=99)
/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 =E2=80=98tests[150].errstr_unpriv=E2=80=99)
/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 =E2=80=98tests[150].errstr_unpriv=E2=80=99)
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_time=
r'
        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