From 2dc7cbd89360a3946f32fa30a7d5b24a14526d15 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Wed, 29 Jul 2020 16:21:48 -0700 Subject: [PATCH] libbpf: Make destructors more robust by handling ERR_PTR(err) cases Most of libbpf "constructors" on failure return ERR_PTR(err) result encoded as a pointer. It's a common mistake to eventually pass such malformed pointers into xxx__destroy()/xxx__free() "destructors". So instead of fixing up clean up code in selftests and user programs, handle such error pointers in destructors themselves. This works beautifully for NULL pointers passed to destructors, so might as well just work for error pointers. Suggested-by: Song Liu Signed-off-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann Acked-by: Song Liu Link: https://lore.kernel.org/bpf/20200729232148.896125-1-andriin@fb.com --- src/btf.c | 4 ++-- src/btf_dump.c | 2 +- src/libbpf.c | 9 ++++----- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/btf.c b/src/btf.c index c9e760e..ded5b29 100644 --- a/src/btf.c +++ b/src/btf.c @@ -386,7 +386,7 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name, void btf__free(struct btf *btf) { - if (!btf) + if (IS_ERR_OR_NULL(btf)) return; if (btf->fd >= 0) @@ -1025,7 +1025,7 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size) void btf_ext__free(struct btf_ext *btf_ext) { - if (!btf_ext) + if (IS_ERR_OR_NULL(btf_ext)) return; free(btf_ext->data); free(btf_ext); diff --git a/src/btf_dump.c b/src/btf_dump.c index e1c3445..cf71116 100644 --- a/src/btf_dump.c +++ b/src/btf_dump.c @@ -183,7 +183,7 @@ void btf_dump__free(struct btf_dump *d) { int i, cnt; - if (!d) + if (IS_ERR_OR_NULL(d)) return; free(d->type_states); diff --git a/src/libbpf.c b/src/libbpf.c index 54830d6..b9f11f8 100644 --- a/src/libbpf.c +++ b/src/libbpf.c @@ -6504,7 +6504,7 @@ void bpf_object__close(struct bpf_object *obj) { size_t i; - if (!obj) + if (IS_ERR_OR_NULL(obj)) return; if (obj->clear_priv) @@ -7690,7 +7690,7 @@ int bpf_link__destroy(struct bpf_link *link) { int err = 0; - if (!link) + if (IS_ERR_OR_NULL(link)) return 0; if (!link->disconnected && link->detach) @@ -8502,7 +8502,7 @@ void perf_buffer__free(struct perf_buffer *pb) { int i; - if (!pb) + if (IS_ERR_OR_NULL(pb)) return; if (pb->cpu_bufs) { for (i = 0; i < pb->cpu_cnt; i++) { @@ -9379,8 +9379,7 @@ void bpf_object__detach_skeleton(struct bpf_object_skeleton *s) for (i = 0; i < s->prog_cnt; i++) { struct bpf_link **link = s->progs[i].link; - if (!IS_ERR_OR_NULL(*link)) - bpf_link__destroy(*link); + bpf_link__destroy(*link); *link = NULL; } }