On Fri, Dec 1, 2023 at 9:47 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>
> On 01/12/2023 17:06, Andrii Nakryiko wrote:
> > Add selftest that establishes dead code-eliminated valid global subprog
> > (global_dead) and makes sure that it's not possible to freplace it, as
> > it's effectively not there. This test will fail with unexpected success
> > before 2afae08c9dcb ("bpf: Validate global subprogs lazily").
> >
> > v1->v2:
> > - don't rely on assembly output in verifier log, which changes between
> > compiler versions (CI).
> >
> > Suggested-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> one minor thing below, but
>
> Reviewed-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
>
> > ---
> > .../bpf/prog_tests/global_func_dead_code.c | 60 +++++++++++++++++++
> > .../bpf/progs/freplace_dead_global_func.c | 11 ++++
> > .../bpf/progs/verifier_global_subprogs.c | 33 ++++++----
> > 3 files changed, 92 insertions(+), 12 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/global_func_dead_code.c
> > create mode 100644 tools/testing/selftests/bpf/progs/freplace_dead_global_func.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/global_func_dead_code.c b/tools/testing/selftests/bpf/prog_tests/global_func_dead_code.c
> > new file mode 100644
> > index 000000000000..d873eb20dd7c
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/global_func_dead_code.c
> > @@ -0,0 +1,60 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> > +
> > +#include <test_progs.h>
> > +#include "verifier_global_subprogs.skel.h"
> > +#include "freplace_dead_global_func.skel.h"
> > +
> > +void test_global_func_dead_code(void)
> > +{
> > + struct verifier_global_subprogs *tgt_skel = NULL;
> > + struct freplace_dead_global_func *skel = NULL;
> > + char log_buf[4096];
> > + int err, tgt_fd;
> > +
> > + /* first, try to load target with good global subprog */
> > + tgt_skel = verifier_global_subprogs__open();
> > + if (!ASSERT_OK_PTR(tgt_skel, "tgt_skel_good_open"))
> > + return;
> > +
> > + bpf_program__set_autoload(tgt_skel->progs.chained_global_func_calls_success, true);
> > +
> > + err = verifier_global_subprogs__load(tgt_skel);
> > + if (!ASSERT_OK(err, "tgt_skel_good_load"))
> > + goto out;
> > +
> > + tgt_fd = bpf_program__fd(tgt_skel->progs.chained_global_func_calls_success);
> > +
> > + /* Attach to good non-eliminated subprog */
> > + skel = freplace_dead_global_func__open();
> > + if (!ASSERT_OK_PTR(skel, "skel_good_open"))
> > + goto out;
> > +
> > + bpf_program__set_attach_target(skel->progs.freplace_prog, tgt_fd, "global_good");
>
> missing "err = " assignment here?
yep, thanks for spotting, will fix
>
> > + ASSERT_OK(err, "attach_target_good");
> > +
> > + err = freplace_dead_global_func__load(skel);
> > + if (!ASSERT_OK(err, "skel_good_load"))
> > + goto out;
> > +
> > + freplace_dead_global_func__destroy(skel);
> > +
> > + /* Try attaching to dead code-eliminated subprog */
> > + skel = freplace_dead_global_func__open();
> > + if (!ASSERT_OK_PTR(skel, "skel_dead_open"))
> > + goto out;
> > +
> > + bpf_program__set_log_buf(skel->progs.freplace_prog, log_buf, sizeof(log_buf));
> > + err = bpf_program__set_attach_target(skel->progs.freplace_prog, tgt_fd, "global_dead");
> > + ASSERT_OK(err, "attach_target_dead");
> > +
> > + err = freplace_dead_global_func__load(skel);
> > + if (!ASSERT_ERR(err, "skel_dead_load"))
> > + goto out;
> > +
> > + ASSERT_HAS_SUBSTR(log_buf, "Subprog global_dead doesn't exist", "dead_subprog_missing_msg");
> > +
> > +out:
> > + verifier_global_subprogs__destroy(tgt_skel);
> > + freplace_dead_global_func__destroy(skel);
> > +}
[...]
