mbox series

[0/3] Dynptr Verifier Adjustments

Message ID 20230406004018.1439952-1-drosen@google.com (mailing list archive)
Headers show
Series Dynptr Verifier Adjustments | expand

Message

Daniel Rosenberg April 6, 2023, 12:40 a.m. UTC
These patches relax a few verifier requirements around dynptrs.

I was unable to test the patch in 0003 due to unrelated issues compiling the
bpf selftests, but did run an equivalent local test program.

This is the issue I was running into:
progs/cgrp_ls_attach_cgroup.c:17:15: error: use of undeclared identifier 'BPF_MAP_TYPE_CGRP_STORAGE'; did you mean 'BPF_MAP_TYPE_CGROUP_STORAGE'?
        __uint(type, BPF_MAP_TYPE_CGRP_STORAGE);
                     ^~~~~~~~~~~~~~~~~~~~~~~~~
                     BPF_MAP_TYPE_CGROUP_STORAGE
/ssd/kernel/fuse-bpf/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
#define __uint(name, val) int (*name)[val]
                                      ^
/ssd/kernel/fuse-bpf/tools/testing/selftests/bpf/tools/include/vmlinux.h:27892:2: note: 'BPF_MAP_TYPE_CGROUP_STORAGE' declared here
        BPF_MAP_TYPE_CGROUP_STORAGE = 19,
        ^
1 error generated.

Daniel Rosenberg (3):
  bpf: verifier: Accept dynptr mem as mem in helpers
  bpf: Allow NULL buffers in bpf_dynptr_slice(_rw)
  selftests/bpf: Test allowing NULL buffer in dynptr slice

 Documentation/bpf/kfuncs.rst                  | 23 ++++++++++++-
 kernel/bpf/helpers.c                          | 32 ++++++++++++-------
 kernel/bpf/verifier.c                         | 21 ++++++++++++
 .../testing/selftests/bpf/prog_tests/dynptr.c |  1 +
 .../selftests/bpf/progs/dynptr_success.c      | 21 ++++++++++++
 5 files changed, 85 insertions(+), 13 deletions(-)


base-commit: 5af607a861d43ffff830fc1890033e579ec44799

Comments

Andrii Nakryiko April 6, 2023, 8:48 p.m. UTC | #1
On Wed, Apr 5, 2023 at 5:40 PM Daniel Rosenberg <drosen@google.com> wrote:
>
> These patches relax a few verifier requirements around dynptrs.
>
> I was unable to test the patch in 0003 due to unrelated issues compiling the
> bpf selftests, but did run an equivalent local test program.
>
> This is the issue I was running into:
> progs/cgrp_ls_attach_cgroup.c:17:15: error: use of undeclared identifier 'BPF_MAP_TYPE_CGRP_STORAGE'; did you mean 'BPF_MAP_TYPE_CGROUP_STORAGE'?
>         __uint(type, BPF_MAP_TYPE_CGRP_STORAGE);
>                      ^~~~~~~~~~~~~~~~~~~~~~~~~
>                      BPF_MAP_TYPE_CGROUP_STORAGE
> /ssd/kernel/fuse-bpf/tools/testing/selftests/bpf/tools/include/bpf/bpf_helpers.h:13:39: note: expanded from macro '__uint'
> #define __uint(name, val) int (*name)[val]
>                                       ^
> /ssd/kernel/fuse-bpf/tools/testing/selftests/bpf/tools/include/vmlinux.h:27892:2: note: 'BPF_MAP_TYPE_CGROUP_STORAGE' declared here
>         BPF_MAP_TYPE_CGROUP_STORAGE = 19,
>         ^
> 1 error generated.

It is expected that you build the freshest vmlinux image before
building selftests, because we generate vmlinux.h from it. In your
case we generated vmlinux.h from your system-wide
/sys/kernel/btf/vmlinux BTF information, which doesn't yet have latest
UAPI enums.

