libbpf: fix inner map removal in bpf_object__create_map

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 <[email protected]>

Signed-off-by Martynas Pumputis <[email protected]>

---
 tools/lib/bpf/libbpf.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

tools/lib/bpf/libbpf.c +1 -2

index 6f5e2757bb3c..dde521366579 100644
@@ -4479,6 +4479,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
 {
        struct bpf_create_map_attr create_attr;
        struct bpf_map_def *def = &map->def;
+       int err = 0;
 
        memset(&create_attr, 0, sizeof(create_attr));
 

@@ -4521,8 +4522,6 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
 
        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, true);
                        if (err) {
                                pr_warn("map '%s': failed to create inner map: %d\n",

tools/lib/bpf/libbpf.c +3 -4

+               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",

@@ -4560,8 +4559,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
                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) {
                if (obj->gen_loader)

@@ -4570,7 +4568,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
                zfree(&map->inner_map);
        }
 
-       return 0;
+       return err;
 }
 
 static int init_map_slots(struct bpf_object *obj, struct bpf_map *map)
--

2.32.0

Add a test case to check whether an unsuccessful creation of an outer map of a BTF-defined map-in-map destroys the inner map.

As bpf_object__create_map() is a static function, we cannot just call it from the test case and then check whether a map accessible via map->inner_map_fd has been closed. Instead, we iterate over all maps and check whether the map "$MAP_NAME.inner" does not exist.

Signed-off-by Martynas Pumputis <[email protected]>

---
 .../bpf/progs/test_map_in_map_invalid.c       | 26 ++++++++
 tools/testing/selftests/bpf/test_maps.c       | 64 ++++++++++++++++++-
 2 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c

tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c new file +10 -0

new file mode 100644
index 000000000000..2918caea1e3d
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Isovalent, Inc. */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+struct inner {
+       __uint(type, BPF_MAP_TYPE_ARRAY);
+       __type(key, __u32);
+       __type(value, int);
+       __uint(max_entries, 4);

tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c new file +6 -0

+int xdp_noop0(struct xdp_md *ctx)
+{
+       return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";

tools/testing/selftests/bpf/test_maps.c +4 -0

index 30cbf5d98f7d..d4184dde04df 100644
@@ -1153,12 +1153,16 @@ static void test_sockmap(unsigned int tasks, void *data)
 }
 
 #define MAPINMAP_PROG "./test_map_in_map.o"
+#define MAPINMAP_INVALID_PROG "./test_map_in_map_invalid.o"
 static void test_map_in_map(void)
 {
        struct bpf_object *obj;
        struct bpf_map *map;
        int mim_fd, fd, err;
        int pos = 0;
+       struct bpf_map_info info = {};
+       __u32 len = sizeof(info);
+       __u32 id = 0;
 
        obj = bpf_object__open(MAPINMAP_PROG);
 

tools/testing/selftests/bpf/test_maps.c +12 -0

        bpf_object__close(obj);
+
+
+       /* Test that failing bpf_object__create_map() destroys the inner map */
+
+       obj = bpf_object__open(MAPINMAP_INVALID_PROG);
+       err = libbpf_get_error(obj);
+       if (err) {
+               printf("Failed to load %s program: %d %d",
+                      MAPINMAP_INVALID_PROG, err, errno);
+               goto out_map_in_map;
+       }
+

Add a test case to check whether an unsuccessful creation of an outer map of a BTF-defined map-in-map destroys the inner map.

As bpf_object__create_map() is a static function, we cannot just call it from the test case and then check whether a map accessible via map->inner_map_fd has been closed. Instead, we iterate over all maps and check whether the map "$MAP_NAME.inner" does not exist.

Signed-off-by Martynas Pumputis <[email protected]>

---
 .../bpf/progs/test_map_in_map_invalid.c       | 26 ++++++++
 tools/testing/selftests/bpf/test_maps.c       | 64 ++++++++++++++++++-
 2 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c

tools/testing/selftests/bpf/test_maps.c +5 -0

+       if (!err) {
+               printf("Loading obj supposed to fail\n");
+               goto out_map_in_map;
+       }
+