Skip to content

Commit

Permalink
Merge branch 'libbpf: stricter BPF program section name handling'
Browse files Browse the repository at this point in the history
Andrii Nakryiko says:

====================

Implement opt-in stricter BPF program section name (SEC()) handling logic. For
a lot of supported ELF section names, enforce exact section name match with no
arbitrary characters added at the end. See patch #9 for more details.

To allow this, patches #2 through #4 clean up and preventively fix selftests,
normalizing existing SEC() usage across multiple selftests. While at it, those
patches also reduce the amount of remaining bpf_object__find_program_by_title()
uses, which should be completely removed soon, given it's an API with
ambiguous semantics and will be deprecated and eventually removed in libbpf 1.0.

Patch #1 also introduces SEC("tc") as an alias for SEC("classifier"). "tc" is
a better and less misleading name, so patch #3 replaces all classifier* uses
with nice and short SEC("tc").

Last patch is also fixing "sk_lookup/" definition to not require and not allow
extra "/blah" parts after it, which serve no meaning.

All the other patches are gradual internal libbpf changes to:
  - allow this optional strict logic for ELF section name handling;
  - allow new use case (for now for "struct_ops", but that could be extended
    to, say, freplace definitions), in which it can be used stand-alone to
    specify just type (SEC("struct_ops")), or also accept extra parameters
    which can be utilized by libbpf to either get more data or double-check
    valid use (e.g., SEC("struct_ops/dctcp_init") to specify desired
    struct_ops operation that is supposed to be implemented);
  - get libbpf's internal logic ready to allow other libraries and
    applications to specify their custom handlers for ELF section name for BPF
    programs. All the pieces are in place, the only thing preventing making
    this as public libbpf API is reliance on internal type for specifying BPF
    program load attributes. The work is planned to revamp related low-level
    libbpf APIs, at which point it will be possible to just re-use such new
    types for coordination between libbpf and custom handlers.

These changes are a part of libbpf 1.0 effort ([0]). They are also intended to
be applied on top of the previous preparatory series [1], so currently CI will
be failing to apply them to bpf-next until that patch set is landed. Once it
is landed, kernel-patches daemon will automatically retest this patch set.

  [0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling
  [1] https://patchwork.kernel.org/project/netdevbpf/list/?series=547675&state=*

v3->v4:
  - replace SEC("classifier*") with SEC("tc") (Daniel);
v2->v3:
  - applied acks, addressed most feedback, added comments to new flags (Dave);
v1->v2:
  - rebase onto latest bpf-next and resolve merge conflicts w/ Dave's changes.
====================

Signed-off-by: Alexei Starovoitov <[email protected]>
  • Loading branch information
Alexei Starovoitov committed Sep 28, 2021
2 parents 29eef85 + 7c80c87 commit 4e874b1
Show file tree
Hide file tree
Showing 71 changed files with 496 additions and 499 deletions.
518 changes: 260 additions & 258 deletions tools/lib/bpf/libbpf.c

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions tools/lib/bpf/libbpf_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,13 @@
(offsetof(TYPE, FIELD) + sizeof(((TYPE *)0)->FIELD))
#endif

/* Check whether a string `str` has prefix `pfx`, regardless if `pfx` is
* a string literal known at compilation time or char * pointer known only at
* runtime.
*/
#define str_has_pfx(str, pfx) \
(strncmp(str, pfx, __builtin_constant_p(pfx) ? sizeof(pfx) - 1 : strlen(pfx)) == 0)

/* Symbol versioning is different between static and shared library.
* Properly versioned symbols are needed for shared library, but
* only the symbol of the new version is needed for static library.
Expand Down
9 changes: 9 additions & 0 deletions tools/lib/bpf/libbpf_legacy.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ enum libbpf_strict_mode {
*/
LIBBPF_STRICT_DIRECT_ERRS = 0x02,

/*
* Enforce strict BPF program section (SEC()) names.
* E.g., while prefiously SEC("xdp_whatever") or SEC("perf_event_blah") were
* allowed, with LIBBPF_STRICT_SEC_PREFIX this will become
* unrecognized by libbpf and would have to be just SEC("xdp") and
* SEC("xdp") and SEC("perf_event").
*/
LIBBPF_STRICT_SEC_NAME = 0x04,

__LIBBPF_STRICT_LAST,
};

