Skip to content

Commit

Permalink
elf: detect map definitions with static qualifier
Browse files Browse the repository at this point in the history
The following map definition

    static struct bpf_map_def foo __section("maps") = {...};

makes clang emit a relocation that points at the maps section, not
at the foo symbol (even though foo is still present in the symbol
table). The library currently doesn't recognize this and tries to
find a map named "maps" at load time, which is pretty confusing.

Try to detect the mistake by refusing direct map loads from the
"maps" and ".maps" (for BTF-style definitions) sections. They will
never succeed anyways.
  • Loading branch information
lmb committed Nov 27, 2020
1 parent 6581b5d commit 0f7e513
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 5 deletions.
12 changes: 9 additions & 3 deletions elf_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,16 @@ outer:
// that goes via the helper. They are distinguished by
// different relocations.
switch typ {
// This is a direct load since the referenced symbol is a
// section. Weirdly, the offset of the real symbol in the
// section is encoded in the instruction stream.
case elf.STT_SECTION:
// This is a direct load since the referenced symbol is a
// section. Weirdly, the offset of the real symbol in the
// section is encoded in the instruction stream.
if name == "maps" || name == ".maps" {
// A direct load from the map definition sections doesn't make
// sense, this usually happens due to a stray `static`.
return fmt.Errorf("possible erroneous static qualifier on map definition: found reference to section %s", name)
}

if bind != elf.STB_LOCAL {
return fmt.Errorf("direct load: %s: unsupported relocation %s", name, bind)
}
Expand Down
10 changes: 10 additions & 0 deletions elf_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ func TestLoadInvalidMap(t *testing.T) {
})
}

func TestLoadInvalidMapMissingSymbol(t *testing.T) {
testutils.TestFiles(t, "testdata/invalid_map_static-el.elf", func(t *testing.T, file string) {
_, err := LoadCollectionSpec(file)
t.Log(err)
if err == nil {
t.Fatal("Loading a map with static qualifier should fail")
}
})
}

func TestLoadRawTracepoint(t *testing.T) {
testutils.SkipOnOldKernel(t, "4.17", "BPF_RAW_TRACEPOINT API")

Expand Down
4 changes: 2 additions & 2 deletions testdata/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ CLANG ?= $(LLVM_PREFIX)/clang
CFLAGS := -target bpf -O2 -g -Wall -Werror $(CFLAGS)

.PHONY: all clean
all: loader-clang-6.0-el.elf loader-clang-7-el.elf loader-clang-8-el.elf loader-clang-9-el.elf invalid_map-el.elf raw_tracepoint-el.elf \
loader-clang-6.0-eb.elf loader-clang-7-eb.elf loader-clang-8-eb.elf loader-clang-9-eb.elf invalid_map-eb.elf raw_tracepoint-eb.elf
all: loader-clang-6.0-el.elf loader-clang-7-el.elf loader-clang-8-el.elf loader-clang-9-el.elf invalid_map-el.elf raw_tracepoint-el.elf invalid_map_static-el.elf \
loader-clang-6.0-eb.elf loader-clang-7-eb.elf loader-clang-8-eb.elf loader-clang-9-eb.elf invalid_map-eb.elf raw_tracepoint-eb.elf invalid_map_static-eb.elf

clean:
-$(RM) *.elf
Expand Down
Binary file added testdata/invalid_map_static-eb.elf
Binary file not shown.
Binary file added testdata/invalid_map_static-el.elf
Binary file not shown.
26 changes: 26 additions & 0 deletions testdata/invalid_map_static.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* This file excercises the ELF loader. It is not a valid BPF program. */

#include "common.h"

struct bpf_map_def dummy __section("maps") = {
.type = BPF_MAP_TYPE_HASH,
.key_size = sizeof(uint32_t),
.value_size = sizeof(uint64_t),
.max_entries = 1,
.map_flags = 0,
};

/* The static qualifier leads to clang not emitting a symbol. */
static struct bpf_map_def hash_map __section("maps") = {
.type = BPF_MAP_TYPE_HASH,
.key_size = sizeof(uint32_t),
.value_size = sizeof(uint64_t),
.max_entries = 1,
.map_flags = 0,
};

__section("xdp") int xdp_prog() {
uint32_t key = 0;
void *p = map_lookup_elem(&hash_map, &key);
return !!p;
}

0 comments on commit 0f7e513

Please sign in to comment.