On Wed, Jul 14, 2021 at 9:52 AM Martynas Pumputis <m@xxxxxxxxx> wrote:
>
> 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 <andrii@xxxxxxxxxx>
> Signed-off-by: Martynas Pumputis <m@xxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6f5e2757bb3c..1a840e81ea0a 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -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 ret = 0;
>
> memset(&create_attr, 0, sizeof(create_attr));
>
> @@ -4561,7 +4562,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
> }
>
> if (map->fd < 0)
> - return -errno;
> + ret = -errno;
Oh, isn't this a complicated function, eh? I stared at the code for a
while until I understood the whole idea with map->inner_map handling
there.
I think your change is correct, I'd just love you to consolidate all
those "int err" definitions, and use just one throughout this
function. It will clean up two other if() blocks, and in this case
"err" name is more appropriate, because it always is <= 0.
>
> if (bpf_map_type__is_map_in_map(def->type) && map->inner_map) {
> if (obj->gen_loader)
> @@ -4570,7 +4571,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
> zfree(&map->inner_map);
> }
>
> - return 0;
> + return ret;
> }
>
> static int init_map_slots(struct bpf_object *obj, struct bpf_map *map)
> --
> 2.32.0
>
