netdev - Re: [PATCH bpf] libbpf: sanitise map names before pinning

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 1 Dec 2020 16:39:48 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...com>,
        Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf] libbpf: sanitise map names before pinning

On Mon, Nov 30, 2020 at 8:17 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> When we added sanitising of map names before loading programs to libbpf, we
> still allowed periods in the name. While the kernel will accept these for
> the map names themselves, they are not allowed in file names when pinning

That sounds like an unnecessary difference in kernel behavior. If the
kernel allows maps with '.' in the name, why not allow to pin it?
Should we fix that in the kernel?

> maps. This means that bpf_object__pin_maps() will fail if called on an
> object that contains internal maps (such as sections .rodata).
>
> Fix this by replacing periods with underscores when constructing map pin
> paths. This only affects the paths generated by libbpf when
> bpf_object__ping_maps() is called with a path argument. Any pin paths set
> by bpf_map__set_pin_path() are unaffected, and it will still be up to the
> caller to avoid invalid characters in those.
>
> Fixes: 113e6b7e15e2 ("libbpf: Sanitise internal map names so they are not rejected by the kernel")
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
>  tools/lib/bpf/libbpf.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8d05132e1945..8a3b4713b356 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7665,8 +7665,8 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
>         }
>
>         bpf_object__for_each_map(map, obj) {
> +               char buf[PATH_MAX], *s = buf;
>                 char *pin_path = NULL;
> -               char buf[PATH_MAX];
>
>                 if (path) {
>                         int len;
> @@ -7680,6 +7680,8 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
>                                 err = -ENAMETOOLONG;
>                                 goto err_unpin_maps;
>                         }
> +                       while ((s = strstr(s, ".")))
> +                           *s = '_';

Let's extract this into a helper method?

>                         pin_path = buf;
>                 } else if (!map->pin_path) {
>                         continue;
> @@ -7712,8 +7714,8 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
>                 return -ENOENT;
>
>         bpf_object__for_each_map(map, obj) {
> +               char buf[PATH_MAX], *s = buf;
>                 char *pin_path = NULL;
> -               char buf[PATH_MAX];
>
>                 if (path) {
>                         int len;
> @@ -7724,6 +7726,8 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
>                                 return -EINVAL;
>                         else if (len >= PATH_MAX)
>                                 return -ENAMETOOLONG;
> +                       while ((s = strstr(s, ".")))
> +                           *s = '_';
>                         pin_path = buf;
>                 } else if (!map->pin_path) {
>                         continue;
> --
> 2.29.2
>