Expand Down
4 changes: 2 additions & 2 deletions tools/testing/selftests/bpf/prog_tests/flow_dissector.c
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,9 @@ static int init_prog_array(struct bpf_object *obj, struct bpf_map *prog_array)
return -1;

for (i = 0; i < bpf_map__def(prog_array)->max_entries; i++) {
snprintf(prog_name, sizeof(prog_name), "flow_dissector/%i", i);
snprintf(prog_name, sizeof(prog_name), "flow_dissector_%d", i);

prog = bpf_object__find_program_by_title(obj, prog_name);
prog = bpf_object__find_program_by_name(obj, prog_name);
if (!prog)
return -1;

Expand Down
23 changes: 10 additions & 13 deletions tools/testing/selftests/bpf/prog_tests/reference_tracking.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
#include <test_progs.h>

static void toggle_object_autoload_progs(const struct bpf_object *obj,
const char *title_load)
const char *name_load)
{
struct bpf_program *prog;

bpf_object__for_each_program(prog, obj) {
const char *title = bpf_program__section_name(prog);
const char *name = bpf_program__name(prog);

if (!strcmp(title_load, title))
if (!strcmp(name_load, name))
bpf_program__set_autoload(prog, true);
else
bpf_program__set_autoload(prog, false);
Expand Down Expand Up @@ -39,23 +39,19 @@ void test_reference_tracking(void)
goto cleanup;

bpf_object__for_each_program(prog, obj_iter) {
const char *title;
const char *name;

/* Ignore .text sections */
title = bpf_program__section_name(prog);
if (strstr(title, ".text") != NULL)
continue;

if (!test__start_subtest(title))
name = bpf_program__name(prog);
if (!test__start_subtest(name))
continue;

obj = bpf_object__open_file(file, &open_opts);
if (!ASSERT_OK_PTR(obj, "obj_open_file"))
goto cleanup;

toggle_object_autoload_progs(obj, title);
toggle_object_autoload_progs(obj, name);
/* Expect verifier failure if test name has 'err' */
if (strstr(title, "err_") != NULL) {
if (strncmp(name, "err_", sizeof("err_") - 1) == 0) {
libbpf_print_fn_t old_print_fn;

old_print_fn = libbpf_set_print(NULL);
Expand All @@ -64,7 +60,8 @@ void test_reference_tracking(void)
} else {
err = bpf_object__load(obj);
}
CHECK(err, title, "\n");
ASSERT_OK(err, name);

bpf_object__close(obj);
obj = NULL;
}
Expand Down
2 changes: 1 addition & 1 deletion tools/testing/selftests/bpf/prog_tests/sk_assign.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ configure_stack(void)
return false;
sprintf(tc_cmd, "%s %s %s %s", "tc filter add dev lo ingress bpf",
"direct-action object-file ./test_sk_assign.o",
"section classifier/sk_assign_test",
"section tc",
(env.verbosity < VERBOSE_VERY) ? " 2>/dev/null" : "verbose");
if (CHECK(system(tc_cmd), "BPF load failed;",
"run with -vv for more info\n"))
Expand Down
30 changes: 15 additions & 15 deletions tools/testing/selftests/bpf/prog_tests/sockopt_multi.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#include <test_progs.h>
#include "cgroup_helpers.h"

static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title, const char *name)
{
enum bpf_attach_type attach_type;
enum bpf_prog_type prog_type;
Expand All @@ -15,23 +15,23 @@ static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
return -1;
}

prog = bpf_object__find_program_by_title(obj, title);
prog = bpf_object__find_program_by_name(obj, name);
if (!prog) {
log_err("Failed to find %s BPF program", title);
log_err("Failed to find %s BPF program", name);
return -1;
}

err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd,
attach_type, BPF_F_ALLOW_MULTI);
if (err) {
log_err("Failed to attach %s BPF program", title);
log_err("Failed to attach %s BPF program", name);
return -1;
}

return 0;
}

