| Message ID | 20210917061020.821711-9-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: stricter BPF program section name handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/apply | fail | Patch does not apply to bpf-next |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| bpf/vmtest-bpf-next | success | VM_Test |
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
On 09/16, Andrii Nakryiko wrote: > Implement strict ELF section name handling for BPF programs. It utilizes > `libbpf_set_strict_mode()` framework and adds new flag: > LIBBPF_STRICT_SEC_NAME. > If this flag is set, libbpf will enforce exact section name matching for > a lot of program types that previously allowed just partial prefix > match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now > in strict mode only SEC("xdp") will be accepted, which makes SEC("") > definitions cleaner and more structured. SEC() now won't be used as yet > another way to uniquely encode BPF program identifier (for that > C function name is better and is guaranteed to be unique within > bpf_object). Now SEC() is strictly BPF program type and, depending on > program type, extra load/attach parameter specification. > Libbpf completely supports multiple BPF programs in the same ELF > section, so multiple BPF programs of the same type/specification easily > co-exist together within the same bpf_object scope. > Additionally, a new (for now internal) convention is introduced: section > name that can be a stand-alone exact BPF program type specificator, but > also could have extra parameters after '/' delimiter. An example of such > section is "struct_ops", which can be specified by itself, but also > allows to specify the intended operation to be attached to, e.g., > "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not allowed. > Such section definition is specified as "struct_ops+". > This change is part of libbpf 1.0 effort ([0], [1]). > [0] Closes: https://github.com/libbpf/libbpf/issues/271 > [1] > https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > tools/lib/bpf/libbpf.c | 135 ++++++++++++++++++++++------------ > tools/lib/bpf/libbpf_legacy.h | 9 +++ > 2 files changed, 98 insertions(+), 46 deletions(-) > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 56082865ceff..f0846f609e26 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -232,6 +232,7 @@ enum sec_def_flags { > SEC_ATTACHABLE_OPT = SEC_ATTACHABLE | SEC_EXP_ATTACH_OPT, > SEC_ATTACH_BTF = 4, > SEC_SLEEPABLE = 8, > + SEC_SLOPPY_PFX = 16, /* allow non-strict prefix matching */ > }; > struct bpf_sec_def { > @@ -7976,15 +7977,15 @@ static struct bpf_link *attach_lsm(const struct > bpf_program *prog, long cookie); > static struct bpf_link *attach_iter(const struct bpf_program *prog, long > cookie); > static const struct bpf_sec_def section_defs[] = { > - SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE), > - SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE), > - SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, > SEC_ATTACHABLE), > + SEC_DEF("socket", SOCKET_FILTER, 0, SEC_SLOPPY_PFX), > + SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > SEC_DEF("kprobe/", KPROBE, 0, SEC_NONE, attach_kprobe), > SEC_DEF("uprobe/", KPROBE, 0, SEC_NONE), > SEC_DEF("kretprobe/", KPROBE, 0, SEC_NONE, attach_kprobe), > SEC_DEF("uretprobe/", KPROBE, 0, SEC_NONE), > - SEC_DEF("classifier", SCHED_CLS, 0, SEC_NONE), > - SEC_DEF("action", SCHED_ACT, 0, SEC_NONE), > + SEC_DEF("classifier", SCHED_CLS, 0, SEC_SLOPPY_PFX), > + SEC_DEF("action", SCHED_ACT, 0, SEC_SLOPPY_PFX), > SEC_DEF("tracepoint/", TRACEPOINT, 0, SEC_NONE, attach_tp), > SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, attach_tp), > SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp), > @@ -8003,44 +8004,44 @@ static const struct bpf_sec_def section_defs[] = { > SEC_DEF("syscall", SYSCALL, 0, SEC_SLEEPABLE), > SEC_DEF("xdp_devmap/", XDP, BPF_XDP_DEVMAP, SEC_ATTACHABLE), > SEC_DEF("xdp_cpumap/", XDP, BPF_XDP_CPUMAP, SEC_ATTACHABLE), > - SEC_DEF("xdp", XDP, BPF_XDP, SEC_ATTACHABLE_OPT), > - SEC_DEF("perf_event", PERF_EVENT, 0, SEC_NONE), > - SEC_DEF("lwt_in", LWT_IN, 0, SEC_NONE), > - SEC_DEF("lwt_out", LWT_OUT, 0, SEC_NONE), > - SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_NONE), > - SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_NONE), > - SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, BPF_CGROUP_INET_INGRESS, > SEC_ATTACHABLE_OPT), > - SEC_DEF("cgroup_skb/egress", CGROUP_SKB, BPF_CGROUP_INET_EGRESS, > SEC_ATTACHABLE_OPT), > - SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_NONE), > - SEC_DEF("cgroup/sock_create", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/sock_release", CGROUP_SOCK, > BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE), > - SEC_DEF("cgroup/sock", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > SEC_ATTACHABLE_OPT), > - SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/dev", CGROUP_DEVICE, BPF_CGROUP_DEVICE, > SEC_ATTACHABLE_OPT), > - SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, SEC_ATTACHABLE_OPT), > - SEC_DEF("sk_skb/stream_parser", SK_SKB, BPF_SK_SKB_STREAM_PARSER, > SEC_ATTACHABLE_OPT), > - SEC_DEF("sk_skb/stream_verdict",SK_SKB, BPF_SK_SKB_STREAM_VERDICT, > SEC_ATTACHABLE_OPT), > - SEC_DEF("sk_skb", SK_SKB, 0, SEC_NONE), > - SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, SEC_ATTACHABLE_OPT), > - SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, SEC_ATTACHABLE_OPT), > - SEC_DEF("flow_dissector", FLOW_DISSECTOR, BPF_FLOW_DISSECTOR, > SEC_ATTACHABLE_OPT), > - SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE), > - SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE), > - SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE), > - SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE), > - SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, BPF_CGROUP_SYSCTL, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT, > SEC_ATTACHABLE), > - SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, > SEC_ATTACHABLE), > - SEC_DEF("struct_ops", STRUCT_OPS, 0, SEC_NONE), > + SEC_DEF("xdp", XDP, BPF_XDP, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("perf_event", PERF_EVENT, 0, SEC_SLOPPY_PFX), > + SEC_DEF("lwt_in", LWT_IN, 0, SEC_SLOPPY_PFX), > + SEC_DEF("lwt_out", LWT_OUT, 0, SEC_SLOPPY_PFX), > + SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_SLOPPY_PFX), > + SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_SLOPPY_PFX), > + SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, BPF_CGROUP_INET_INGRESS, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup_skb/egress", CGROUP_SKB, BPF_CGROUP_INET_EGRESS, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/sock_create", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/sock_release", CGROUP_SOCK, > BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/sock", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/dev", CGROUP_DEVICE, BPF_CGROUP_DEVICE, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, SEC_ATTACHABLE_OPT | > SEC_SLOPPY_PFX), > + SEC_DEF("sk_skb/stream_parser", SK_SKB, BPF_SK_SKB_STREAM_PARSER, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("sk_skb/stream_verdict",SK_SKB, BPF_SK_SKB_STREAM_VERDICT, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("sk_skb", SK_SKB, 0, SEC_SLOPPY_PFX), > + SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, SEC_ATTACHABLE_OPT | > SEC_SLOPPY_PFX), > + SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, SEC_ATTACHABLE_OPT | > SEC_SLOPPY_PFX), > + SEC_DEF("flow_dissector", FLOW_DISSECTOR, BPF_FLOW_DISSECTOR, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, BPF_CGROUP_SYSCTL, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("struct_ops+", STRUCT_OPS, 0, SEC_NONE), > SEC_DEF("sk_lookup/", SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE), > }; > @@ -8048,11 +8049,53 @@ static const struct bpf_sec_def section_defs[] = { > static const struct bpf_sec_def *find_sec_def(const char *sec_name) > { > - int i, n = ARRAY_SIZE(section_defs); > + const struct bpf_sec_def *sec_def; > + enum sec_def_flags sec_flags; > + int i, n = ARRAY_SIZE(section_defs), len; > + bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME; > for (i = 0; i < n; i++) { > - if (str_has_pfx(sec_name, section_defs[i].sec)) > - return §ion_defs[i]; > + sec_def = §ion_defs[i]; > + sec_flags = sec_def->cookie; > + len = strlen(sec_def->sec); > + > + /* "type/" always has to have proper SEC("type/extras") form */ > + if (sec_def->sec[len - 1] == '/') { > + if (str_has_pfx(sec_name, sec_def->sec)) > + return sec_def; > + continue; > + } > + > + /* "type+" means it can be either exact SEC("type") or > + * well-formed SEC("type/extras") with proper '/' separator > + */ > + if (sec_def->sec[len - 1] == '+') { > + len--; > + /* not even a prefix */ > + if (strncmp(sec_name, sec_def->sec, len) != 0) > + continue; > + /* exact match or has '/' separator */ > + if (sec_name[len] == '\0' || sec_name[len] == '/') > + return sec_def; > + continue; > + } > + > + /* SEC_SLOPPY_PFX definitions are allowed to be just prefix > + * matches, unless strict section name mode > + * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the > + * match has to be exact. > + */ > + if ((sec_flags & SEC_SLOPPY_PFX) && !strict) { > + if (str_has_pfx(sec_name, sec_def->sec)) > + return sec_def; > + continue; > + } > + > + /* Definitions not marked SEC_SLOPPY_PFX (e.g., > + * SEC("syscall")) are exact matches in both modes. > + */ > + if (strcmp(sec_name, sec_def->sec) == 0) > + return sec_def; > } > return NULL; > } > diff --git a/tools/lib/bpf/libbpf_legacy.h b/tools/lib/bpf/libbpf_legacy.h > index df0d03dcffab..74e6f860f703 100644 > --- a/tools/lib/bpf/libbpf_legacy.h > +++ b/tools/lib/bpf/libbpf_legacy.h > @@ -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, To clarify: I'm assuming, as discussed, we'll still support that old, non-conforming naming in libbpf 1.0, right? It just won't be enabled by default. Btw, forgot to update you, I've enabled LIBBPF_STRICT_DIRECT_ERRS and LIBBPF_STRICT_CLEAN_PTRS and everything seems to be working fine
On Fri, Sep 17, 2021 at 10:26 AM <sdf@google.com> wrote: > > On 09/16, Andrii Nakryiko wrote: > > Implement strict ELF section name handling for BPF programs. It utilizes > > `libbpf_set_strict_mode()` framework and adds new flag: > > LIBBPF_STRICT_SEC_NAME. > > > If this flag is set, libbpf will enforce exact section name matching for > > a lot of program types that previously allowed just partial prefix > > match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now > > in strict mode only SEC("xdp") will be accepted, which makes SEC("") > > definitions cleaner and more structured. SEC() now won't be used as yet > > another way to uniquely encode BPF program identifier (for that > > C function name is better and is guaranteed to be unique within > > bpf_object). Now SEC() is strictly BPF program type and, depending on > > program type, extra load/attach parameter specification. > > > Libbpf completely supports multiple BPF programs in the same ELF > > section, so multiple BPF programs of the same type/specification easily > > co-exist together within the same bpf_object scope. > > > Additionally, a new (for now internal) convention is introduced: section > > name that can be a stand-alone exact BPF program type specificator, but > > also could have extra parameters after '/' delimiter. An example of such > > section is "struct_ops", which can be specified by itself, but also > > allows to specify the intended operation to be attached to, e.g., > > "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not allowed. > > Such section definition is specified as "struct_ops+". > > > This change is part of libbpf 1.0 effort ([0], [1]). > > > [0] Closes: https://github.com/libbpf/libbpf/issues/271 > > [1] > > https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > --- > > tools/lib/bpf/libbpf.c | 135 ++++++++++++++++++++++------------ > > tools/lib/bpf/libbpf_legacy.h | 9 +++ > > 2 files changed, 98 insertions(+), 46 deletions(-) > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 56082865ceff..f0846f609e26 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -232,6 +232,7 @@ enum sec_def_flags { > > SEC_ATTACHABLE_OPT = SEC_ATTACHABLE | SEC_EXP_ATTACH_OPT, > > SEC_ATTACH_BTF = 4, > > SEC_SLEEPABLE = 8, > > + SEC_SLOPPY_PFX = 16, /* allow non-strict prefix matching */ > > }; > > > struct bpf_sec_def { > > @@ -7976,15 +7977,15 @@ static struct bpf_link *attach_lsm(const struct > > bpf_program *prog, long cookie); > > static struct bpf_link *attach_iter(const struct bpf_program *prog, long > > cookie); > > > static const struct bpf_sec_def section_defs[] = { > > - SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE), > > - SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, > > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE), > > - SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, > > SEC_ATTACHABLE), > > + SEC_DEF("socket", SOCKET_FILTER, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, > > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > SEC_DEF("kprobe/", KPROBE, 0, SEC_NONE, attach_kprobe), > > SEC_DEF("uprobe/", KPROBE, 0, SEC_NONE), > > SEC_DEF("kretprobe/", KPROBE, 0, SEC_NONE, attach_kprobe), > > SEC_DEF("uretprobe/", KPROBE, 0, SEC_NONE), > > - SEC_DEF("classifier", SCHED_CLS, 0, SEC_NONE), > > - SEC_DEF("action", SCHED_ACT, 0, SEC_NONE), > > + SEC_DEF("classifier", SCHED_CLS, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("action", SCHED_ACT, 0, SEC_SLOPPY_PFX), > > SEC_DEF("tracepoint/", TRACEPOINT, 0, SEC_NONE, attach_tp), > > SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, attach_tp), > > SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp), > > @@ -8003,44 +8004,44 @@ static const struct bpf_sec_def section_defs[] = { > > SEC_DEF("syscall", SYSCALL, 0, SEC_SLEEPABLE), > > SEC_DEF("xdp_devmap/", XDP, BPF_XDP_DEVMAP, SEC_ATTACHABLE), > > SEC_DEF("xdp_cpumap/", XDP, BPF_XDP_CPUMAP, SEC_ATTACHABLE), > > - SEC_DEF("xdp", XDP, BPF_XDP, SEC_ATTACHABLE_OPT), > > - SEC_DEF("perf_event", PERF_EVENT, 0, SEC_NONE), > > - SEC_DEF("lwt_in", LWT_IN, 0, SEC_NONE), > > - SEC_DEF("lwt_out", LWT_OUT, 0, SEC_NONE), > > - SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_NONE), > > - SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_NONE), > > - SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, BPF_CGROUP_INET_INGRESS, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("cgroup_skb/egress", CGROUP_SKB, BPF_CGROUP_INET_EGRESS, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_NONE), > > - SEC_DEF("cgroup/sock_create", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/sock_release", CGROUP_SOCK, > > BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE), > > - SEC_DEF("cgroup/sock", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/dev", CGROUP_DEVICE, BPF_CGROUP_DEVICE, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, SEC_ATTACHABLE_OPT), > > - SEC_DEF("sk_skb/stream_parser", SK_SKB, BPF_SK_SKB_STREAM_PARSER, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("sk_skb/stream_verdict",SK_SKB, BPF_SK_SKB_STREAM_VERDICT, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("sk_skb", SK_SKB, 0, SEC_NONE), > > - SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, SEC_ATTACHABLE_OPT), > > - SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, SEC_ATTACHABLE_OPT), > > - SEC_DEF("flow_dissector", FLOW_DISSECTOR, BPF_FLOW_DISSECTOR, > > SEC_ATTACHABLE_OPT), > > - SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE), > > - SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE), > > - SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE), > > - SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE), > > - SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, BPF_CGROUP_SYSCTL, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT, > > SEC_ATTACHABLE), > > - SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, > > SEC_ATTACHABLE), > > - SEC_DEF("struct_ops", STRUCT_OPS, 0, SEC_NONE), > > + SEC_DEF("xdp", XDP, BPF_XDP, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("perf_event", PERF_EVENT, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("lwt_in", LWT_IN, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("lwt_out", LWT_OUT, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, BPF_CGROUP_INET_INGRESS, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup_skb/egress", CGROUP_SKB, BPF_CGROUP_INET_EGRESS, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/sock_create", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/sock_release", CGROUP_SOCK, > > BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/sock", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/dev", CGROUP_DEVICE, BPF_CGROUP_DEVICE, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, SEC_ATTACHABLE_OPT | > > SEC_SLOPPY_PFX), > > + SEC_DEF("sk_skb/stream_parser", SK_SKB, BPF_SK_SKB_STREAM_PARSER, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("sk_skb/stream_verdict",SK_SKB, BPF_SK_SKB_STREAM_VERDICT, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("sk_skb", SK_SKB, 0, SEC_SLOPPY_PFX), > > + SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, SEC_ATTACHABLE_OPT | > > SEC_SLOPPY_PFX), > > + SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, SEC_ATTACHABLE_OPT | > > SEC_SLOPPY_PFX), > > + SEC_DEF("flow_dissector", FLOW_DISSECTOR, BPF_FLOW_DISSECTOR, > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, > > BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, BPF_CGROUP_SYSCTL, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > + SEC_DEF("struct_ops+", STRUCT_OPS, 0, SEC_NONE), > > SEC_DEF("sk_lookup/", SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE), > > }; > > > @@ -8048,11 +8049,53 @@ static const struct bpf_sec_def section_defs[] = { > > > static const struct bpf_sec_def *find_sec_def(const char *sec_name) > > { > > - int i, n = ARRAY_SIZE(section_defs); > > + const struct bpf_sec_def *sec_def; > > + enum sec_def_flags sec_flags; > > + int i, n = ARRAY_SIZE(section_defs), len; > > + bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME; > > > for (i = 0; i < n; i++) { > > - if (str_has_pfx(sec_name, section_defs[i].sec)) > > - return §ion_defs[i]; > > + sec_def = §ion_defs[i]; > > + sec_flags = sec_def->cookie; > > + len = strlen(sec_def->sec); > > + > > + /* "type/" always has to have proper SEC("type/extras") form */ > > + if (sec_def->sec[len - 1] == '/') { > > + if (str_has_pfx(sec_name, sec_def->sec)) > > + return sec_def; > > + continue; > > + } > > + > > + /* "type+" means it can be either exact SEC("type") or > > + * well-formed SEC("type/extras") with proper '/' separator > > + */ > > + if (sec_def->sec[len - 1] == '+') { > > + len--; > > + /* not even a prefix */ > > + if (strncmp(sec_name, sec_def->sec, len) != 0) > > + continue; > > + /* exact match or has '/' separator */ > > + if (sec_name[len] == '\0' || sec_name[len] == '/') > > + return sec_def; > > + continue; > > + } > > + > > + /* SEC_SLOPPY_PFX definitions are allowed to be just prefix > > + * matches, unless strict section name mode > > + * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the > > + * match has to be exact. > > + */ > > + if ((sec_flags & SEC_SLOPPY_PFX) && !strict) { > > + if (str_has_pfx(sec_name, sec_def->sec)) > > + return sec_def; > > + continue; > > + } > > + > > + /* Definitions not marked SEC_SLOPPY_PFX (e.g., > > + * SEC("syscall")) are exact matches in both modes. > > + */ > > + if (strcmp(sec_name, sec_def->sec) == 0) > > + return sec_def; > > } > > return NULL; > > } > > diff --git a/tools/lib/bpf/libbpf_legacy.h b/tools/lib/bpf/libbpf_legacy.h > > index df0d03dcffab..74e6f860f703 100644 > > --- a/tools/lib/bpf/libbpf_legacy.h > > +++ b/tools/lib/bpf/libbpf_legacy.h > > @@ -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, > > To clarify: I'm assuming, as discussed, we'll still support that old, > non-conforming naming in libbpf 1.0, right? It just won't be enabled > by default. No, we won't. All those opt-in strict flags will be turned on permanently in libbpf 1.0. But I'm adding an ability to provide custom callbacks to handle whatever (reasonable) BPF program section names. So if someone has a real important case needing custom handling, it's not a big problem to implement that logic on their own. If someone is just resisting making their code conforming, well... Stay on the old fixed version, write a callback, or just do the mechanical rename, how hard can that be? We are dropping bpf_program__find_program_by_title() in libbpf 1.0, that API is meaningless with multiple programs per section, so you'd have to update your logic to either skeleton or bpf_program__find_program_by_name() anyways. > > Btw, forgot to update you, I've enabled LIBBPF_STRICT_DIRECT_ERRS and > LIBBPF_STRICT_CLEAN_PTRS and everything seems to be working fine
On 09/17, Andrii Nakryiko wrote: > On Fri, Sep 17, 2021 at 10:26 AM <sdf@google.com> wrote: > > > > On 09/16, Andrii Nakryiko wrote: > > > Implement strict ELF section name handling for BPF programs. It > utilizes > > > `libbpf_set_strict_mode()` framework and adds new flag: > > > LIBBPF_STRICT_SEC_NAME. > > > > > If this flag is set, libbpf will enforce exact section name matching > for > > > a lot of program types that previously allowed just partial prefix > > > match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now > > > in strict mode only SEC("xdp") will be accepted, which makes SEC("") > > > definitions cleaner and more structured. SEC() now won't be used as > yet > > > another way to uniquely encode BPF program identifier (for that > > > C function name is better and is guaranteed to be unique within > > > bpf_object). Now SEC() is strictly BPF program type and, depending on > > > program type, extra load/attach parameter specification. > > > > > Libbpf completely supports multiple BPF programs in the same ELF > > > section, so multiple BPF programs of the same type/specification > easily > > > co-exist together within the same bpf_object scope. > > > > > Additionally, a new (for now internal) convention is introduced: > section > > > name that can be a stand-alone exact BPF program type specificator, > but > > > also could have extra parameters after '/' delimiter. An example of > such > > > section is "struct_ops", which can be specified by itself, but also > > > allows to specify the intended operation to be attached to, e.g., > > > "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not > allowed. > > > Such section definition is specified as "struct_ops+". > > > > > This change is part of libbpf 1.0 effort ([0], [1]). > > > > > [0] Closes: https://github.com/libbpf/libbpf/issues/271 > > > [1] > > > > https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > --- > > > tools/lib/bpf/libbpf.c | 135 > ++++++++++++++++++++++------------ > > > tools/lib/bpf/libbpf_legacy.h | 9 +++ > > > 2 files changed, 98 insertions(+), 46 deletions(-) > > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > > index 56082865ceff..f0846f609e26 100644 > > > --- a/tools/lib/bpf/libbpf.c > > > +++ b/tools/lib/bpf/libbpf.c > > > @@ -232,6 +232,7 @@ enum sec_def_flags { > > > SEC_ATTACHABLE_OPT = SEC_ATTACHABLE | SEC_EXP_ATTACH_OPT, > > > SEC_ATTACH_BTF = 4, > > > SEC_SLEEPABLE = 8, > > > + SEC_SLOPPY_PFX = 16, /* allow non-strict prefix matching */ > > > }; > > > > > struct bpf_sec_def { > > > @@ -7976,15 +7977,15 @@ static struct bpf_link *attach_lsm(const > struct > > > bpf_program *prog, long cookie); > > > static struct bpf_link *attach_iter(const struct bpf_program *prog, > long > > > cookie); > > > > > static const struct bpf_sec_def section_defs[] = { > > > - SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE), > > > - SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, > > > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE), > > > - SEC_DEF("sk_reuseport", SK_REUSEPORT, > BPF_SK_REUSEPORT_SELECT, > > > SEC_ATTACHABLE), > > > + SEC_DEF("socket", SOCKET_FILTER, 0, > SEC_SLOPPY_PFX), > > > + SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, > > > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("sk_reuseport", SK_REUSEPORT, > BPF_SK_REUSEPORT_SELECT, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > SEC_DEF("kprobe/", KPROBE, 0, SEC_NONE, > attach_kprobe), > > > SEC_DEF("uprobe/", KPROBE, 0, SEC_NONE), > > > SEC_DEF("kretprobe/", KPROBE, 0, SEC_NONE, > attach_kprobe), > > > SEC_DEF("uretprobe/", KPROBE, 0, SEC_NONE), > > > - SEC_DEF("classifier", SCHED_CLS, 0, SEC_NONE), > > > - SEC_DEF("action", SCHED_ACT, 0, SEC_NONE), > > > + SEC_DEF("classifier", SCHED_CLS, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("action", SCHED_ACT, 0, SEC_SLOPPY_PFX), > > > SEC_DEF("tracepoint/", TRACEPOINT, 0, SEC_NONE, > attach_tp), > > > SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, > attach_tp), > > > SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, > attach_raw_tp), > > > @@ -8003,44 +8004,44 @@ static const struct bpf_sec_def > section_defs[] = { > > > SEC_DEF("syscall", SYSCALL, 0, SEC_SLEEPABLE), > > > SEC_DEF("xdp_devmap/", XDP, BPF_XDP_DEVMAP, > SEC_ATTACHABLE), > > > SEC_DEF("xdp_cpumap/", XDP, BPF_XDP_CPUMAP, > SEC_ATTACHABLE), > > > - SEC_DEF("xdp", XDP, BPF_XDP, > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("perf_event", PERF_EVENT, 0, SEC_NONE), > > > - SEC_DEF("lwt_in", LWT_IN, 0, SEC_NONE), > > > - SEC_DEF("lwt_out", LWT_OUT, 0, SEC_NONE), > > > - SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_NONE), > > > - SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_NONE), > > > - SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, > BPF_CGROUP_INET_INGRESS, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("cgroup_skb/egress", CGROUP_SKB, > BPF_CGROUP_INET_EGRESS, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_NONE), > > > - SEC_DEF("cgroup/sock_create", CGROUP_SOCK, > BPF_CGROUP_INET_SOCK_CREATE, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/sock_release", CGROUP_SOCK, > > > BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/sock", CGROUP_SOCK, > BPF_CGROUP_INET_SOCK_CREATE, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, > BPF_CGROUP_INET4_POST_BIND, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, > BPF_CGROUP_INET6_POST_BIND, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/dev", CGROUP_DEVICE, > BPF_CGROUP_DEVICE, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("sk_skb/stream_parser", SK_SKB, > BPF_SK_SKB_STREAM_PARSER, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("sk_skb/stream_verdict",SK_SKB, > BPF_SK_SKB_STREAM_VERDICT, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("sk_skb", SK_SKB, 0, SEC_NONE), > > > - SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("flow_dissector", FLOW_DISSECTOR, > BPF_FLOW_DISSECTOR, > > > SEC_ATTACHABLE_OPT), > > > - SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_BIND, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_BIND, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_CONNECT, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_CONNECT, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP4_SENDMSG, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP6_SENDMSG, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP4_RECVMSG, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP6_RECVMSG, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, > BPF_CGROUP_SYSCTL, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, > BPF_CGROUP_GETSOCKOPT, > > > SEC_ATTACHABLE), > > > - SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, > BPF_CGROUP_SETSOCKOPT, > > > SEC_ATTACHABLE), > > > - SEC_DEF("struct_ops", STRUCT_OPS, 0, SEC_NONE), > > > + SEC_DEF("xdp", XDP, BPF_XDP, > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("perf_event", PERF_EVENT, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("lwt_in", LWT_IN, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("lwt_out", LWT_OUT, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, > SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, > BPF_CGROUP_INET_INGRESS, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup_skb/egress", CGROUP_SKB, > BPF_CGROUP_INET_EGRESS, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/sock_create", CGROUP_SOCK, > BPF_CGROUP_INET_SOCK_CREATE, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/sock_release", CGROUP_SOCK, > > > BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/sock", CGROUP_SOCK, > BPF_CGROUP_INET_SOCK_CREATE, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, > BPF_CGROUP_INET4_POST_BIND, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, > BPF_CGROUP_INET6_POST_BIND, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/dev", CGROUP_DEVICE, > BPF_CGROUP_DEVICE, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, > SEC_ATTACHABLE_OPT | > > > SEC_SLOPPY_PFX), > > > + SEC_DEF("sk_skb/stream_parser", SK_SKB, > BPF_SK_SKB_STREAM_PARSER, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("sk_skb/stream_verdict",SK_SKB, > BPF_SK_SKB_STREAM_VERDICT, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("sk_skb", SK_SKB, 0, SEC_SLOPPY_PFX), > > > + SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, > SEC_ATTACHABLE_OPT | > > > SEC_SLOPPY_PFX), > > > + SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, > SEC_ATTACHABLE_OPT | > > > SEC_SLOPPY_PFX), > > > + SEC_DEF("flow_dissector", FLOW_DISSECTOR, > BPF_FLOW_DISSECTOR, > > > SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_BIND, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_BIND, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET4_CONNECT, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, > BPF_CGROUP_INET6_CONNECT, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP4_SENDMSG, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP6_SENDMSG, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP4_RECVMSG, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, > BPF_CGROUP_UDP6_RECVMSG, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, > > > BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, > BPF_CGROUP_SYSCTL, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, > BPF_CGROUP_GETSOCKOPT, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, > BPF_CGROUP_SETSOCKOPT, > > > SEC_ATTACHABLE | SEC_SLOPPY_PFX), > > > + SEC_DEF("struct_ops+", STRUCT_OPS, 0, SEC_NONE), > > > SEC_DEF("sk_lookup/", SK_LOOKUP, BPF_SK_LOOKUP, > SEC_ATTACHABLE), > > > }; > > > > > @@ -8048,11 +8049,53 @@ static const struct bpf_sec_def > section_defs[] = { > > > > > static const struct bpf_sec_def *find_sec_def(const char *sec_name) > > > { > > > - int i, n = ARRAY_SIZE(section_defs); > > > + const struct bpf_sec_def *sec_def; > > > + enum sec_def_flags sec_flags; > > > + int i, n = ARRAY_SIZE(section_defs), len; > > > + bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME; > > > > > for (i = 0; i < n; i++) { > > > - if (str_has_pfx(sec_name, section_defs[i].sec)) > > > - return §ion_defs[i]; > > > + sec_def = §ion_defs[i]; > > > + sec_flags = sec_def->cookie; > > > + len = strlen(sec_def->sec); > > > + > > > + /* "type/" always has to have proper SEC("type/extras") > form */ > > > + if (sec_def->sec[len - 1] == '/') { > > > + if (str_has_pfx(sec_name, sec_def->sec)) > > > + return sec_def; > > > + continue; > > > + } > > > + > > > + /* "type+" means it can be either exact SEC("type") or > > > + * well-formed SEC("type/extras") with proper '/' > separator > > > + */ > > > + if (sec_def->sec[len - 1] == '+') { > > > + len--; > > > + /* not even a prefix */ > > > + if (strncmp(sec_name, sec_def->sec, len) != 0) > > > + continue; > > > + /* exact match or has '/' separator */ > > > + if (sec_name[len] == '\0' || sec_name[len] > == '/') > > > + return sec_def; > > > + continue; > > > + } > > > + > > > + /* SEC_SLOPPY_PFX definitions are allowed to be just > prefix > > > + * matches, unless strict section name mode > > > + * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case > the > > > + * match has to be exact. > > > + */ > > > + if ((sec_flags & SEC_SLOPPY_PFX) && !strict) { > > > + if (str_has_pfx(sec_name, sec_def->sec)) > > > + return sec_def; > > > + continue; > > > + } > > > + > > > + /* Definitions not marked SEC_SLOPPY_PFX (e.g., > > > + * SEC("syscall")) are exact matches in both modes. > > > + */ > > > + if (strcmp(sec_name, sec_def->sec) == 0) > > > + return sec_def; > > > } > > > return NULL; > > > } > > > diff --git a/tools/lib/bpf/libbpf_legacy.h > b/tools/lib/bpf/libbpf_legacy.h > > > index df0d03dcffab..74e6f860f703 100644 > > > --- a/tools/lib/bpf/libbpf_legacy.h > > > +++ b/tools/lib/bpf/libbpf_legacy.h > > > @@ -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, > > > > To clarify: I'm assuming, as discussed, we'll still support that old, > > non-conforming naming in libbpf 1.0, right? It just won't be enabled > > by default. > No, we won't. All those opt-in strict flags will be turned on > permanently in libbpf 1.0. But I'm adding an ability to provide custom > callbacks to handle whatever (reasonable) BPF program section names. > So if someone has a real important case needing custom handling, it's > not a big problem to implement that logic on their own. If someone is > just resisting making their code conforming, well... Stay on the old > fixed version, write a callback, or just do the mechanical rename, how > hard can that be? We are dropping bpf_program__find_program_by_title() > in libbpf 1.0, that API is meaningless with multiple programs per > section, so you'd have to update your logic to either skeleton or > bpf_program__find_program_by_name() anyways. I see. I was assuming some of them would stay, iirc Toke also was asking for this one to stay (or was it the old maps format?). FTR, I'm not resisting any changes, I'm willing to invest some time to update our callers, just trying to understand what my options are. We do have some cases where we depend on the section names, so maybe I should just switch from bpf_program__title to bpf_program__name (and do appropriate renaming). RE skeleton: I'm not too eager to adopt it, I'll wait for version 2 :-) > > > > Btw, forgot to update you, I've enabled LIBBPF_STRICT_DIRECT_ERRS and > > LIBBPF_STRICT_CLEAN_PTRS and everything seems to be working fine
On Fri, Sep 17, 2021 at 4:11 PM <sdf@google.com> wrote: > > On 09/17, Andrii Nakryiko wrote: > > On Fri, Sep 17, 2021 at 10:26 AM <sdf@google.com> wrote: > > > > > > On 09/16, Andrii Nakryiko wrote: > > > > Implement strict ELF section name handling for BPF programs. It > > utilizes > > > > `libbpf_set_strict_mode()` framework and adds new flag: > > > > LIBBPF_STRICT_SEC_NAME. > > > > > > > If this flag is set, libbpf will enforce exact section name matching > > for > > > > a lot of program types that previously allowed just partial prefix > > > > match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now > > > > in strict mode only SEC("xdp") will be accepted, which makes SEC("") > > > > definitions cleaner and more structured. SEC() now won't be used as > > yet > > > > another way to uniquely encode BPF program identifier (for that > > > > C function name is better and is guaranteed to be unique within > > > > bpf_object). Now SEC() is strictly BPF program type and, depending on > > > > program type, extra load/attach parameter specification. > > > > > > > Libbpf completely supports multiple BPF programs in the same ELF > > > > section, so multiple BPF programs of the same type/specification > > easily > > > > co-exist together within the same bpf_object scope. > > > > > > > Additionally, a new (for now internal) convention is introduced: > > section > > > > name that can be a stand-alone exact BPF program type specificator, > > but > > > > also could have extra parameters after '/' delimiter. An example of > > such > > > > section is "struct_ops", which can be specified by itself, but also > > > > allows to specify the intended operation to be attached to, e.g., > > > > "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not > > allowed. > > > > Such section definition is specified as "struct_ops+". > > > > > > > This change is part of libbpf 1.0 effort ([0], [1]). > > > > > > > [0] Closes: https://github.com/libbpf/libbpf/issues/271 > > > > [1] > > > > > > https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling > > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > > --- > > > > tools/lib/bpf/libbpf.c | 135 > > ++++++++++++++++++++++------------ > > > > tools/lib/bpf/libbpf_legacy.h | 9 +++ > > > > 2 files changed, 98 insertions(+), 46 deletions(-) > > > [...] > > > > + /* > > > > + * 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, > > > > > > To clarify: I'm assuming, as discussed, we'll still support that old, > > > non-conforming naming in libbpf 1.0, right? It just won't be enabled > > > by default. > > > No, we won't. All those opt-in strict flags will be turned on > > permanently in libbpf 1.0. But I'm adding an ability to provide custom > > callbacks to handle whatever (reasonable) BPF program section names. > > So if someone has a real important case needing custom handling, it's > > not a big problem to implement that logic on their own. If someone is > > just resisting making their code conforming, well... Stay on the old > > fixed version, write a callback, or just do the mechanical rename, how > > hard can that be? We are dropping bpf_program__find_program_by_title() > > in libbpf 1.0, that API is meaningless with multiple programs per > > section, so you'd have to update your logic to either skeleton or > > bpf_program__find_program_by_name() anyways. > > I see. I was assuming some of them would stay, iirc Toke also was asking > for this one to stay (or was it the old maps format?). FTR, I'm not > resisting any changes, I'm willing to invest some time to update our > callers, just trying to understand what my options are. We do have some > cases where we depend on the section names, so maybe I should just > switch from bpf_program__title to bpf_program__name (and do appropriate > renaming). Switching to name over title (section name) is a good idea for sure. > > RE skeleton: I'm not too eager to adopt it, I'll wait for version 2 :-) Honest curiosity, what's wrong with the current version of skeleton? Can you please expand on this? > > > > > > > > Btw, forgot to update you, I've enabled LIBBPF_STRICT_DIRECT_ERRS and > > > LIBBPF_STRICT_CLEAN_PTRS and everything seems to be working fine
On 09/17, Andrii Nakryiko wrote: > On Fri, Sep 17, 2021 at 4:11 PM <sdf@google.com> wrote: > > > > On 09/17, Andrii Nakryiko wrote: > > > On Fri, Sep 17, 2021 at 10:26 AM <sdf@google.com> wrote: > > > > > > > > On 09/16, Andrii Nakryiko wrote: > > > > > Implement strict ELF section name handling for BPF programs. It > > > utilizes > > > > > `libbpf_set_strict_mode()` framework and adds new flag: > > > > > LIBBPF_STRICT_SEC_NAME. > > > > > > > > > If this flag is set, libbpf will enforce exact section name > matching > > > for > > > > > a lot of program types that previously allowed just partial prefix > > > > > match. E.g., if previously SEC("xdp_whatever_i_want") was > allowed, now > > > > > in strict mode only SEC("xdp") will be accepted, which makes > SEC("") > > > > > definitions cleaner and more structured. SEC() now won't be used > as > > > yet > > > > > another way to uniquely encode BPF program identifier (for that > > > > > C function name is better and is guaranteed to be unique within > > > > > bpf_object). Now SEC() is strictly BPF program type and, > depending on > > > > > program type, extra load/attach parameter specification. > > > > > > > > > Libbpf completely supports multiple BPF programs in the same ELF > > > > > section, so multiple BPF programs of the same type/specification > > > easily > > > > > co-exist together within the same bpf_object scope. > > > > > > > > > Additionally, a new (for now internal) convention is introduced: > > > section > > > > > name that can be a stand-alone exact BPF program type > specificator, > > > but > > > > > also could have extra parameters after '/' delimiter. An example > of > > > such > > > > > section is "struct_ops", which can be specified by itself, but > also > > > > > allows to specify the intended operation to be attached to, e.g., > > > > > "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not > > > allowed. > > > > > Such section definition is specified as "struct_ops+". > > > > > > > > > This change is part of libbpf 1.0 effort ([0], [1]). > > > > > > > > > [0] Closes: https://github.com/libbpf/libbpf/issues/271 > > > > > [1] > > > > > > > > > https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling > > > > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > > > --- > > > > > tools/lib/bpf/libbpf.c | 135 > > > ++++++++++++++++++++++------------ > > > > > tools/lib/bpf/libbpf_legacy.h | 9 +++ > > > > > 2 files changed, 98 insertions(+), 46 deletions(-) > > > > > [...] > > > > > + /* > > > > > + * 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, > > > > > > > > To clarify: I'm assuming, as discussed, we'll still support that > old, > > > > non-conforming naming in libbpf 1.0, right? It just won't be enabled > > > > by default. > > > > > No, we won't. All those opt-in strict flags will be turned on > > > permanently in libbpf 1.0. But I'm adding an ability to provide custom > > > callbacks to handle whatever (reasonable) BPF program section names. > > > So if someone has a real important case needing custom handling, it's > > > not a big problem to implement that logic on their own. If someone is > > > just resisting making their code conforming, well... Stay on the old > > > fixed version, write a callback, or just do the mechanical rename, how > > > hard can that be? We are dropping bpf_program__find_program_by_title() > > > in libbpf 1.0, that API is meaningless with multiple programs per > > > section, so you'd have to update your logic to either skeleton or > > > bpf_program__find_program_by_name() anyways. > > > > I see. I was assuming some of them would stay, iirc Toke also was asking > > for this one to stay (or was it the old maps format?). FTR, I'm not > > resisting any changes, I'm willing to invest some time to update our > > callers, just trying to understand what my options are. We do have some > > cases where we depend on the section names, so maybe I should just > > switch from bpf_program__title to bpf_program__name (and do appropriate > > renaming). > Switching to name over title (section name) is a good idea for sure. > > > > RE skeleton: I'm not too eager to adopt it, I'll wait for version 2 :-) > Honest curiosity, what's wrong with the current version of skeleton? > Can you please expand on this? Nothing wrong, I'm just joking, but I guess we went with a different path long time ago where we ship .o more-or-less independently and it's hard to adopt skeletons at this point (where .o is bundled with the userspace part). We also ship several .o's compiled for different kernel versions because prior to BTF we didn't have a good way todo '#ifdef KERNEL_X_Y_Z' and some of that still lives on (but probably should die eventually). Maybe we should do some talk on some conference on the way we ship bpf to the fleet. Again, it's not like we don't like to change anything here, but there is some existing "legacy" infra that has some assumptions that might not easy to change in a short period of time.
On Fri, Sep 17, 2021 at 4:39 PM <sdf@google.com> wrote: > > On 09/17, Andrii Nakryiko wrote: > > On Fri, Sep 17, 2021 at 4:11 PM <sdf@google.com> wrote: > > > > > > On 09/17, Andrii Nakryiko wrote: > > > > On Fri, Sep 17, 2021 at 10:26 AM <sdf@google.com> wrote: > > > > > > > > > > On 09/16, Andrii Nakryiko wrote: > > > > > > Implement strict ELF section name handling for BPF programs. It > > > > utilizes > > > > > > `libbpf_set_strict_mode()` framework and adds new flag: > > > > > > LIBBPF_STRICT_SEC_NAME. > > > > > > > > > > > If this flag is set, libbpf will enforce exact section name > > matching > > > > for > > > > > > a lot of program types that previously allowed just partial prefix > > > > > > match. E.g., if previously SEC("xdp_whatever_i_want") was > > allowed, now > > > > > > in strict mode only SEC("xdp") will be accepted, which makes > > SEC("") > > > > > > definitions cleaner and more structured. SEC() now won't be used > > as > > > > yet > > > > > > another way to uniquely encode BPF program identifier (for that > > > > > > C function name is better and is guaranteed to be unique within > > > > > > bpf_object). Now SEC() is strictly BPF program type and, > > depending on > > > > > > program type, extra load/attach parameter specification. > > > > > > > > > > > Libbpf completely supports multiple BPF programs in the same ELF > > > > > > section, so multiple BPF programs of the same type/specification > > > > easily > > > > > > co-exist together within the same bpf_object scope. > > > > > > > > > > > Additionally, a new (for now internal) convention is introduced: > > > > section > > > > > > name that can be a stand-alone exact BPF program type > > specificator, > > > > but > > > > > > also could have extra parameters after '/' delimiter. An example > > of > > > > such > > > > > > section is "struct_ops", which can be specified by itself, but > > also > > > > > > allows to specify the intended operation to be attached to, e.g., > > > > > > "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not > > > > allowed. > > > > > > Such section definition is specified as "struct_ops+". > > > > > > > > > > > This change is part of libbpf 1.0 effort ([0], [1]). > > > > > > > > > > > [0] Closes: https://github.com/libbpf/libbpf/issues/271 > > > > > > [1] > > > > > > > > > > > > https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling > > > > > > > > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > > > > > --- > > > > > > tools/lib/bpf/libbpf.c | 135 > > > > ++++++++++++++++++++++------------ > > > > > > tools/lib/bpf/libbpf_legacy.h | 9 +++ > > > > > > 2 files changed, 98 insertions(+), 46 deletions(-) > > > > > > > > [...] > > > > > > > + /* > > > > > > + * 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, > > > > > > > > > > To clarify: I'm assuming, as discussed, we'll still support that > > old, > > > > > non-conforming naming in libbpf 1.0, right? It just won't be enabled > > > > > by default. > > > > > > > No, we won't. All those opt-in strict flags will be turned on > > > > permanently in libbpf 1.0. But I'm adding an ability to provide custom > > > > callbacks to handle whatever (reasonable) BPF program section names. > > > > So if someone has a real important case needing custom handling, it's > > > > not a big problem to implement that logic on their own. If someone is > > > > just resisting making their code conforming, well... Stay on the old > > > > fixed version, write a callback, or just do the mechanical rename, how > > > > hard can that be? We are dropping bpf_program__find_program_by_title() > > > > in libbpf 1.0, that API is meaningless with multiple programs per > > > > section, so you'd have to update your logic to either skeleton or > > > > bpf_program__find_program_by_name() anyways. > > > > > > I see. I was assuming some of them would stay, iirc Toke also was asking > > > for this one to stay (or was it the old maps format?). FTR, I'm not > > > resisting any changes, I'm willing to invest some time to update our > > > callers, just trying to understand what my options are. We do have some > > > cases where we depend on the section names, so maybe I should just > > > switch from bpf_program__title to bpf_program__name (and do appropriate > > > renaming). > > > Switching to name over title (section name) is a good idea for sure. > > > > > > > RE skeleton: I'm not too eager to adopt it, I'll wait for version 2 :-) > > > Honest curiosity, what's wrong with the current version of skeleton? > > Can you please expand on this? > > Nothing wrong, I'm just joking, but I guess we went with a different path > long time ago where we ship .o more-or-less independently and it's hard to > adopt skeletons at this point (where .o is bundled with the userspace part). > We also ship several .o's compiled for different kernel versions because > prior to BTF we didn't have a good way todo '#ifdef KERNEL_X_Y_Z' and > some of that still lives on (but probably should die eventually). We also have some applications with "old model", though they've used their custom embedding logic to not distribute separate .o files. We are working on switching them to BPF skeletons this half, though. Note that BPF skeleton generation logic recently added a way to get raw ELF bytes without necessarily instantiating the skeleton itself (see <xxx>__elf_bytes()). There are also plans to add small C++ constructor/destructor shims to make it harder for C++ apps to miss the need to destroy the skeleton, which helps with BPF skeleton adoption in C++ applications. > > Maybe we should do some talk on some conference on the way we ship > bpf to the fleet. Again, it's not like we don't like to change anything > here, but there is some existing "legacy" infra that has some > assumptions that might not easy to change in a short period of time. Sure, some sort of lighting talk would be interesting. The problem is that there needs to be a persistent and energetic initiative to break all assumptions and switch to newer (arguably, better) ways. Doesn't happen by itself ;)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 56082865ceff..f0846f609e26 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -232,6 +232,7 @@ enum sec_def_flags { SEC_ATTACHABLE_OPT = SEC_ATTACHABLE | SEC_EXP_ATTACH_OPT, SEC_ATTACH_BTF = 4, SEC_SLEEPABLE = 8, + SEC_SLOPPY_PFX = 16, /* allow non-strict prefix matching */ }; struct bpf_sec_def { @@ -7976,15 +7977,15 @@ static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie); static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie); static const struct bpf_sec_def section_defs[] = { - SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE), - SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE), - SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE), + SEC_DEF("socket", SOCKET_FILTER, 0, SEC_SLOPPY_PFX), + SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX), SEC_DEF("kprobe/", KPROBE, 0, SEC_NONE, attach_kprobe), SEC_DEF("uprobe/", KPROBE, 0, SEC_NONE), SEC_DEF("kretprobe/", KPROBE, 0, SEC_NONE, attach_kprobe), SEC_DEF("uretprobe/", KPROBE, 0, SEC_NONE), - SEC_DEF("classifier", SCHED_CLS, 0, SEC_NONE), - SEC_DEF("action", SCHED_ACT, 0, SEC_NONE), + SEC_DEF("classifier", SCHED_CLS, 0, SEC_SLOPPY_PFX), + SEC_DEF("action", SCHED_ACT, 0, SEC_SLOPPY_PFX), SEC_DEF("tracepoint/", TRACEPOINT, 0, SEC_NONE, attach_tp), SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, attach_tp), SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp), @@ -8003,44 +8004,44 @@ static const struct bpf_sec_def section_defs[] = { SEC_DEF("syscall", SYSCALL, 0, SEC_SLEEPABLE), SEC_DEF("xdp_devmap/", XDP, BPF_XDP_DEVMAP, SEC_ATTACHABLE), SEC_DEF("xdp_cpumap/", XDP, BPF_XDP_CPUMAP, SEC_ATTACHABLE), - SEC_DEF("xdp", XDP, BPF_XDP, SEC_ATTACHABLE_OPT), - SEC_DEF("perf_event", PERF_EVENT, 0, SEC_NONE), - SEC_DEF("lwt_in", LWT_IN, 0, SEC_NONE), - SEC_DEF("lwt_out", LWT_OUT, 0, SEC_NONE), - SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_NONE), - SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_NONE), - SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, BPF_CGROUP_INET_INGRESS, SEC_ATTACHABLE_OPT), - SEC_DEF("cgroup_skb/egress", CGROUP_SKB, BPF_CGROUP_INET_EGRESS, SEC_ATTACHABLE_OPT), - SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_NONE), - SEC_DEF("cgroup/sock_create", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, SEC_ATTACHABLE), - SEC_DEF("cgroup/sock_release", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE), - SEC_DEF("cgroup/sock", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, SEC_ATTACHABLE_OPT), - SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND, SEC_ATTACHABLE), - SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND, SEC_ATTACHABLE), - SEC_DEF("cgroup/dev", CGROUP_DEVICE, BPF_CGROUP_DEVICE, SEC_ATTACHABLE_OPT), - SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, SEC_ATTACHABLE_OPT), - SEC_DEF("sk_skb/stream_parser", SK_SKB, BPF_SK_SKB_STREAM_PARSER, SEC_ATTACHABLE_OPT), - SEC_DEF("sk_skb/stream_verdict",SK_SKB, BPF_SK_SKB_STREAM_VERDICT, SEC_ATTACHABLE_OPT), - SEC_DEF("sk_skb", SK_SKB, 0, SEC_NONE), - SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, SEC_ATTACHABLE_OPT), - SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, SEC_ATTACHABLE_OPT), - SEC_DEF("flow_dissector", FLOW_DISSECTOR, BPF_FLOW_DISSECTOR, SEC_ATTACHABLE_OPT), - SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND, SEC_ATTACHABLE), - SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND, SEC_ATTACHABLE), - SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT, SEC_ATTACHABLE), - SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT, SEC_ATTACHABLE), - SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG, SEC_ATTACHABLE), - SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG, SEC_ATTACHABLE), - SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG, SEC_ATTACHABLE), - SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG, SEC_ATTACHABLE), - SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE), - SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE), - SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE), - SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE), - SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, BPF_CGROUP_SYSCTL, SEC_ATTACHABLE), - SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT, SEC_ATTACHABLE), - SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, SEC_ATTACHABLE), - SEC_DEF("struct_ops", STRUCT_OPS, 0, SEC_NONE), + SEC_DEF("xdp", XDP, BPF_XDP, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("perf_event", PERF_EVENT, 0, SEC_SLOPPY_PFX), + SEC_DEF("lwt_in", LWT_IN, 0, SEC_SLOPPY_PFX), + SEC_DEF("lwt_out", LWT_OUT, 0, SEC_SLOPPY_PFX), + SEC_DEF("lwt_xmit", LWT_XMIT, 0, SEC_SLOPPY_PFX), + SEC_DEF("lwt_seg6local", LWT_SEG6LOCAL, 0, SEC_SLOPPY_PFX), + SEC_DEF("cgroup_skb/ingress", CGROUP_SKB, BPF_CGROUP_INET_INGRESS, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("cgroup_skb/egress", CGROUP_SKB, BPF_CGROUP_INET_EGRESS, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/skb", CGROUP_SKB, 0, SEC_SLOPPY_PFX), + SEC_DEF("cgroup/sock_create", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/sock_release", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_RELEASE, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/sock", CGROUP_SOCK, BPF_CGROUP_INET_SOCK_CREATE, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/post_bind4", CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/post_bind6", CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/dev", CGROUP_DEVICE, BPF_CGROUP_DEVICE, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("sockops", SOCK_OPS, BPF_CGROUP_SOCK_OPS, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("sk_skb/stream_parser", SK_SKB, BPF_SK_SKB_STREAM_PARSER, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("sk_skb/stream_verdict",SK_SKB, BPF_SK_SKB_STREAM_VERDICT, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("sk_skb", SK_SKB, 0, SEC_SLOPPY_PFX), + SEC_DEF("sk_msg", SK_MSG, BPF_SK_MSG_VERDICT, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("lirc_mode2", LIRC_MODE2, BPF_LIRC_MODE2, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("flow_dissector", FLOW_DISSECTOR, BPF_FLOW_DISSECTOR, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/bind4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/bind6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/connect4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/connect6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/sendmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/sendmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/recvmsg4", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/recvmsg6", CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/getpeername4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/getpeername6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_GETPEERNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/getsockname4", CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/getsockname6", CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_GETSOCKNAME, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/sysctl", CGROUP_SYSCTL, BPF_CGROUP_SYSCTL, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/getsockopt", CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, SEC_ATTACHABLE | SEC_SLOPPY_PFX), + SEC_DEF("struct_ops+", STRUCT_OPS, 0, SEC_NONE), SEC_DEF("sk_lookup/", SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE), }; @@ -8048,11 +8049,53 @@ static const struct bpf_sec_def section_defs[] = { static const struct bpf_sec_def *find_sec_def(const char *sec_name) { - int i, n = ARRAY_SIZE(section_defs); + const struct bpf_sec_def *sec_def; + enum sec_def_flags sec_flags; + int i, n = ARRAY_SIZE(section_defs), len; + bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME; for (i = 0; i < n; i++) { - if (str_has_pfx(sec_name, section_defs[i].sec)) - return §ion_defs[i]; + sec_def = §ion_defs[i]; + sec_flags = sec_def->cookie; + len = strlen(sec_def->sec); + + /* "type/" always has to have proper SEC("type/extras") form */ + if (sec_def->sec[len - 1] == '/') { + if (str_has_pfx(sec_name, sec_def->sec)) + return sec_def; + continue; + } + + /* "type+" means it can be either exact SEC("type") or + * well-formed SEC("type/extras") with proper '/' separator + */ + if (sec_def->sec[len - 1] == '+') { + len--; + /* not even a prefix */ + if (strncmp(sec_name, sec_def->sec, len) != 0) + continue; + /* exact match or has '/' separator */ + if (sec_name[len] == '\0' || sec_name[len] == '/') + return sec_def; + continue; + } + + /* SEC_SLOPPY_PFX definitions are allowed to be just prefix + * matches, unless strict section name mode + * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the + * match has to be exact. + */ + if ((sec_flags & SEC_SLOPPY_PFX) && !strict) { + if (str_has_pfx(sec_name, sec_def->sec)) + return sec_def; + continue; + } + + /* Definitions not marked SEC_SLOPPY_PFX (e.g., + * SEC("syscall")) are exact matches in both modes. + */ + if (strcmp(sec_name, sec_def->sec) == 0) + return sec_def; } return NULL; } diff --git a/tools/lib/bpf/libbpf_legacy.h b/tools/lib/bpf/libbpf_legacy.h index df0d03dcffab..74e6f860f703 100644 --- a/tools/lib/bpf/libbpf_legacy.h +++ b/tools/lib/bpf/libbpf_legacy.h @@ -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, };
Implement strict ELF section name handling for BPF programs. It utilizes `libbpf_set_strict_mode()` framework and adds new flag: LIBBPF_STRICT_SEC_NAME. If this flag is set, libbpf will enforce exact section name matching for a lot of program types that previously allowed just partial prefix match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now in strict mode only SEC("xdp") will be accepted, which makes SEC("") definitions cleaner and more structured. SEC() now won't be used as yet another way to uniquely encode BPF program identifier (for that C function name is better and is guaranteed to be unique within bpf_object). Now SEC() is strictly BPF program type and, depending on program type, extra load/attach parameter specification. Libbpf completely supports multiple BPF programs in the same ELF section, so multiple BPF programs of the same type/specification easily co-exist together within the same bpf_object scope. Additionally, a new (for now internal) convention is introduced: section name that can be a stand-alone exact BPF program type specificator, but also could have extra parameters after '/' delimiter. An example of such section is "struct_ops", which can be specified by itself, but also allows to specify the intended operation to be attached to, e.g., "struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not allowed. Such section definition is specified as "struct_ops+". This change is part of libbpf 1.0 effort ([0], [1]). [0] Closes: https://github.com/libbpf/libbpf/issues/271 [1] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/libbpf.c | 135 ++++++++++++++++++++++------------ tools/lib/bpf/libbpf_legacy.h | 9 +++ 2 files changed, 98 insertions(+), 46 deletions(-)