* [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 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 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 ` [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).