From 6ca0b4089166c6e03b95e48866931d23f4ff542f Mon Sep 17 00:00:00 2001 From: Shuyi Cheng Date: Tue, 13 Jul 2021 20:42:38 +0800 Subject: libbpf: Fix the possible memory leak on error [ Upstream commit 18353c87e0e0440d4c7c746ed740738bbc1b538e ] If the strdup() fails then we need to call bpf_object__close(obj) to avoid a resource leak. Fixes: 166750bc1dd2 ("libbpf: Support libbpf-provided extern variables") Signed-off-by: Shuyi Cheng Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/1626180159-112996-3-git-send-email-chengshuyi@linux.alibaba.com Signed-off-by: Sasha Levin --- tools/lib/bpf/libbpf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'tools/lib/bpf/libbpf.c') diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 95eef7ebdac5..04cde732d686 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -6907,8 +6907,10 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz, kconfig = OPTS_GET(opts, kconfig, NULL); if (kconfig) { obj->kconfig = strdup(kconfig); - if (!obj->kconfig) - return ERR_PTR(-ENOMEM); + if (!obj->kconfig) { + err = -ENOMEM; + goto out; + } } err = bpf_object__elf_init(obj); -- cgit v1.2.3 From 8c3b5028ec02924f3b037a6d07124a006a9a3c4f Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Mon, 19 Jul 2021 19:38:37 +0200 Subject: libbpf: Fix removal of inner map in bpf_object__create_map [ Upstream commit a21ab4c59e09c2a9994a6e393b7484e3b3f78a99 ] If creating an outer map of a BTF-defined map-in-map fails (via bpf_object__create_map()), then the previously created its inner map won't be destroyed. Fix this by ensuring that the destroy routines are not bypassed in the case of a failure. Fixes: 646f02ffdd49c ("libbpf: Add BTF-defined map-in-map support") Reported-by: Andrii Nakryiko Signed-off-by: Martynas Pumputis Signed-off-by: Andrii Nakryiko Acked-by: John Fastabend Link: https://lore.kernel.org/bpf/20210719173838.423148-2-m@lambda.lt Signed-off-by: Sasha Levin --- tools/lib/bpf/libbpf.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'tools/lib/bpf/libbpf.c') diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 04cde732d686..28923b776cdc 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -4123,6 +4123,7 @@ 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; + int err = 0; memset(&create_attr, 0, sizeof(create_attr)); @@ -4165,8 +4166,6 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map) if (bpf_map_type__is_map_in_map(def->type)) { if (map->inner_map) { - int err; - err = bpf_object__create_map(obj, map->inner_map); if (err) { pr_warn("map '%s': failed to create inner map: %d\n", @@ -4183,8 +4182,8 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map) if (map->fd < 0 && (create_attr.btf_key_type_id || create_attr.btf_value_type_id)) { char *cp, errmsg[STRERR_BUFSIZE]; - int err = -errno; + 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); @@ -4196,15 +4195,14 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map) map->fd = bpf_create_map_xattr(&create_attr); } - if (map->fd < 0) - return -errno; + err = map->fd < 0 ? -errno : 0; if (bpf_map_type__is_map_in_map(def->type) && map->inner_map) { bpf_map__destroy(map->inner_map); zfree(&map->inner_map); } - return 0; + return err; } static int init_map_slots(struct bpf_map *map) -- cgit v1.2.3 From c327b69e96b0d17b56ebf15ebdf3262148f7e7af Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Mon, 12 Jul 2021 14:55:51 +0200 Subject: libbpf: Fix reuse of pinned map on older kernel [ Upstream commit 97eb31384af943d6b97eb5947262cee4ef25cb87 ] When loading a BPF program with a pinned map, the loader checks whether the pinned map can be reused, i.e. their properties match. To derive such of the pinned map, the loader invokes BPF_OBJ_GET_INFO_BY_FD and then does the comparison. Unfortunately, on < 4.12 kernels the BPF_OBJ_GET_INFO_BY_FD is not available, so loading the program fails with the following error: libbpf: failed to get map info for map FD 5: Invalid argument libbpf: couldn't reuse pinned map at '/sys/fs/bpf/tc/globals/cilium_call_policy': parameter mismatch" libbpf: map 'cilium_call_policy': error reusing pinned map libbpf: map 'cilium_call_policy': failed to create: Invalid argument(-22) libbpf: failed to load object 'bpf_overlay.o' To fix this, fallback to derivation of the map properties via /proc/$PID/fdinfo/$MAP_FD if BPF_OBJ_GET_INFO_BY_FD fails with EINVAL, which can be used as an indicator that the kernel doesn't support the latter. Signed-off-by: Martynas Pumputis Signed-off-by: Andrii Nakryiko Acked-by: John Fastabend Link: https://lore.kernel.org/bpf/20210712125552.58705-1-m@lambda.lt Signed-off-by: Sasha Levin --- tools/lib/bpf/libbpf.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) (limited to 'tools/lib/bpf/libbpf.c') diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 28923b776cdc..0dad862b3b9d 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -3613,6 +3613,42 @@ static int bpf_map_find_btf_info(struct bpf_object *obj, struct bpf_map *map) return 0; } +static int bpf_get_map_info_from_fdinfo(int fd, struct bpf_map_info *info) +{ + char file[PATH_MAX], buff[4096]; + FILE *fp; + __u32 val; + int err; + + snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd); + memset(info, 0, sizeof(*info)); + + fp = fopen(file, "r"); + if (!fp) { + err = -errno; + pr_warn("failed to open %s: %d. No procfs support?\n", file, + err); + return err; + } + + while (fgets(buff, sizeof(buff), fp)) { + if (sscanf(buff, "map_type:\t%u", &val) == 1) + info->type = val; + else if (sscanf(buff, "key_size:\t%u", &val) == 1) + info->key_size = val; + else if (sscanf(buff, "value_size:\t%u", &val) == 1) + info->value_size = val; + else if (sscanf(buff, "max_entries:\t%u", &val) == 1) + info->max_entries = val; + else if (sscanf(buff, "map_flags:\t%i", &val) == 1) + info->map_flags = val; + } + + fclose(fp); + + return 0; +} + int bpf_map__reuse_fd(struct bpf_map *map, int fd) { struct bpf_map_info info = {}; @@ -3621,6 +3657,8 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd) char *new_name; err = bpf_obj_get_info_by_fd(fd, &info, &len); + if (err && errno == EINVAL) + err = bpf_get_map_info_from_fdinfo(fd, &info); if (err) return err; @@ -4032,12 +4070,16 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) struct bpf_map_info map_info = {}; char msg[STRERR_BUFSIZE]; __u32 map_info_len; + int err; map_info_len = sizeof(map_info); - if (bpf_obj_get_info_by_fd(map_fd, &map_info, &map_info_len)) { - pr_warn("failed to get map info for map FD %d: %s\n", - map_fd, libbpf_strerror_r(errno, msg, sizeof(msg))); + err = bpf_obj_get_info_by_fd(map_fd, &map_info, &map_info_len); + if (err && errno == EINVAL) + err = bpf_get_map_info_from_fdinfo(map_fd, &map_info); + if (err) { + pr_warn("failed to get map info for map FD %d: %s\n", map_fd, + libbpf_strerror_r(errno, msg, sizeof(msg))); return false; } -- cgit v1.2.3 From 4af60a543ba60239d8931215c7e239c91e42c129 Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Mon, 26 Jul 2021 17:20:01 +0200 Subject: libbpf: Fix race when pinning maps in parallel [ Upstream commit 043c5bb3c4f43670ab4fea0b847373ab42d25f3e ] When loading in parallel multiple programs which use the same to-be pinned map, it is possible that two instances of the loader will call bpf_object__create_maps() at the same time. If the map doesn't exist when both instances call bpf_object__reuse_map(), then one of the instances will fail with EEXIST when calling bpf_map__pin(). Fix the race by retrying reusing a map if bpf_map__pin() returns EEXIST. The fix is similar to the one in iproute2: e4c4685fd6e4 ("bpf: Fix race condition with map pinning"). Before retrying the pinning, we don't do any special cleaning of an internal map state. The closer code inspection revealed that it's not required: - bpf_object__create_map(): map->inner_map is destroyed after a successful call, map->fd is closed if pinning fails. - bpf_object__populate_internal_map(): created map elements is destroyed upon close(map->fd). - init_map_slots(): slots are freed after their initialization. Signed-off-by: Martynas Pumputis Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20210726152001.34845-1-m@lambda.lt Signed-off-by: Sasha Levin --- tools/lib/bpf/libbpf.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'tools/lib/bpf/libbpf.c') diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 0dad862b3b9d..b337d6f29098 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -4284,10 +4284,13 @@ bpf_object__create_maps(struct bpf_object *obj) char *cp, errmsg[STRERR_BUFSIZE]; unsigned int i, j; int err; + bool retried; for (i = 0; i < obj->nr_maps; i++) { map = &obj->maps[i]; + retried = false; +retry: if (map->pin_path) { err = bpf_object__reuse_map(map); if (err) { @@ -4295,6 +4298,12 @@ bpf_object__create_maps(struct bpf_object *obj) map->name); goto err_out; } + if (retried && map->fd < 0) { + pr_warn("map '%s': cannot find pinned map\n", + map->name); + err = -ENOENT; + goto err_out; + } } if (map->fd >= 0) { @@ -4328,9 +4337,13 @@ bpf_object__create_maps(struct bpf_object *obj) if (map->pin_path && !map->pinned) { err = bpf_map__pin(map, NULL); if (err) { + zclose(map->fd); + if (!retried && err == -EEXIST) { + retried = true; + goto retry; + } pr_warn("map '%s': failed to auto-pin at '%s': %d\n", map->name, map->pin_path, err); - zclose(map->fd); goto err_out; } } -- cgit v1.2.3