Skip to content

Commit

Permalink
libbpf: enforce strict libbpf 1.0 behaviors
Browse files Browse the repository at this point in the history 
Remove support for legacy features and behaviors that previously had to
be disabled by calling libbpf_set_strict_mode():
  - legacy BPF map definitions are not supported now;
  - RLIMIT_MEMLOCK auto-setting, if necessary, is always on (but see
    libbpf_set_memlock_rlim());
  - program name is used for program pinning (instead of section name);
  - cleaned up error returning logic;
  - entry BPF programs should have SEC() always.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20220627211527.2245459-15-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Andrii Nakryiko authored and Alexei Starovoitov committed Jun 28, 2022
1 parent 31e4272 commit bd05410
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 261 deletions.
4 changes: 0 additions & 4 deletions tools/lib/bpf/bpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,6 @@ int bump_rlimit_memlock(void)
{
struct rlimit rlim;

/* this the default in libbpf 1.0, but for now user has to opt-in explicitly */
if (!(libbpf_mode & LIBBPF_STRICT_AUTO_RLIMIT_MEMLOCK))
return 0;

/* if kernel supports memcg-based accounting, skip bumping RLIMIT_MEMLOCK */
if (memlock_bumped || kernel_supports(NULL, FEAT_MEMCG_ACCOUNT))
return 0;
Expand Down
212 changes: 9 additions & 203 deletions tools/lib/bpf/libbpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,9 @@ static inline __u64 ptr_to_u64(const void *ptr)
return (__u64) (unsigned long) ptr;
}

/* this goes away in libbpf 1.0 */
enum libbpf_strict_mode libbpf_mode = LIBBPF_STRICT_NONE;

int libbpf_set_strict_mode(enum libbpf_strict_mode mode)
{
libbpf_mode = mode;
/* as of v1.0 libbpf_set_strict_mode() is a no-op */
return 0;
}

