| Message ID | 20210719173838.423148-2-m@lambda.lt (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: fix inner map removal in bpf_object__create_map | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 1 blamed authors not CCed: toke@redhat.com; 7 maintainers not CCed: netdev@vger.kernel.org yhs@fb.com kpsingh@kernel.org kafai@fb.com john.fastabend@gmail.com songliubraving@fb.com toke@redhat.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Jul 19, 2021 at 10:36 AM Martynas Pumputis <m@lambda.lt> 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@kernel.org> Please preserve received acks between versions (unless some significant changes happen between versions invalidating the original ack): Acked-by: John Fastabend <john.fastabend@gmail.com> Thanks, applied (with few adjustments) to bpf-next, given we lived with this bug for a long while and no one ever noticed/reported it (and BPF map will be cleaned up when the process exits anyway). Keeping it in bpf-next will make it easier for follow-up patches changing/enhancing bpf_map__create_map() and will avoid merge conflicts down the line. > Signed-off-by: Martynas Pumputis <m@lambda.lt> > --- > tools/lib/bpf/libbpf.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 6f5e2757bb3c..dde521366579 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 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", > @@ -4547,7 +4546,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b > if (map->fd < 0 && (create_attr.btf_key_type_id || > create_attr.btf_value_type_id)) { > char *cp, errmsg[STRERR_BUFSIZE]; > - int err = -errno; needed empty line here > + 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 >
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 6f5e2757bb3c..dde521366579 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 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", @@ -4547,7 +4546,7 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b 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", @@ -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)
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@kernel.org> Signed-off-by: Martynas Pumputis <m@lambda.lt> --- tools/lib/bpf/libbpf.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)