* [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free()
@ 2022-01-07 15:26 Mauricio Vásquez
2022-01-07 15:26 ` [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() Mauricio Vásquez
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mauricio Vásquez @ 2022-01-07 15:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Quentin Monnet
hashmap__new() uses ERR_PTR() to return an error so it's better to
use IS_ERR_OR_NULL() in order to check the pointer before calling
free(). This will prevent freeing an invalid pointer if somebody calls
hashmap__free() with the result of a failed hashmap__new() call.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
---
tools/lib/bpf/hashmap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/lib/bpf/hashmap.c b/tools/lib/bpf/hashmap.c
index 3c20b126d60d..aeb09c288716 100644
--- a/tools/lib/bpf/hashmap.c
+++ b/tools/lib/bpf/hashmap.c
@@ -75,7 +75,7 @@ void hashmap__clear(struct hashmap *map)
void hashmap__free(struct hashmap *map)
{
- if (!map)
+ if (IS_ERR_OR_NULL(map))
return;
hashmap__clear(map);
@@ -238,4 +238,3 @@ bool hashmap__delete(struct hashmap *map, const void *key,
return true;
}
-
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() 2022-01-07 15:26 [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Mauricio Vásquez @ 2022-01-07 15:26 ` Mauricio Vásquez 2022-01-07 18:02 ` Quentin Monnet 2022-01-07 18:51 ` Song Liu 2022-01-07 17:40 ` [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Song Liu 2022-01-07 22:00 ` patchwork-bot+netdevbpf 2 siblings, 2 replies; 6+ messages in thread From: Mauricio Vásquez @ 2022-01-07 15:26 UTC (permalink / raw) To: netdev, bpf Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Quentin Monnet hashmap__new() encodes errors with ERR_PTR(), hence it's not valid to check the returned pointer against NULL and IS_ERR() has to be used instead. libbpf_get_error() can't be used in this case as hashmap__new() is not part of the public libbpf API and it'll continue using ERR_PTR() after libbpf 1.0. Fixes: 8f184732b60b ("bpftool: Switch to libbpf's hashmap for pinned paths of BPF objects") Fixes: 2828d0d75b73 ("bpftool: Switch to libbpf's hashmap for programs/maps in BTF listing") Fixes: d6699f8e0f83 ("bpftool: Switch to libbpf's hashmap for PIDs/names references") Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io> --- tools/bpf/bpftool/btf.c | 2 +- tools/bpf/bpftool/link.c | 3 ++- tools/bpf/bpftool/map.c | 2 +- tools/bpf/bpftool/pids.c | 3 ++- tools/bpf/bpftool/prog.c | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index 59833125ac0a..a2c665beda87 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -902,7 +902,7 @@ static int do_show(int argc, char **argv) equal_fn_for_key_as_id, NULL); btf_map_table = hashmap__new(hash_fn_for_key_as_id, equal_fn_for_key_as_id, NULL); - if (!btf_prog_table || !btf_map_table) { + if (IS_ERR(btf_prog_table) || IS_ERR(btf_map_table)) { hashmap__free(btf_prog_table); hashmap__free(btf_map_table); if (fd >= 0) diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c index 2c258db0d352..97dec81950e5 100644 --- a/tools/bpf/bpftool/link.c +++ b/tools/bpf/bpftool/link.c @@ -2,6 +2,7 @@ /* Copyright (C) 2020 Facebook */ #include <errno.h> +#include <linux/err.h> #include <net/if.h> #include <stdio.h> #include <unistd.h> @@ -306,7 +307,7 @@ static int do_show(int argc, char **argv) if (show_pinned) { link_table = hashmap__new(hash_fn_for_key_as_id, equal_fn_for_key_as_id, NULL); - if (!link_table) { + if (IS_ERR(link_table)) { p_err("failed to create hashmap for pinned paths"); return -1; } diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c index cc530a229812..c66a3c979b7a 100644 --- a/tools/bpf/bpftool/map.c +++ b/tools/bpf/bpftool/map.c @@ -699,7 +699,7 @@ static int do_show(int argc, char **argv) if (show_pinned) { map_table = hashmap__new(hash_fn_for_key_as_id, equal_fn_for_key_as_id, NULL); - if (!map_table) { + if (IS_ERR(map_table)) { p_err("failed to create hashmap for pinned paths"); return -1; } diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c index 56b598eee043..7c384d10e95f 100644 --- a/tools/bpf/bpftool/pids.c +++ b/tools/bpf/bpftool/pids.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) /* Copyright (C) 2020 Facebook */ #include <errno.h> +#include <linux/err.h> #include <stdbool.h> #include <stdio.h> #include <stdlib.h> @@ -101,7 +102,7 @@ int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type) libbpf_print_fn_t default_print; *map = hashmap__new(hash_fn_for_key_as_id, equal_fn_for_key_as_id, NULL); - if (!*map) { + if (IS_ERR(*map)) { p_err("failed to create hashmap for PID references"); return -1; } diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 2a21d50516bc..33ca834d5f51 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -641,7 +641,7 @@ static int do_show(int argc, char **argv) if (show_pinned) { prog_table = hashmap__new(hash_fn_for_key_as_id, equal_fn_for_key_as_id, NULL); - if (!prog_table) { + if (IS_ERR(prog_table)) { p_err("failed to create hashmap for pinned paths"); return -1; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() 2022-01-07 15:26 ` [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() Mauricio Vásquez @ 2022-01-07 18:02 ` Quentin Monnet 2022-01-07 18:51 ` Song Liu 1 sibling, 0 replies; 6+ messages in thread From: Quentin Monnet @ 2022-01-07 18:02 UTC (permalink / raw) To: Mauricio Vásquez, netdev, bpf Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko 2022-01-07 10:26 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io> > hashmap__new() encodes errors with ERR_PTR(), hence it's not valid to > check the returned pointer against NULL and IS_ERR() has to be used > instead. > > libbpf_get_error() can't be used in this case as hashmap__new() is not > part of the public libbpf API and it'll continue using ERR_PTR() after > libbpf 1.0. > > Fixes: 8f184732b60b ("bpftool: Switch to libbpf's hashmap for pinned paths of BPF objects") > Fixes: 2828d0d75b73 ("bpftool: Switch to libbpf's hashmap for programs/maps in BTF listing") > Fixes: d6699f8e0f83 ("bpftool: Switch to libbpf's hashmap for PIDs/names references") > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io> This looks good to me, thank you for the fix! Reviewed-by: Quentin Monnet <quentin@isovalent.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() 2022-01-07 15:26 ` [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() Mauricio Vásquez 2022-01-07 18:02 ` Quentin Monnet @ 2022-01-07 18:51 ` Song Liu 1 sibling, 0 replies; 6+ messages in thread From: Song Liu @ 2022-01-07 18:51 UTC (permalink / raw) To: Mauricio Vásquez Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Quentin Monnet On Fri, Jan 7, 2022 at 7:26 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote: > > hashmap__new() encodes errors with ERR_PTR(), hence it's not valid to > check the returned pointer against NULL and IS_ERR() has to be used > instead. > > libbpf_get_error() can't be used in this case as hashmap__new() is not > part of the public libbpf API and it'll continue using ERR_PTR() after > libbpf 1.0. > > Fixes: 8f184732b60b ("bpftool: Switch to libbpf's hashmap for pinned paths of BPF objects") > Fixes: 2828d0d75b73 ("bpftool: Switch to libbpf's hashmap for programs/maps in BTF listing") > Fixes: d6699f8e0f83 ("bpftool: Switch to libbpf's hashmap for PIDs/names references") > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io> Acked-by: Song Liu <songliubraving@fb.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() 2022-01-07 15:26 [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Mauricio Vásquez 2022-01-07 15:26 ` [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() Mauricio Vásquez @ 2022-01-07 17:40 ` Song Liu 2022-01-07 22:00 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 6+ messages in thread From: Song Liu @ 2022-01-07 17:40 UTC (permalink / raw) To: Mauricio Vásquez Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Quentin Monnet On Fri, Jan 7, 2022 at 7:26 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote: > > hashmap__new() uses ERR_PTR() to return an error so it's better to > use IS_ERR_OR_NULL() in order to check the pointer before calling > free(). This will prevent freeing an invalid pointer if somebody calls > hashmap__free() with the result of a failed hashmap__new() call. > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io> Acked-by: Song Liu <songliubraving@fb.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() 2022-01-07 15:26 [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Mauricio Vásquez 2022-01-07 15:26 ` [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() Mauricio Vásquez 2022-01-07 17:40 ` [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Song Liu @ 2022-01-07 22:00 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2022-01-07 22:00 UTC (permalink / raw) To: =?utf-8?q?Mauricio_V=C3=A1squez_=3Cmauricio=40kinvolk=2Eio=3E?= Cc: netdev, bpf, ast, daniel, andrii, quentin Hello: This series was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Fri, 7 Jan 2022 10:26:19 -0500 you wrote: > hashmap__new() uses ERR_PTR() to return an error so it's better to > use IS_ERR_OR_NULL() in order to check the pointer before calling > free(). This will prevent freeing an invalid pointer if somebody calls > hashmap__free() with the result of a failed hashmap__new() call. > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io> > > [...] Here is the summary with links: - [bpf-next,1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() https://git.kernel.org/bpf/bpf-next/c/d793c2eb5dbc - [bpf-next,2/2] bpftool: Fix error check when calling hashmap__new() https://git.kernel.org/bpf/bpf-next/c/2318517920d1 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-07 22:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-07 15:26 [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Mauricio Vásquez 2022-01-07 15:26 ` [PATCH bpf-next 2/2] bpftool: Fix error check when calling hashmap__new() Mauricio Vásquez 2022-01-07 18:02 ` Quentin Monnet 2022-01-07 18:51 ` Song Liu 2022-01-07 17:40 ` [PATCH bpf-next 1/2] libbpf: Use IS_ERR_OR_NULL() in hashmap__free() Song Liu 2022-01-07 22:00 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).