| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Dec 2018 10:18:55 +0000
From: Quentin Monnet <quentin.monnet@...ronome.com>
To: Daniel Borkmann <daniel@...earbox.net>,
Alexei Starovoitov <ast@...nel.org>
Cc: netdev@...r.kernel.org, oss-drivers@...ronome.com,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Jesper Dangaard Brouer <brouer@...hat.com>,
Stanislav Fomichev <sdf@...gle.com>
Subject: Re: [PATCH bpf-next 6/8] tools: bpftool: add probes for eBPF helper
functions
2018-12-16 00:57 UTC+0100 ~ Daniel Borkmann <daniel@...earbox.net>
> On 12/15/2018 04:32 AM, Quentin Monnet wrote:
>> 2018-12-15 01:08 UTC+0100 ~ Daniel Borkmann <daniel@...earbox.net>
>>> On 12/13/2018 01:19 PM, Quentin Monnet wrote:
>>>> Similarly to what was done for program types and map types, add a set of
>>>> probes to test the availability of the different eBPF helper functions
>>>> on the current system.
>>>>
>>>> Sample output:
>>>>
>>>> # bpftool feature probe kernel
>>>> ...
>>>> Scanning eBPF helper functions...
>>>> eBPF helper bpf_map_lookup_elem is available
>>>> eBPF helper bpf_map_update_elem is available
>>>> eBPF helper bpf_map_delete_elem is available
>>>> ...
>>>>
>>>> # bpftool --json --pretty feature probe kernel
>>>> {
>>>> ...
>>>> "helpers": {
>>>> "have_bpf_map_lookup_elem_helper": true,
>>>> "have_bpf_map_update_elem_helper": true,
>>>> "have_bpf_map_delete_elem_helper": true,
>>>> ...
>>>> }
>>>> }
>>>>
>>>> # bpftool feature probe kernel macros prefix BPFTOOL_
>>>> ...
>>>> /*** eBPF helper functions ***/
>>>> #define BPFTOOL_BPF_MAP_LOOKUP_ELEM_HELPER
>>>> #define BPFTOOL_BPF_MAP_UPDATE_ELEM_HELPER
>>>> #define BPFTOOL_BPF_MAP_DELETE_ELEM_HELPER
>>>
>>> Small nit: instead of BPFTOOL_ prefix, would it make sense
>>> to generally use Have_ prefix (similar to json output). The
>>> former seems somewhat bpftool related though it solely probes
>>> the underlying kernel.
>>
>> "BPFTOOL_" is provided on the command line ("prefix BPFTOOL_"), and it's
>> here just for the example. I initially had a non-configurable "HAVE_"
>> prefix instead, but after discussing it with Jakub we thought it best to
>> leave the choice of the prefix to the users, so they can choose what
>> works best for them, or adapt it and avoid any potential name conflict.
>
> Ah, good point, makes sense then. As default you have no prefix? Perhaps
> there it could be "HAVE_" prefix.
Yes, I can probably reinstate it as a default option if none was
provided from the command line, for prog/map types and helpers.
>>>> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
>>>> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>>>> ---
>>>> .../bpftool/Documentation/bpftool-feature.rst | 4 +
>>>> tools/bpf/bpftool/feature.c | 152 ++++++++++++++++++
>>>> 2 files changed, 156 insertions(+)
>>>>
>>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
>>>> index 23920a7490e9..083d30510cce 100644
>>>> --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
>>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
>>>> @@ -39,6 +39,10 @@ DESCRIPTION
>>>> names when including the output of this command as a header
>>>> file.
>>>>
>>>> + Note that when probed, some eBPF helpers (e.g.
>>>> + **bpf_trace_printk**\ () or **bpf_probe_write_user**\ ()) may
>>>> + print warnings to kernel logs.
>>>> +
>>>> **bpftool feature help**
>>>> Print short help message.
>>>>
>>>> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
>>>> index 85928f172413..77221fff6ba9 100644
>>>> --- a/tools/bpf/bpftool/feature.c
>>>> +++ b/tools/bpf/bpftool/feature.c
>>>> @@ -24,6 +24,113 @@ enum probe_component {
>>>> COMPONENT_KERNEL,
>>>> };
>>>>
>>>> +#define MAX_HELPER_NAME_LEN 32
>>>> +struct helper_param {
>>>> + enum bpf_prog_type progtype;
>>>> + const char name[MAX_HELPER_NAME_LEN];
>>>> +};
>>>> +
>>>> +/* helper_progtype_and_name[index] associates to the BPF helper function of id
>>>> + * "index" a name and a program type to run this helper with. In order to probe
>>>> + * helper availability for programs offloaded to a network device, use
>>>> + * offload-compatible types (e.g. XDP) everywhere we can. Caveats: helper
>>>> + * probing may fail currently if only TC (but not XDP) is supported for
>>>> + * offload.
>>>> + */
>>>> +static const struct helper_param helper_progtype_and_name[] = {
>>>> + { BPF_PROG_TYPE_XDP, "no_helper_with_id_0" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_map_lookup_elem" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_map_update_elem" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_map_delete_elem" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_probe_read" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_ktime_get_ns" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_trace_printk" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_get_prandom_u32" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_get_smp_processor_id" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_store_bytes" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_l3_csum_replace" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_l4_csum_replace" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_tail_call" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_clone_redirect" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_current_pid_tgid" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_current_uid_gid" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_current_comm" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_get_cgroup_classid" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_vlan_push" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_vlan_pop" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_get_tunnel_key" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_set_tunnel_key" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_perf_event_read" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_redirect" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_get_route_realm" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_perf_event_output" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_load_bytes" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_stackid" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_csum_diff" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_get_tunnel_opt" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_set_tunnel_opt" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_change_proto" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_change_type" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_under_cgroup" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_get_hash_recalc" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_current_task" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_probe_write_user" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_current_task_under_cgroup" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_change_tail" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_pull_data" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_csum_update" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_set_hash_invalid" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_get_numa_node_id" },
>>>> + { BPF_PROG_TYPE_SK_SKB, "bpf_skb_change_head" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_xdp_adjust_head" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_probe_read_str" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_get_socket_cookie" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_get_socket_uid" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_set_hash" },
>>>> + { BPF_PROG_TYPE_SOCK_OPS, "bpf_setsockopt" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_adjust_room" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_redirect_map" },
>>>> + { BPF_PROG_TYPE_SK_SKB, "bpf_sk_redirect_map" },
>>>> + { BPF_PROG_TYPE_SOCK_OPS, "bpf_sock_map_update" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_xdp_adjust_meta" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_perf_event_read_value" },
>>>> + { BPF_PROG_TYPE_PERF_EVENT, "bpf_perf_prog_read_value" },
>>>> + { BPF_PROG_TYPE_SOCK_OPS, "bpf_getsockopt" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_override_return" },
>>>> + { BPF_PROG_TYPE_SOCK_OPS, "bpf_sock_ops_cb_flags_set" },
>>>> + { BPF_PROG_TYPE_SK_MSG, "bpf_msg_redirect_map" },
>>>> + { BPF_PROG_TYPE_SK_MSG, "bpf_msg_apply_bytes" },
>>>> + { BPF_PROG_TYPE_SK_MSG, "bpf_msg_cork_bytes" },
>>>> + { BPF_PROG_TYPE_SK_MSG, "bpf_msg_pull_data" },
>>>> + { BPF_PROG_TYPE_CGROUP_SOCK_ADDR, "bpf_bind" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_xdp_adjust_tail" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_get_xfrm_state" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_stack" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_load_bytes_relative" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_fib_lookup" },
>>>> + { BPF_PROG_TYPE_SOCK_OPS, "bpf_sock_hash_update" },
>>>> + { BPF_PROG_TYPE_SK_MSG, "bpf_msg_redirect_hash" },
>>>> + { BPF_PROG_TYPE_SK_SKB, "bpf_sk_redirect_hash" },
>>>> + { BPF_PROG_TYPE_LWT_IN, "bpf_lwt_push_encap" },
>>>> + { BPF_PROG_TYPE_LWT_SEG6LOCAL, "bpf_lwt_seg6_store_bytes" },
>>>> + { BPF_PROG_TYPE_LWT_SEG6LOCAL, "bpf_lwt_seg6_adjust_srh" },
>>>> + { BPF_PROG_TYPE_LWT_SEG6LOCAL, "bpf_lwt_seg6_action" },
>>>> + { BPF_PROG_TYPE_LIRC_MODE2, "bpf_rc_repeat" },
>>>> + { BPF_PROG_TYPE_LIRC_MODE2, "bpf_rc_keydown" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_cgroup_id" },
>>>> + { BPF_PROG_TYPE_KPROBE, "bpf_get_current_cgroup_id" },
>>>> + { BPF_PROG_TYPE_CGROUP_SKB, "bpf_get_local_storage" },
>>>> + { BPF_PROG_TYPE_SK_REUSEPORT, "bpf_sk_select_reuseport" },
>>>> + { BPF_PROG_TYPE_SCHED_CLS, "bpf_skb_ancestor_cgroup_id" },
>>>> + { BPF_PROG_TYPE_SK_SKB, "bpf_sk_lookup_tcp" },
>>>> + { BPF_PROG_TYPE_SK_SKB, "bpf_sk_lookup_udp" },
>>>> + { BPF_PROG_TYPE_SK_SKB, "bpf_sk_release" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_map_push_elem" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_map_pop_elem" },
>>>> + { BPF_PROG_TYPE_XDP, "bpf_map_peek_elem" },
>>>> + { BPF_PROG_TYPE_SK_MSG, "bpf_msg_push_data" },
>>>
>>> Some of these helpers like bpf_perf_event_output are available on
>>> multiple types; should they all be tested in case the one probed
>>> prog type would not be available on the underlying kernel?
>>>
>>> Thanks,
>>> Daniel
>>
>> That's a good point, but it will make things more complicated. We would
>> need a list of compatible program types for each helper, and then try
>> them all until one works. I can do that, but I fear this will get huge
>> and hard to maintain.
>
> Hmm, true, one way to overcome this maintenance burden would be to try to
> brute-force all helpers over all prog types but obviously from verifier
> side it's more expensive. Perhaps worth testing whether this overhead would
> be acceptable. On the upside you'd have a listing of all supported helpers
> for each prog type.
Having the listing sounds nice. My concern is that the compatibility
list for a couple of helpers has been modified in the past (e.g.
csum_diff added to XDP), and I fear such changes will be easy to miss.
Brute-forcing all programs might be doable (programs are only 2-insn
long, no jumps) but seems less clean. I can have a look at it. Is there
any particular metric I should focus on?
Naive question, just in case: I don't suppose it is desirable to make
the list of supported helpers for each program type accessible to user
space? (I guess it would come down to implement those probes as a new
bpf() command, which has been rejected before if I remember correctly?)
Powered by blists - more mailing lists