Skip to content

Commit

Permalink
libbpf: Refactor map creation logic and fix cleanup leak
Browse files Browse the repository at this point in the history 
Factor out map creation and destruction logic to simplify code and especially
error handling. Also fix map FD leak in case of partially successful map
creation during bpf_object load operation.

Fixes: 57a00f4 ("libbpf: Add auto-pinning of maps when loading BPF objects")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/bpf/20200429002739.48006-3-andriin@fb.com
  • Loading branch information
Andrii Nakryiko authored and Alexei Starovoitov committed Apr 29, 2020
1 parent 41017e5 commit 2d39d7c
Showing 1 changed file with 121 additions and 105 deletions.
226 changes: 121 additions & 105 deletions tools/lib/bpf/libbpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3493,124 +3493,135 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
return 0;
}

static void bpf_map__destroy(struct bpf_map *map);

static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map)
{
struct bpf_create_map_attr create_attr;
struct bpf_map_def *def = &map->def;

memset(&create_attr, 0, sizeof(create_attr));

if (obj->caps.name)
create_attr.name = map->name;
create_attr.map_ifindex = map->map_ifindex;
create_attr.map_type = def->type;
create_attr.map_flags = def->map_flags;
create_attr.key_size = def->key_size;
create_attr.value_size = def->value_size;

if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY && !def->max_entries) {
int nr_cpus;

nr_cpus = libbpf_num_possible_cpus();
if (nr_cpus < 0) {
pr_warn("map '%s': failed to determine number of system CPUs: %d\n",
map->name, nr_cpus);
return nr_cpus;
}
pr_debug("map '%s': setting size to %d\n", map->name, nr_cpus);
create_attr.max_entries = nr_cpus;
} else {
create_attr.max_entries = def->max_entries;
}

if (bpf_map__is_struct_ops(map))
create_attr.btf_vmlinux_value_type_id =
map->btf_vmlinux_value_type_id;

create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
if (obj->btf && !bpf_map_find_btf_info(obj, map)) {
create_attr.btf_fd = btf__fd(obj->btf);
create_attr.btf_key_type_id = map->btf_key_type_id;
create_attr.btf_value_type_id = map->btf_value_type_id;
}

map->fd = bpf_create_map_xattr(&create_attr);
if (map->fd < 0 && (create_attr.btf_key_type_id ||
create_attr.btf_value_type_id)) {
char *cp, errmsg[STRERR_BUFSIZE];
int err = -errno;

cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
map->name, cp, err);
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
map->btf_key_type_id = 0;
map->btf_value_type_id = 0;
map->fd = bpf_create_map_xattr(&create_attr);
}

if (map->fd < 0)
return -errno;

return 0;
}