Expand Down Expand Up @@ -367,9 +364,10 @@ struct bpf_sec_def {
* linux/filter.h.
*/
struct bpf_program {
const struct bpf_sec_def *sec_def;
char *name;
char *sec_name;
size_t sec_idx;
const struct bpf_sec_def *sec_def;
/* this program's instruction offset (in number of instructions)
* within its containing ELF section
*/
Expand All @@ -389,12 +387,6 @@ struct bpf_program {
*/
size_t sub_insn_off;

char *name;
/* name with / replaced by _; makes recursive pinning
* in bpf_object__pin_programs easier
*/
char *pin_name;

/* instructions that belong to BPF program; insns[0] is located at
* sec_insn_off instruction within its ELF section in ELF file, so
* when mapping ELF file instruction index to the local instruction,
Expand Down Expand Up @@ -695,7 +687,6 @@ static void bpf_program__exit(struct bpf_program *prog)
bpf_program__unload(prog);
zfree(&prog->name);
zfree(&prog->sec_name);
zfree(&prog->pin_name);
zfree(&prog->insns);
zfree(&prog->reloc_desc);

Expand All @@ -704,26 +695,6 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->sec_idx = -1;
}

static char *__bpf_program__pin_name(struct bpf_program *prog)
{
char *name, *p;

if (libbpf_mode & LIBBPF_STRICT_SEC_NAME)
name = strdup(prog->name);
else
name = strdup(prog->sec_name);

if (!name)
return NULL;

p = name;

while ((p = strchr(p, '/')))
*p = '_';

return name;
}

static bool insn_is_subprog_call(const struct bpf_insn *insn)
{
return BPF_CLASS(insn->code) == BPF_JMP &&
Expand Down Expand Up @@ -790,10 +761,6 @@ bpf_object__init_prog(struct bpf_object *obj, struct bpf_program *prog,
if (!prog->name)
goto errout;

prog->pin_name = __bpf_program__pin_name(prog);
if (!prog->pin_name)
goto errout;

prog->insns = malloc(insn_data_sz);
if (!prog->insns)
goto errout;
Expand Down Expand Up @@ -2007,143 +1974,6 @@ static int bpf_object__init_kconfig_map(struct bpf_object *obj)
return 0;
}

static int bpf_object__init_user_maps(struct bpf_object *obj, bool strict)
{
Elf_Data *symbols = obj->efile.symbols;
int i, map_def_sz = 0, nr_maps = 0, nr_syms;
Elf_Data *data = NULL;
Elf_Scn *scn;

if (obj->efile.maps_shndx < 0)
return 0;

if (libbpf_mode & LIBBPF_STRICT_MAP_DEFINITIONS) {
pr_warn("legacy map definitions in SEC(\"maps\") are not supported\n");
return -EOPNOTSUPP;
}

if (!symbols)
return -EINVAL;

scn = elf_sec_by_idx(obj, obj->efile.maps_shndx);
data = elf_sec_data(obj, scn);
if (!scn || !data) {
pr_warn("elf: failed to get legacy map definitions for %s\n",
obj->path);
return -EINVAL;
}

/*
* Count number of maps. Each map has a name.
* Array of maps is not supported: only the first element is
* considered.
*
* TODO: Detect array of map and report error.
*/
nr_syms = symbols->d_size / sizeof(Elf64_Sym);
for (i = 0; i < nr_syms; i++) {
Elf64_Sym *sym = elf_sym_by_idx(obj, i);

if (sym->st_shndx != obj->efile.maps_shndx)
continue;
if (ELF64_ST_TYPE(sym->st_info) == STT_SECTION)
continue;
nr_maps++;
}
/* Assume equally sized map definitions */
pr_debug("elf: found %d legacy map definitions (%zd bytes) in %s\n",
nr_maps, data->d_size, obj->path);

if (!data->d_size || nr_maps == 0 || (data->d_size % nr_maps) != 0) {
pr_warn("elf: unable to determine legacy map definition size in %s\n",
obj->path);
return -EINVAL;
}
map_def_sz = data->d_size / nr_maps;

/* Fill obj->maps using data in "maps" section. */
for (i = 0; i < nr_syms; i++) {
Elf64_Sym *sym = elf_sym_by_idx(obj, i);
const char *map_name;
struct bpf_map_def *def;
struct bpf_map *map;

if (sym->st_shndx != obj->efile.maps_shndx)
continue;
if (ELF64_ST_TYPE(sym->st_info) == STT_SECTION)
continue;

map = bpf_object__add_map(obj);
if (IS_ERR(map))
return PTR_ERR(map);

map_name = elf_sym_str(obj, sym->st_name);
if (!map_name) {
pr_warn("failed to get map #%d name sym string for obj %s\n",
i, obj->path);
return -LIBBPF_ERRNO__FORMAT;
}

pr_warn("map '%s' (legacy): legacy map definitions are deprecated, use BTF-defined maps instead\n", map_name);

if (ELF64_ST_BIND(sym->st_info) == STB_LOCAL) {
pr_warn("map '%s' (legacy): static maps are not supported\n", map_name);
return -ENOTSUP;
}

map->libbpf_type = LIBBPF_MAP_UNSPEC;
map->sec_idx = sym->st_shndx;
map->sec_offset = sym->st_value;
pr_debug("map '%s' (legacy): at sec_idx %d, offset %zu.\n",
map_name, map->sec_idx, map->sec_offset);
if (sym->st_value + map_def_sz > data->d_size) {
pr_warn("corrupted maps section in %s: last map \"%s\" too small\n",
obj->path, map_name);
return -EINVAL;
}

map->name = strdup(map_name);
if (!map->name) {
pr_warn("map '%s': failed to alloc map name\n", map_name);
return -ENOMEM;
}
pr_debug("map %d is \"%s\"\n", i, map->name);
def = (struct bpf_map_def *)(data->d_buf + sym->st_value);
/*
* If the definition of the map in the object file fits in
* bpf_map_def, copy it. Any extra fields in our version
* of bpf_map_def will default to zero as a result of the
* calloc above.
*/
if (map_def_sz <= sizeof(struct bpf_map_def)) {
memcpy(&map->def, def, map_def_sz);
} else {
/*
* Here the map structure being read is bigger than what
* we expect, truncate if the excess bits are all zero.
* If they are not zero, reject this map as
* incompatible.
*/
char *b;

for (b = ((char *)def) + sizeof(struct bpf_map_def);
b < ((char *)def) + map_def_sz; b++) {
if (*b != 0) {
pr_warn("maps section in %s: \"%s\" has unrecognized, non-zero options\n",
obj->path, map_name);
if (strict)
return -EINVAL;
}
}
memcpy(&map->def, def, sizeof(struct bpf_map_def));
}

/* btf info may not exist but fill it in if it does exist */
(void) bpf_map_find_btf_info(obj, map);
}
return 0;
}

const struct btf_type *
skip_mods_and_typedefs(const struct btf *btf, __u32 id, __u32 *res_id)
{
Expand Down Expand Up @@ -2700,12 +2530,11 @@ static int bpf_object__init_maps(struct bpf_object *obj,
{
const char *pin_root_path;
bool strict;
int err;
int err = 0;

strict = !OPTS_GET(opts, relaxed_maps, false);
pin_root_path = OPTS_GET(opts, pin_root_path, NULL);

err = bpf_object__init_user_maps(obj, strict);
err = err ?: bpf_object__init_user_btf_maps(obj, strict, pin_root_path);
err = err ?: bpf_object__init_global_data_maps(obj);
err = err ?: bpf_object__init_kconfig_map(obj);
Expand Down Expand Up @@ -3979,28 +3808,8 @@ static int bpf_object__collect_externs(struct bpf_object *obj)
return 0;
}

static bool prog_is_subprog(const struct bpf_object *obj,
const struct bpf_program *prog)
{
/* For legacy reasons, libbpf supports an entry-point BPF programs
* without SEC() attribute, i.e., those in the .text section. But if
* there are 2 or more such programs in the .text section, they all
* must be subprograms called from entry-point BPF programs in
* designated SEC()'tions, otherwise there is no way to distinguish
* which of those programs should be loaded vs which are a subprogram.
* Similarly, if there is a function/program in .text and at least one
* other BPF program with custom SEC() attribute, then we just assume
* .text programs are subprograms (even if they are not called from
* other programs), because libbpf never explicitly supported mixing
* SEC()-designated BPF programs and .text entry-point BPF programs.
*
* In libbpf 1.0 strict mode, we always consider .text
* programs to be subprograms.
*/

if (libbpf_mode & LIBBPF_STRICT_SEC_NAME)
return prog->sec_idx == obj->efile.text_shndx;

static bool prog_is_subprog(const struct bpf_object *obj, const struct bpf_program *prog)
{
return prog->sec_idx == obj->efile.text_shndx && obj->nr_programs > 1;
}

Expand Down Expand Up @@ -8163,8 +7972,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
char buf[PATH_MAX];
int len;

len = snprintf(buf, PATH_MAX, "%s/%s", path,
prog->pin_name);
len = snprintf(buf, PATH_MAX, "%s/%s", path, prog->name);
if (len < 0) {
err = -EINVAL;
goto err_unpin_programs;
Expand All @@ -8185,8 +7993,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
char buf[PATH_MAX];
int len;

len = snprintf(buf, PATH_MAX, "%s/%s", path,
prog->pin_name);
len = snprintf(buf, PATH_MAX, "%s/%s", path, prog->name);
if (len < 0)
continue;
else if (len >= PATH_MAX)
Expand All @@ -8210,8 +8017,7 @@ int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
char buf[PATH_MAX];
int len;

len = snprintf(buf, PATH_MAX, "%s/%s", path,
prog->pin_name);
len = snprintf(buf, PATH_MAX, "%s/%s", path, prog->name);
if (len < 0)
return libbpf_err(-EINVAL);
else if (len >= PATH_MAX)
Expand Down
34 changes: 0 additions & 34 deletions tools/lib/bpf/libbpf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -886,40 +886,6 @@ LIBBPF_API int bpf_map__lookup_and_delete_elem(const struct bpf_map *map,
LIBBPF_API int bpf_map__get_next_key(const struct bpf_map *map,
const void *cur_key, void *next_key, size_t key_sz);

/**
* @brief **libbpf_get_error()** extracts the error code from the passed
* pointer
* @param ptr pointer returned from libbpf API function
* @return error code; or 0 if no error occured
*
* Many libbpf API functions which return pointers have logic to encode error
* codes as pointers, and do not return NULL. Meaning **libbpf_get_error()**
* should be used on the return value from these functions immediately after
* calling the API function, with no intervening calls that could clobber the
* `errno` variable. Consult the individual functions documentation to verify
* if this logic applies should be used.
*
* For these API functions, if `libbpf_set_strict_mode(LIBBPF_STRICT_CLEAN_PTRS)`
* is enabled, NULL is returned on error instead.
*
* If ptr is NULL, then errno should be already set by the failing
* API, because libbpf never returns NULL on success and it now always
* sets errno on error.
*
* Example usage:
*
* struct perf_buffer *pb;
*
* pb = perf_buffer__new(bpf_map__fd(obj->maps.events), PERF_BUFFER_PAGES, &opts);
* err = libbpf_get_error(pb);
* if (err) {
* pb = NULL;
* fprintf(stderr, "failed to open perf buffer: %d\n", err);
* goto cleanup;
* }
*/
LIBBPF_API long libbpf_get_error(const void *ptr);

struct bpf_xdp_set_link_opts {
size_t sz;
int old_fd;
Expand Down
Loading

0 comments on commit bd05410

Please sign in to comment.