Re: [PATCH v14 bpf-next 6/6] selftests/bpf: Add test that uses fsverity and xattr to sign a file

[Song Liu <song@kernel.org>]

On Wed, Nov 29, 2023 at 6:56 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Nov 29, 2023 at 3:20 AM Song Liu <song@kernel.org> wrote:
> >
> > On Tue, Nov 28, 2023 at 10:47 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Tue, Nov 28, 2023 at 4:37 PM Song Liu <song@kernel.org> wrote:
> > > > +char digest[MAGIC_SIZE + sizeof(struct fsverity_digest) + SHA256_DIGEST_SIZE];
> > >
> > > when vmlinux is built without CONFIG_FS_VERITY the above fails
> > > in a weird way:
> > >   CLNG-BPF [test_maps] test_sig_in_xattr.bpf.o
> > > progs/test_sig_in_xattr.c:36:26: error: invalid application of
> > > 'sizeof' to an incomplete type 'struct fsverity_digest'
> > >    36 | char digest[MAGIC_SIZE + sizeof(struct fsverity_digest) +
> > > SHA256_DIGEST_SIZE];
> > >       |                          ^     ~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Is there a way to somehow print a hint during the build what
> > > configs users need to enable to pass the build ?
> >
> > Patch 5/6 added CONFIG_FS_VERITY to tools/testing/selftests/bpf/config.
> > This is a more general question for all required CONFIG_* specified in the
> > file (and the config files for other selftests).
> >
> > In selftests/bpf/Makefile, we have logic to find vmlinux. We can add similar
> > logic to find .config used to build the vmlinux, and grep for each required
> > CONFIG_* from the .config file. Does this sound like a viable solution?
>
> No need for new logic to parse .config.
> libbpf does it already and
> extern bool CONFIG_FS_VERITY __kconfig __weak;
> works.
>
> Since you hard code MAGIC_SIZE anyway I'm asking
> to hard code sizeof(struct fsverity_digest) as well, since the bpf prog
> doesn't access it directly. It only needs to know its size.
>
> While inside:
> int BPF_PROG(test_file_open, struct file *f)
> {
>   if (!CONFIG_FS_VERITY) {
>      skip_fs_verity_test = true;
>      return 0;
>   }
>
> and report it as a clean error message in test_progs.

Yeah, this makes sense. Let me update the tests.

Thanks,
Song

> We keep adding new config requirements selftests/bpf/config which
> forces all developers to keep adding new configs to their builds.
> In the past, when we didn't have BPF CI, that was necessary, but now
> BPF CI does it for us.
> With clean error message from test_progs the developers can either
> ignore the error and proceed with their work or adjust their .config
> eventually. While hard selftest build error forces all devs to
> update .config right away and build error has no info of what needs
> to be done which is not developer friendly.
>
> pw-bot: cr
>