>
> Daniel Rosenberg (3):
>   bpf: verifier: Accept dynptr mem as mem in helpers
>   bpf: Allow NULL buffers in bpf_dynptr_slice(_rw)
>   selftests/bpf: Test allowing NULL buffer in dynptr slice
>
>  Documentation/bpf/kfuncs.rst                  | 23 ++++++++++++-
>  kernel/bpf/helpers.c                          | 32 ++++++++++++-------
>  kernel/bpf/verifier.c                         | 21 ++++++++++++
>  .../testing/selftests/bpf/prog_tests/dynptr.c |  1 +
>  .../selftests/bpf/progs/dynptr_success.c      | 21 ++++++++++++
>  5 files changed, 85 insertions(+), 13 deletions(-)
>
>
> base-commit: 5af607a861d43ffff830fc1890033e579ec44799
> --
> 2.40.0.577.gac1e443424-goog
>
Daniel Rosenberg April 26, 2023, 10:07 p.m. UTC | #2
>
> It is expected that you build the freshest vmlinux image before
> building selftests, because we generate vmlinux.h from it. In your
> case we generated vmlinux.h from your system-wide
> /sys/kernel/btf/vmlinux BTF information, which doesn't yet have latest
> UAPI enums.
>
I'm still unable to build the selftests. I've got it pointed to a
locally built kernel built using the config/config.x86_64, and have
tried running the vmtest.sh script, and building just the tests via
make. I'm using O= to direct it to the out directory for the kernel
build. I've been hitting various errors when trying this. Confusingly
the error message isn't always the same. Currently from a clean build,
it complains about "linux/atomic.h" not found via #include
"../../../include/linux/filter.h"'s in various files. Other times it's
complained about the various helper functions from bpf_helper_defs.h
being unused.

I'm not sure if I'm invoking the command wrong, or missing
dependencies or something. I got past some earlier issues by updating
clang. Any idea what I'm doing wrong?
Andrii Nakryiko April 26, 2023, 11:39 p.m. UTC | #3
On Wed, Apr 26, 2023 at 3:07 PM Daniel Rosenberg <drosen@google.com> wrote:
>
> >
> > It is expected that you build the freshest vmlinux image before
> > building selftests, because we generate vmlinux.h from it. In your
> > case we generated vmlinux.h from your system-wide
> > /sys/kernel/btf/vmlinux BTF information, which doesn't yet have latest
> > UAPI enums.
> >
> I'm still unable to build the selftests. I've got it pointed to a
> locally built kernel built using the config/config.x86_64, and have
> tried running the vmtest.sh script, and building just the tests via
> make. I'm using O= to direct it to the out directory for the kernel
> build. I've been hitting various errors when trying this. Confusingly
> the error message isn't always the same. Currently from a clean build,
> it complains about "linux/atomic.h" not found via #include
> "../../../include/linux/filter.h"'s in various files. Other times it's
> complained about the various helper functions from bpf_helper_defs.h
> being unused.
>
> I'm not sure if I'm invoking the command wrong, or missing
> dependencies or something. I got past some earlier issues by updating
> clang. Any idea what I'm doing wrong?

Don't know, show the sequence of commands you are running?

I have linux source in ~/linux, and KBUILD_OUTPUT set to
~/linux-build/default. And it only takes this:

$ cd ~/linux
$ make -j90 # build kernel
$ cd tools/testing/selftests/bpf
$ make -j90 # build selftests

And that's it.
Daniel Rosenberg April 27, 2023, 11:36 p.m. UTC | #4
On Wed, Apr 26, 2023 at 4:39 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> Don't know, show the sequence of commands you are running?
>
> I have linux source in ~/linux, and KBUILD_OUTPUT set to
> ~/linux-build/default. And it only takes this:
>
> $ cd ~/linux
> $ make -j90 # build kernel
> $ cd tools/testing/selftests/bpf
> $ make -j90 # build selftests
>
> And that's it.

I've tried the same, modulo some paths. I'm pretty sure it's version
related at this point.
The current issue I'm seeing is "error: indirect call in function,
which are not supported by eBPF" when using GCC-BPF for
progs/bind4_prog.c

