netdev - Re: [PATCH iproute2 master] bpf: Fix race condition with map 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAOftzPiw3ZeCx30MYhg8KBbFe3-FKzKqitd7ipmsxiKgXOwJog@mail.gmail.com>
Date:   Thu, 19 Sep 2019 18:56:46 -0700
From:   Joe Stringer <joe@...d.net.nz>
To:     Joe Stringer <joe@...d.net.nz>
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        netdev <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH iproute2 master] bpf: Fix race condition with map pinning

On Thu, Sep 19, 2019 at 3:07 PM Joe Stringer <joe@...d.net.nz> wrote:
>
> If two processes attempt to invoke bpf_map_attach() at the same time,
> then they will both create maps, then the first will successfully pin
> the map to the filesystem and the second will not pin the map, but will
> continue operating with a reference to its own copy of the map. As a
> result, the sharing of the same map will be broken from the two programs
> that were concurrently loaded via loaders using this library.
>
> Fix this by adding a retry in the case where the pinning fails because
> the map already exists on the filesystem. In that case, re-attempt
> opening a fd to the map on the filesystem as it shows that another
> program already created and pinned a map at that location.
>
> Signed-off-by: Joe Stringer <joe@...d.net.nz>
> ---
>  lib/bpf.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/bpf.c b/lib/bpf.c
> index f64b58c3bb19..23eb8952cc28 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -1625,7 +1625,9 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
>                           int *have_map_in_map)
>  {
>         int fd, ifindex, ret, map_inner_fd = 0;
> +       bool retried = false;
>
> +probe:
>         fd = bpf_probe_pinned(name, ctx, map->pinning);
>         if (fd > 0) {
>                 ret = bpf_map_selfcheck_pinned(fd, map, ext,
> @@ -1674,7 +1676,11 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
>         }
>
>         ret = bpf_place_pinned(fd, name, ctx, map->pinning);
> -       if (ret < 0 && errno != EEXIST) {
> +       if (ret < 0) {
> +               if (!retried && errno == EEXIST) {
> +                       retried = true;
> +                       goto probe;
> +               }

Ah, forgot to close 'fd' before the jump in this retry case. Will fix
that up in v2.