linux-kernel - Re: [PATCH perf/core 1/6] tools lib bpf: Fix map offsets in relocation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ab6cd89a-6bb6-6c5d-45c5-b25bdb493a5f@huawei.com>
Date:   Thu, 19 Jan 2017 14:07:40 +0800
From:   "Wangnan (F)" <wangnan0@...wei.com>
To:     Joe Stringer <joe@....org>, <linux-kernel@...r.kernel.org>
CC:     <ast@...com>, <daniel@...earbox.net>, <acme@...nel.org>
Subject: Re: [PATCH perf/core 1/6] tools lib bpf: Fix map offsets in
 relocation



On 2017/1/19 7:57, Joe Stringer wrote:
> Commit 4708bbda5cb2 ("tools lib bpf: Fix maps resolution") attempted to
> fix map resolution by identifying the number of symbols that point to
> maps, and using this number to resolve each of the maps.
>
> However, during relocation the original definition of the map size was
> still in use.


Oops... Didn't realize that.


>   For up to two maps, the calculation was correct if there
> was a small difference in size between the map definition in libbpf and
> the one that the client library uses. However if the difference was
> large, particularly if more than two maps were used in the BPF program,
> the relocation would fail.
>
> For example, when using a map definition with size 28, with three maps,
> map relocation would count
>      (sym_offset / sizeof(struct bpf_map_def) => map_idx)
>      (0 / 16 => 0), ie map_idx = 0
>      (28 / 16 => 1), ie map_idx = 1
>      (56 / 16 => 3), ie map_idx = 3
>
> So, libbpf reports:
>      libbpf: bpf relocation: map_idx 3 large than 2

Understand.

> Fix map relocation by tracking the size of the map definition during
> map counting, then reuse this instead of the size of the libbpf map
> structure.

Then we add a restriction that map size is fixed
in one object, but we don't have to. During relocating,
we can get exact map positioning information from
sym.st_value. I think finding a map with exact offset
from the map array would be better.

I'll write a patch to replace this one.

Thank you.

>   With this patch applied, libbpf will accept such ELFs.
>
> Fixes: 4708bbda5cb2 ("tools lib bpf: Fix maps resolution")
> Signed-off-by: Joe Stringer <joe@....org>
> ---
>   tools/lib/bpf/libbpf.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 84e6b35da4bd..350ee4c59f85 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -201,6 +201,7 @@ struct bpf_object {
>   	size_t nr_programs;
>   	struct bpf_map *maps;
>   	size_t nr_maps;
> +	size_t map_len;
>   
>   	bool loaded;
>   
> @@ -379,6 +380,7 @@ static struct bpf_object *bpf_object__new(const char *path,
>   	obj->efile.maps_shndx = -1;
>   
>   	obj->loaded = false;
> +	obj->map_len = sizeof(struct bpf_map_def);
>   
>   	INIT_LIST_HEAD(&obj->list);
>   	list_add(&obj->list, &bpf_objects_list);
> @@ -602,6 +604,7 @@ bpf_object__init_maps(struct bpf_object *obj)
>   		return -ENOMEM;
>   	}
>   	obj->nr_maps = nr_maps;
> +	obj->map_len = data->d_size / nr_maps;
>   
>   	/*
>   	 * fill all fd with -1 so won't close incorrect
> @@ -829,7 +832,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
>   			return -LIBBPF_ERRNO__RELOC;
>   		}
>   
> -		map_idx = sym.st_value / sizeof(struct bpf_map_def);
> +		map_idx = sym.st_value / prog->obj->map_len;
>   		if (map_idx >= nr_maps) {
>   			pr_warning("bpf relocation: map_idx %d large than %d\n",
>   				   (int)map_idx, (int)nr_maps - 1);