Currently using clang 16.0.0 and gcc 12.2.0-14.
I did manage to get it to build by just commenting out TEST_GEN_PROGS
+= test_progs-bpf_gcc
Andrii Nakryiko April 27, 2023, 11:42 p.m. UTC | #5
On Thu, Apr 27, 2023 at 4:36 PM Daniel Rosenberg <drosen@google.com> wrote:
>
> On Wed, Apr 26, 2023 at 4:39 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > Don't know, show the sequence of commands you are running?
> >
> > I have linux source in ~/linux, and KBUILD_OUTPUT set to
> > ~/linux-build/default. And it only takes this:
> >
> > $ cd ~/linux
> > $ make -j90 # build kernel
> > $ cd tools/testing/selftests/bpf
> > $ make -j90 # build selftests
> >
> > And that's it.
>
> I've tried the same, modulo some paths. I'm pretty sure it's version
> related at this point.
> The current issue I'm seeing is "error: indirect call in function,
> which are not supported by eBPF" when using GCC-BPF for
> progs/bind4_prog.c

I don't think GCC-BPF is able to compile selftests properly just yet.
So I guess the problem is that you do have some version of gcc-bpf in
the system and selftest's Makefile tries to build gcc variants of
test_progs? That's bad (I don't have GCC-BPF locally, and everyone
else apparently as well).

So for now just `make BPF_GCC=` ? CC'ing Jose, we should probably
agree on some criteria of "GCC-BPF is really capable of building
selftests" and adjust Makefile to only attempt GCC BPF build in that
case.


>
> Currently using clang 16.0.0 and gcc 12.2.0-14.
> I did manage to get it to build by just commenting out TEST_GEN_PROGS
> += test_progs-bpf_gcc
Jose E. Marchesi April 28, 2023, 9:03 a.m. UTC | #6
> On Thu, Apr 27, 2023 at 4:36 PM Daniel Rosenberg <drosen@google.com> wrote:
>>
>> On Wed, Apr 26, 2023 at 4:39 PM Andrii Nakryiko
>> <andrii.nakryiko@gmail.com> wrote:
>> >
>> > Don't know, show the sequence of commands you are running?
>> >
>> > I have linux source in ~/linux, and KBUILD_OUTPUT set to
>> > ~/linux-build/default. And it only takes this:
>> >
>> > $ cd ~/linux
>> > $ make -j90 # build kernel
>> > $ cd tools/testing/selftests/bpf
>> > $ make -j90 # build selftests
>> >
>> > And that's it.
>>
>> I've tried the same, modulo some paths. I'm pretty sure it's version
>> related at this point.
>> The current issue I'm seeing is "error: indirect call in function,
>> which are not supported by eBPF" when using GCC-BPF for
>> progs/bind4_prog.c
>
> I don't think GCC-BPF is able to compile selftests properly just yet.
> So I guess the problem is that you do have some version of gcc-bpf in
> the system and selftest's Makefile tries to build gcc variants of
> test_progs? That's bad (I don't have GCC-BPF locally, and everyone
> else apparently as well).
>
> So for now just `make BPF_GCC=` ? CC'ing Jose, we should probably
> agree on some criteria of "GCC-BPF is really capable of building
> selftests" and adjust Makefile to only attempt GCC BPF build in that
> case.

Being able to run the selftests is our goal at the moment, but we are
not there yet, no.

What about making the kernel build system to emit a visible warning
before it builds the GCC variants of the tests programs?  Something like
"this is experimental and will likely fail".