static int prog_detach(struct bpf_object *obj, int cgroup_fd, const char *title)
static int prog_detach(struct bpf_object *obj, int cgroup_fd, const char *title, const char *name)
{
enum bpf_attach_type attach_type;
enum bpf_prog_type prog_type;
Expand All @@ -42,7 +42,7 @@ static int prog_detach(struct bpf_object *obj, int cgroup_fd, const char *title)
if (err)
return -1;

prog = bpf_object__find_program_by_title(obj, title);
prog = bpf_object__find_program_by_name(obj, name);
if (!prog)
return -1;

Expand Down Expand Up @@ -89,7 +89,7 @@ static int run_getsockopt_test(struct bpf_object *obj, int cg_parent,
* - child: 0x80 -> 0x90
*/

err = prog_attach(obj, cg_child, "cgroup/getsockopt/child");
err = prog_attach(obj, cg_child, "cgroup/getsockopt", "_getsockopt_child");
if (err)
goto detach;

Expand All @@ -113,7 +113,7 @@ static int run_getsockopt_test(struct bpf_object *obj, int cg_parent,
* - parent: 0x90 -> 0xA0
*/

err = prog_attach(obj, cg_parent, "cgroup/getsockopt/parent");
err = prog_attach(obj, cg_parent, "cgroup/getsockopt", "_getsockopt_parent");
if (err)
goto detach;

Expand Down Expand Up @@ -157,7 +157,7 @@ static int run_getsockopt_test(struct bpf_object *obj, int cg_parent,
* - parent: unexpected 0x40, EPERM
*/

err = prog_detach(obj, cg_child, "cgroup/getsockopt/child");
err = prog_detach(obj, cg_child, "cgroup/getsockopt", "_getsockopt_child");
if (err) {
log_err("Failed to detach child program");
goto detach;
Expand Down Expand Up @@ -198,8 +198,8 @@ static int run_getsockopt_test(struct bpf_object *obj, int cg_parent,
}

detach:
prog_detach(obj, cg_child, "cgroup/getsockopt/child");
prog_detach(obj, cg_parent, "cgroup/getsockopt/parent");
prog_detach(obj, cg_child, "cgroup/getsockopt", "_getsockopt_child");
prog_detach(obj, cg_parent, "cgroup/getsockopt", "_getsockopt_parent");

return err;
}
Expand Down Expand Up @@ -236,7 +236,7 @@ static int run_setsockopt_test(struct bpf_object *obj, int cg_parent,

/* Attach child program and make sure it adds 0x10. */

err = prog_attach(obj, cg_child, "cgroup/setsockopt");
err = prog_attach(obj, cg_child, "cgroup/setsockopt", "_setsockopt");
if (err)
goto detach;

Expand All @@ -263,7 +263,7 @@ static int run_setsockopt_test(struct bpf_object *obj, int cg_parent,

/* Attach parent program and make sure it adds another 0x10. */

err = prog_attach(obj, cg_parent, "cgroup/setsockopt");
err = prog_attach(obj, cg_parent, "cgroup/setsockopt", "_setsockopt");
if (err)
goto detach;

Expand All @@ -289,8 +289,8 @@ static int run_setsockopt_test(struct bpf_object *obj, int cg_parent,
}

detach:
prog_detach(obj, cg_child, "cgroup/setsockopt");
prog_detach(obj, cg_parent, "cgroup/setsockopt");
prog_detach(obj, cg_child, "cgroup/setsockopt", "_setsockopt");
prog_detach(obj, cg_parent, "cgroup/setsockopt", "_setsockopt");

return err;
}
Expand Down
Loading

0 comments on commit 4e874b1

Please sign in to comment.