| 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, 23 Jul 2018 11:41:12 -0700
From: Martin KaFai Lau <kafai@...com>
To: Yonghong Song <yhs@...com>
CC: <netdev@...r.kernel.org>, Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>, <kernel-team@...com>
Subject: Re: [PATCH v2 bpf 3/3] bpf: Introduce BPF_ANNOTATE_KV_PAIR
On Mon, Jul 23, 2018 at 11:31:43AM -0700, Yonghong Song wrote:
>
>
> On 7/21/18 11:20 AM, Martin KaFai Lau wrote:
> > This patch introduces BPF_ANNOTATE_KV_PAIR to signal the
> > bpf loader about the btf key_type and value_type of a bpf map.
> > Please refer to the changes in test_btf_haskv.c for its usage.
> > Both iproute2 and libbpf loader will then have the same
> > convention to find out the map's btf_key_type_id and
> > btf_value_type_id from a map's name.
> >
> > Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
> > Suggested-by: Daniel Borkmann <daniel@...earbox.net>
> > Signed-off-by: Martin KaFai Lau <kafai@...com>
> > ---
> > tools/lib/bpf/btf.c | 7 +-
> > tools/lib/bpf/btf.h | 2 +
> > tools/lib/bpf/libbpf.c | 75 +++++++++++---------
> > tools/testing/selftests/bpf/bpf_helpers.h | 9 +++
> > tools/testing/selftests/bpf/test_btf_haskv.c | 7 +-
> > 5 files changed, 56 insertions(+), 44 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index ce77b5b57912..321a99e648ed 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -189,8 +189,7 @@ static int btf_parse_type_sec(struct btf *btf, btf_print_fn_t err_log)
> > return 0;
> > }
> > -static const struct btf_type *btf_type_by_id(const struct btf *btf,
> > - __u32 type_id)
> > +const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 type_id)
> > {
> > if (type_id > btf->nr_types)
> > return NULL;
> > @@ -233,7 +232,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
> > __s64 size = -1;
> > int i;
> > - t = btf_type_by_id(btf, type_id);
> > + t = btf__type_by_id(btf, type_id);
> > for (i = 0; i < MAX_RESOLVE_DEPTH && !btf_type_is_void_or_null(t);
> > i++) {
> > size = btf_type_size(t);
> > @@ -258,7 +257,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
> > return -EINVAL;
> > }
> > - t = btf_type_by_id(btf, type_id);
> > + t = btf__type_by_id(btf, type_id);
> > }
> > if (size < 0)
> > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > index ed3a84370ccc..e2a09a155f84 100644
> > --- a/tools/lib/bpf/btf.h
> > +++ b/tools/lib/bpf/btf.h
> > @@ -9,6 +9,7 @@
> > #define BTF_ELF_SEC ".BTF"
> > struct btf;
> > +struct btf_type;
> > typedef int (*btf_print_fn_t)(const char *, ...)
> > __attribute__((format(printf, 1, 2)));
> > @@ -16,6 +17,7 @@ typedef int (*btf_print_fn_t)(const char *, ...)
> > void btf__free(struct btf *btf);
> > struct btf *btf__new(__u8 *data, __u32 size, btf_print_fn_t err_log);
> > __s32 btf__find_by_name(const struct btf *btf, const char *type_name);
> > +const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 id);
> > __s64 btf__resolve_size(const struct btf *btf, __u32 type_id);
> > int btf__fd(const struct btf *btf);
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6deb4fe4fffe..d881d370616c 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -36,6 +36,7 @@
> > #include <linux/err.h>
> > #include <linux/kernel.h>
> > #include <linux/bpf.h>
> > +#include <linux/btf.h>
> > #include <linux/list.h>
> > #include <linux/limits.h>
> > #include <sys/stat.h>
> > @@ -1014,68 +1015,72 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
> > static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf)
> > {
> > + const struct btf_type *container_type;
> > + const struct btf_member *key, *value;
> > struct bpf_map_def *def = &map->def;
> > const size_t max_name = 256;
> > + char container_name[max_name];
> > __s64 key_size, value_size;
> > - __s32 key_id, value_id;
> > - char name[max_name];
> > + __s32 container_id;
> > - /* Find key type by name from BTF */
> > - if (snprintf(name, max_name, "%s_key", map->name) == max_name) {
> > - pr_warning("map:%s length of BTF key_type:%s_key is too long\n",
> > + if (snprintf(container_name, max_name, "____btf_map_%s", map->name) ==
> > + max_name) {
> > + pr_warning("map:%s length of '____btf_map_%s' is too long\n",
> > map->name, map->name);
> > return -EINVAL;
> > }
> > - key_id = btf__find_by_name(btf, name);
> > - if (key_id < 0) {
> > - pr_debug("map:%s key_type:%s cannot be found in BTF\n",
> > - map->name, name);
> > - return key_id;
> > + container_id = btf__find_by_name(btf, container_name);
> > + if (container_id < 0) {
> > + pr_debug("map:%s container_name:%s cannot be found in BTF. Missing BPF_ANNOTATE_KV_PAIR?\n",
> > + map->name, container_name);
> > + return container_id;
> > }
> > - key_size = btf__resolve_size(btf, key_id);
> > - if (key_size < 0) {
> > - pr_warning("map:%s key_type:%s cannot get the BTF type_size\n",
> > - map->name, name);
> > - return key_size;
> > + container_type = btf__type_by_id(btf, container_id);
> > + if (!container_type) {
> > + pr_warning("map:%s cannot find BTF type for container_id:%u\n",
> > + map->name, container_id);
> > + return -EINVAL;
> > }
> > - if (def->key_size != key_size) {
> > - pr_warning("map:%s key_type:%s has BTF type_size:%u != key_size:%u\n",
> > - map->name, name, (unsigned int)key_size, def->key_size);
> > + if (BTF_INFO_KIND(container_type->info) != BTF_KIND_STRUCT ||
> > + BTF_INFO_VLEN(container_type->info) < 2) {
>
> Should "BTF_INFO_VLEN(container_type->info) < 2" be
> "BTF_INFO_VLEN(container_type->info) != 2"?
I intentionally use "<" in case we may want to extend
BPF_ANNOTATE_KV_PAIR in the future.
>
> > + pr_warning("map:%s container_name:%s is an invalid container struct\n",
> > + map->name, container_name);
> > return -EINVAL;
> > }
> > - /* Find value type from BTF */
> > - if (snprintf(name, max_name, "%s_value", map->name) == max_name) {
> > - pr_warning("map:%s length of BTF value_type:%s_value is too long\n",
> > - map->name, map->name);
> > - return -EINVAL;
> > + key = (struct btf_member *)(container_type + 1);
> > + value = key + 1;
> > +
> > + key_size = btf__resolve_size(btf, key->type);
> > + if (key_size < 0) {
> > + pr_warning("map:%s invalid BTF key_type_size\n",
> > + map->name);
> > + return key_size;
> > }
> > - value_id = btf__find_by_name(btf, name);
> > - if (value_id < 0) {
> > - pr_debug("map:%s value_type:%s cannot be found in BTF\n",
> > - map->name, name);
> > - return value_id;
> > + if (def->key_size != key_size) {
> > + pr_warning("map:%s btf_key_type_size:%u != map_def_key_size:%u\n",
> > + map->name, (__u32)key_size, def->key_size);
> > + return -EINVAL;
> > }
> > - value_size = btf__resolve_size(btf, value_id);
> > + value_size = btf__resolve_size(btf, value->type);
> > if (value_size < 0) {
> > - pr_warning("map:%s value_type:%s cannot get the BTF type_size\n",
> > - map->name, name);
> > + pr_warning("map:%s invalid BTF value_type_size\n", map->name);
> > return value_size;
> > }
> > if (def->value_size != value_size) {
> > - pr_warning("map:%s value_type:%s has BTF type_size:%u != value_size:%u\n",
> > - map->name, name, (unsigned int)value_size, def->value_size);
> > + pr_warning("map:%s btf_value_type_size:%u != map_def_value_size:%u\n",
> > + map->name, (__u32)value_size, def->value_size);
> > return -EINVAL;
> > }
> > - map->btf_key_type_id = key_id;
> > - map->btf_value_type_id = value_id;
> > + map->btf_key_type_id = key->type;
> > + map->btf_value_type_id = value->type;
> > return 0;
> > }
> > diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> > index f2f28b6c8915..810de20e8e26 100644
> > --- a/tools/testing/selftests/bpf/bpf_helpers.h
> > +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> > @@ -158,6 +158,15 @@ struct bpf_map_def {
> > unsigned int numa_node;
> > };
> > +#define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val) \
> > + struct ____btf_map_##name { \
> > + type_key key; \
> > + type_val value; \
> > + }; \
> > + struct ____btf_map_##name \
> > + __attribute__ ((section(".maps." #name), used)) \
> > + ____btf_map_##name = { }
> > +
> > static int (*bpf_skb_load_bytes)(void *ctx, int off, void *to, int len) =
> > (void *) BPF_FUNC_skb_load_bytes;
> > static int (*bpf_skb_store_bytes)(void *ctx, int off, void *from, int len, int flags) =
> > diff --git a/tools/testing/selftests/bpf/test_btf_haskv.c b/tools/testing/selftests/bpf/test_btf_haskv.c
> > index 8c7ca096ecf2..b21b876f475d 100644
> > --- a/tools/testing/selftests/bpf/test_btf_haskv.c
> > +++ b/tools/testing/selftests/bpf/test_btf_haskv.c
> > @@ -10,11 +10,6 @@ struct ipv_counts {
> > unsigned int v6;
> > };
> > -typedef int btf_map_key;
> > -typedef struct ipv_counts btf_map_value;
> > -btf_map_key dumm_key;
> > -btf_map_value dummy_value;
> > -
> > struct bpf_map_def SEC("maps") btf_map = {
> > .type = BPF_MAP_TYPE_ARRAY,
> > .key_size = sizeof(int),
> > @@ -22,6 +17,8 @@ struct bpf_map_def SEC("maps") btf_map = {
> > .max_entries = 4,
> > };
> > +BPF_ANNOTATE_KV_PAIR(btf_map, int, struct ipv_counts);
> > +
> > struct dummy_tracepoint_args {
> > unsigned long long pad;
> > struct sock *sock;
> >
Powered by blists - more mailing lists