static int
bpf_object__create_maps(struct bpf_object *obj)
{
struct bpf_create_map_attr create_attr = {};
int nr_cpus = 0;
unsigned int i;
struct bpf_map *map;
char *cp, errmsg[STRERR_BUFSIZE];
unsigned int i, j;
int err;

for (i = 0; i < obj->nr_maps; i++) {
struct bpf_map *map = &obj->maps[i];
struct bpf_map_def *def = &map->def;
char *cp, errmsg[STRERR_BUFSIZE];
int *pfd = &map->fd;
map = &obj->maps[i];

if (map->pin_path) {
err = bpf_object__reuse_map(map);
if (err) {
pr_warn("error reusing pinned map %s\n",
pr_warn("map '%s': error reusing pinned map\n",
map->name);
return err;
goto err_out;
}
}

if (map->fd >= 0) {
pr_debug("skip map create (preset) %s: fd=%d\n",
pr_debug("map '%s': skipping creation (preset fd=%d)\n",
map->name, map->fd);
continue;
}

if (obj->caps.name)
create_attr.name = map->name;
create_attr.map_ifindex = map->map_ifindex;
create_attr.map_type = def->type;
create_attr.map_flags = def->map_flags;
create_attr.key_size = def->key_size;
create_attr.value_size = def->value_size;
if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY &&
!def->max_entries) {
if (!nr_cpus)
nr_cpus = libbpf_num_possible_cpus();
if (nr_cpus < 0) {
pr_warn("failed to determine number of system CPUs: %d\n",
nr_cpus);
err = nr_cpus;
goto err_out;
}
pr_debug("map '%s': setting size to %d\n",
map->name, nr_cpus);
create_attr.max_entries = nr_cpus;
} else {
create_attr.max_entries = def->max_entries;
}
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
if (bpf_map_type__is_map_in_map(def->type) &&
map->inner_map_fd >= 0)
create_attr.inner_map_fd = map->inner_map_fd;
if (bpf_map__is_struct_ops(map))
create_attr.btf_vmlinux_value_type_id =
map->btf_vmlinux_value_type_id;

if (obj->btf && !bpf_map_find_btf_info(obj, map)) {
create_attr.btf_fd = btf__fd(obj->btf);
create_attr.btf_key_type_id = map->btf_key_type_id;
create_attr.btf_value_type_id = map->btf_value_type_id;
}

*pfd = bpf_create_map_xattr(&create_attr);
if (*pfd < 0 && (create_attr.btf_key_type_id ||
create_attr.btf_value_type_id)) {
err = -errno;
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
map->name, cp, err);
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
map->btf_key_type_id = 0;
map->btf_value_type_id = 0;
*pfd = bpf_create_map_xattr(&create_attr);
}

if (*pfd < 0) {
size_t j;
err = bpf_object__create_map(obj, map);
if (err)
goto err_out;

err = -errno;
err_out:
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("failed to create map (name: '%s'): %s(%d)\n",
map->name, cp, err);
pr_perm_msg(err);
for (j = 0; j < i; j++)
zclose(obj->maps[j].fd);
return err;
}
pr_debug("map '%s': created successfully, fd=%d\n", map->name,
map->fd);

if (bpf_map__is_internal(map)) {
err = bpf_object__populate_internal_map(obj, map);
if (err < 0) {
zclose(*pfd);
zclose(map->fd);
goto err_out;
}
}

if (map->pin_path && !map->pinned) {
err = bpf_map__pin(map, NULL);
if (err) {
pr_warn("failed to auto-pin map name '%s' at '%s'\n",
map->name, map->pin_path);
return err;
pr_warn("map '%s': failed to auto-pin at '%s': %d\n",
map->name, map->pin_path, err);
zclose(map->fd);
goto err_out;
}
}

pr_debug("created map %s: fd=%d\n", map->name, *pfd);
}

return 0;

err_out:
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("map '%s': failed to create: %s(%d)\n", map->name, cp, err);
pr_perm_msg(err);
for (j = 0; j < i; j++)
zclose(obj->maps[j].fd);
return err;
}

static int
Expand Down Expand Up @@ -5966,6 +5977,32 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
return 0;
}

static void bpf_map__destroy(struct bpf_map *map)
{
if (map->clear_priv)
map->clear_priv(map, map->priv);
map->priv = NULL;
map->clear_priv = NULL;

if (map->mmaped) {
munmap(map->mmaped, bpf_map_mmap_sz(map));
map->mmaped = NULL;
}

if (map->st_ops) {
zfree(&map->st_ops->data);
zfree(&map->st_ops->progs);
zfree(&map->st_ops->kern_func_off);
zfree(&map->st_ops);
}

zfree(&map->name);
zfree(&map->pin_path);

if (map->fd >= 0)
zclose(map->fd);
}

void bpf_object__close(struct bpf_object *obj)
{
size_t i;
Expand All @@ -5981,29 +6018,8 @@ void bpf_object__close(struct bpf_object *obj)
btf__free(obj->btf);
btf_ext__free(obj->btf_ext);

for (i = 0; i < obj->nr_maps; i++) {
struct bpf_map *map = &obj->maps[i];

if (map->clear_priv)
map->clear_priv(map, map->priv);
map->priv = NULL;
map->clear_priv = NULL;

if (map->mmaped) {
munmap(map->mmaped, bpf_map_mmap_sz(map));
map->mmaped = NULL;
}

if (map->st_ops) {
zfree(&map->st_ops->data);
zfree(&map->st_ops->progs);
zfree(&map->st_ops->kern_func_off);
zfree(&map->st_ops);
}

zfree(&map->name);
zfree(&map->pin_path);
}
for (i = 0; i < obj->nr_maps; i++)
bpf_map__destroy(&obj->maps[i]);

zfree(&obj->kconfig);
zfree(&obj->externs);
Expand Down

0 comments on commit 2d39d7c

Please sign in to comment.