On Mon, Nov 19, 2018 at 5:28 PM Quentin Monnet
<quentin.mon...@netronome.com> wrote:
>
> In order to compare BPF map symbol type correctly in regard to the
> latest LLVM, commit 7a04dd84a7f9 ("bpf: check map symbol type properly
> with newer llvm compiler") compares map symbol type to both NOTYPE and
> OBJECT. To do so, it first retrieves the type from "sym.st_info" and
> stores it into a temporary variable.
>
> However, the type is collected from the symbol "sym" before this latter
> symbol is actually updated. gelf_getsym() is called after that and
> updates "sym", and when comparison with OBJECT or NOTYPE happens it is
> done on the type of the symbol collected in the previous passage of the
> loop (or on an uninitialised symbol on the first passage). This may
> eventually break map collection from the ELF file.
>
> Fix this by assigning the type to the temporary variable only after the
> call to gelf_getsym().
>
> Fixes: 7a04dd84a7f9 ("bpf: check map symbol type properly with newer llvm
> compiler")
> Reported-by: Ron Philip <ron.phi...@netronome.com>
> Signed-off-by: Quentin Monnet <quentin.mon...@netronome.com>
> Reviewed-by: Jiong Wang <jiong.w...@netronome.com>
Thanks for the fix!
Acked-by: Yonghong Song <y...@fb.com>
> ---
> lib/bpf.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/bpf.c b/lib/bpf.c
> index 45f279fa4a41..6aff8f7bad7f 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -1758,11 +1758,12 @@ static const char *bpf_map_fetch_name(struct
> bpf_elf_ctx *ctx, int which)
> int i;
>
> for (i = 0; i < ctx->sym_num; i++) {
> - int type = GELF_ST_TYPE(sym.st_info);
> + int type;
>
> if (gelf_getsym(ctx->sym_tab, i, &sym) != &sym)
> continue;
>
> + type = GELF_ST_TYPE(sym.st_info);
> if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
> (type != STT_NOTYPE && type != STT_OBJECT) ||
> sym.st_shndx != ctx->sec_maps ||
> @@ -1851,11 +1852,12 @@ static int bpf_map_num_sym(struct bpf_elf_ctx *ctx)
> GElf_Sym sym;
>
> for (i = 0; i < ctx->sym_num; i++) {
> - int type = GELF_ST_TYPE(sym.st_info);
> + int type;
>
> if (gelf_getsym(ctx->sym_tab, i, &sym) != &sym)
> continue;
>
> + type = GELF_ST_TYPE(sym.st_info);
> if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
> (type != STT_NOTYPE && type != STT_OBJECT) ||
> sym.st_shndx != ctx->sec_maps)
> @@ -1931,10 +1933,12 @@ static int bpf_map_verify_all_offs(struct bpf_elf_ctx
> *ctx, int end)
> * the table again.
> */
> for (i = 0; i < ctx->sym_num; i++) {
> - int type = GELF_ST_TYPE(sym.st_info);
> + int type;
>
> if (gelf_getsym(ctx->sym_tab, i, &sym) != &sym)
> continue;
> +
> + type = GELF_ST_TYPE(sym.st_info);
> if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
> (type != STT_NOTYPE && type != STT_OBJECT) ||
> sym.st_shndx != ctx->sec_maps)
> --
> 2.7.4
>