>>
>> Currently using clang 16.0.0 and gcc 12.2.0-14.
>> I did manage to get it to build by just commenting out TEST_GEN_PROGS
>> += test_progs-bpf_gcc
Andrii Nakryiko April 28, 2023, 5:20 p.m. UTC | #7
On Fri, Apr 28, 2023 at 2:04 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On Thu, Apr 27, 2023 at 4:36 PM Daniel Rosenberg <drosen@google.com> wrote:
> >>
> >> On Wed, Apr 26, 2023 at 4:39 PM Andrii Nakryiko
> >> <andrii.nakryiko@gmail.com> wrote:
> >> >
> >> > Don't know, show the sequence of commands you are running?
> >> >
> >> > I have linux source in ~/linux, and KBUILD_OUTPUT set to
> >> > ~/linux-build/default. And it only takes this:
> >> >
> >> > $ cd ~/linux
> >> > $ make -j90 # build kernel
> >> > $ cd tools/testing/selftests/bpf
> >> > $ make -j90 # build selftests
> >> >
> >> > And that's it.
> >>
> >> I've tried the same, modulo some paths. I'm pretty sure it's version
> >> related at this point.
> >> The current issue I'm seeing is "error: indirect call in function,
> >> which are not supported by eBPF" when using GCC-BPF for
> >> progs/bind4_prog.c
> >
> > I don't think GCC-BPF is able to compile selftests properly just yet.
> > So I guess the problem is that you do have some version of gcc-bpf in
> > the system and selftest's Makefile tries to build gcc variants of
> > test_progs? That's bad (I don't have GCC-BPF locally, and everyone
> > else apparently as well).
> >
> > So for now just `make BPF_GCC=` ? CC'ing Jose, we should probably
> > agree on some criteria of "GCC-BPF is really capable of building
> > selftests" and adjust Makefile to only attempt GCC BPF build in that
> > case.
>
> Being able to run the selftests is our goal at the moment, but we are
> not there yet, no.
>
> What about making the kernel build system to emit a visible warning
> before it builds the GCC variants of the tests programs?  Something like
> "this is experimental and will likely fail".

Given gcc-bpf can't build selftests right now, should we just disable
it until there is a version on which gcc-bpf works? We can make it
such that you can force it to build using gcc-bpf (make USE_GCC_BPF=1
or something).

>
> >>
> >> Currently using clang 16.0.0 and gcc 12.2.0-14.
> >> I did manage to get it to build by just commenting out TEST_GEN_PROGS
> >> += test_progs-bpf_gcc
Jose E. Marchesi May 3, 2023, 6:27 p.m. UTC | #8
> On Fri, Apr 28, 2023 at 2:04 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> > On Thu, Apr 27, 2023 at 4:36 PM Daniel Rosenberg <drosen@google.com> wrote:
>> >>
>> >> On Wed, Apr 26, 2023 at 4:39 PM Andrii Nakryiko
>> >> <andrii.nakryiko@gmail.com> wrote:
>> >> >
>> >> > Don't know, show the sequence of commands you are running?
>> >> >
>> >> > I have linux source in ~/linux, and KBUILD_OUTPUT set to
>> >> > ~/linux-build/default. And it only takes this:
>> >> >
>> >> > $ cd ~/linux
>> >> > $ make -j90 # build kernel
>> >> > $ cd tools/testing/selftests/bpf
>> >> > $ make -j90 # build selftests
>> >> >
>> >> > And that's it.
>> >>
>> >> I've tried the same, modulo some paths. I'm pretty sure it's version
>> >> related at this point.
>> >> The current issue I'm seeing is "error: indirect call in function,
>> >> which are not supported by eBPF" when using GCC-BPF for
>> >> progs/bind4_prog.c
>> >
>> > I don't think GCC-BPF is able to compile selftests properly just yet.
>> > So I guess the problem is that you do have some version of gcc-bpf in
>> > the system and selftest's Makefile tries to build gcc variants of
>> > test_progs? That's bad (I don't have GCC-BPF locally, and everyone
>> > else apparently as well).
>> >
>> > So for now just `make BPF_GCC=` ? CC'ing Jose, we should probably
>> > agree on some criteria of "GCC-BPF is really capable of building
>> > selftests" and adjust Makefile to only attempt GCC BPF build in that
>> > case.
>>
>> Being able to run the selftests is our goal at the moment, but we are
>> not there yet, no.
>>
>> What about making the kernel build system to emit a visible warning
>> before it builds the GCC variants of the tests programs?  Something like
>> "this is experimental and will likely fail".
>
> Given gcc-bpf can't build selftests right now, should we just disable
> it until there is a version on which gcc-bpf works? We can make it
> such that you can force it to build using gcc-bpf (make USE_GCC_BPF=1
> or something).

I think that makes sense.

>>
>> >>
>> >> Currently using clang 16.0.0 and gcc 12.2.0-14.
>> >> I did manage to get it to build by just commenting out TEST_GEN_PROGS
>> >> += test_progs-bpf_gcc