netdev - Re: [PATCH bpf-next 5/5] bpftool: Switch to libbpf's hashmap for PIDs/names references

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Oct 2021 18:50:17 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Quentin Monnet <quentin@...valent.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 5/5] bpftool: Switch to libbpf's hashmap for
 PIDs/names references

On Fri, Oct 22, 2021 at 10:16 AM Quentin Monnet <quentin@...valent.com> wrote:
>
> In order to show PIDs and names for processes holding references to BPF
> programs, maps, links, or BTF objects, bpftool creates hash maps to
> store all relevant information. This commit is part of a set that
> transitions from the kernel's hash map implementation to the one coming
> with libbpf.
>
> The motivation is to make bpftool less dependent of kernel headers, to
> ease the path to a potential out-of-tree mirror, like libbpf has.
>
> This is the third and final step of the transition, in which we convert
> the hash maps used for storing the information about the processes
> holding references to BPF objects (programs, maps, links, BTF), and at
> last we drop the inclusion of tools/include/linux/hashtable.h.
>
> Note: Checkpatch complains about the use of __weak declarations, and the
> missing empty lines after the bunch of empty function declarations when
> compiling without the BPF skeletons (none of these were introduced in
> this patch). We want to keep things as they are, and the reports should
> be safe to ignore.
>
> Signed-off-by: Quentin Monnet <quentin@...valent.com>
> ---
>  tools/bpf/bpftool/btf.c  |  7 ++--
>  tools/bpf/bpftool/link.c |  6 +--
>  tools/bpf/bpftool/main.c |  5 ++-
>  tools/bpf/bpftool/main.h | 17 +++-----
>  tools/bpf/bpftool/map.c  |  6 +--
>  tools/bpf/bpftool/pids.c | 84 ++++++++++++++++++++++------------------
>  tools/bpf/bpftool/prog.c |  6 +--
>  7 files changed, 67 insertions(+), 64 deletions(-)
>

[...]

>  #include "pid_iter.skel.h"
>
> -static void add_ref(struct obj_refs_table *table, struct pid_iter_entry *e)
> +static void add_ref(struct hashmap *map, struct pid_iter_entry *e)
>  {
> +       struct hashmap_entry *entry;
>         struct obj_refs *refs;
>         struct obj_ref *ref;
>         void *tmp;
>         int i;
>
> -       hash_for_each_possible(table->table, refs, node, e->id) {
> -               if (refs->id != e->id)
> -                       continue;
> +       hashmap__for_each_key_entry(map, entry, u32_as_hash_field(e->id)) {
> +               refs = entry->value;
>
>                 for (i = 0; i < refs->ref_cnt; i++) {
>                         if (refs->refs[i].pid == e->pid)
> @@ -64,7 +66,6 @@ static void add_ref(struct obj_refs_table *table, struct pid_iter_entry *e)
>                 return;
>         }
>
> -       refs->id = e->id;
>         refs->refs = malloc(sizeof(*refs->refs));
>         if (!refs->refs) {
>                 free(refs);
> @@ -76,7 +77,7 @@ static void add_ref(struct obj_refs_table *table, struct pid_iter_entry *e)
>         ref->pid = e->pid;
>         memcpy(ref->comm, e->comm, sizeof(ref->comm));
>         refs->ref_cnt = 1;
> -       hash_add(table->table, &refs->node, e->id);
> +       hashmap__append(map, u32_as_hash_field(e->id), refs);

here as well, can fail

>  }
>
>  static int __printf(2, 0)
> @@ -87,7 +88,7 @@ libbpf_print_none(__maybe_unused enum libbpf_print_level level,
>         return 0;
>  }
>

[...]