netdev - Re: [PATCH bpf-next v4 0/3] xdp: Allow lookup into devmaps before redirect

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:   Thu, 13 Jun 2019 11:40:25 -0700
From:   "Jonathan Lemon" <jonathan.lemon@...il.com>
To:     "Toke Høiland-Jørgensen" <toke@...hat.com>
Cc:     netdev@...r.kernel.org,
        "Jesper Dangaard Brouer" <brouer@...hat.com>,
        "Daniel Borkmann" <daniel@...earbox.net>,
        "Alexei Starovoitov" <ast@...nel.org>,
        "David Miller" <davem@...emloft.net>
Subject: Re: [PATCH bpf-next v4 0/3] xdp: Allow lookup into devmaps before
 redirect



On 13 Jun 2019, at 4:17, Toke Høiland-Jørgensen wrote:

> When using the bpf_redirect_map() helper to redirect packets from XDP, 
> the eBPF
> program cannot currently know whether the redirect will succeed, which 
> makes it
> impossible to gracefully handle errors. To properly fix this will 
> probably
> require deeper changes to the way TX resources are allocated, but one 
> thing that
> is fairly straight forward to fix is to allow lookups into devmaps, so 
> programs
> can at least know when a redirect is *guaranteed* to fail because 
> there is no
> entry in the map. Currently, programs work around this by keeping a 
> shadow map
> of another type which indicates whether a map index is valid.
>
> This series contains two changes that are complementary ways to fix 
> this issue:
>
> - Moving the map lookup into the bpf_redirect_map() helper (and 
> caching the
>   result), so the helper can return an error if no value is found in 
> the map.
>   This includes a refactoring of the devmap and cpumap code to not 
> care about
>   the index on enqueue.
>
> - Allowing regular lookups into devmaps from eBPF programs, using the 
> read-only
>   flag to make sure they don't change the values.
>
> The performance impact of the series is negligible, in the sense that 
> I cannot
> measure it because the variance between test runs is higher than the 
> difference
> pre/post series.
>
> Changelog:
>
> v4:
>   - Fix a few nits from Andrii
>   - Lose the #defines in bpf.h and just compare the flags argument 
> directly to
>     XDP_TX in bpf_xdp_redirect_map().
>
> v3:
>   - Adopt Jonathan's idea of using the lower two bits of the flag 
> value as the
>     return code.
>   - Always do the lookup, and cache the result for use in 
> xdp_do_redirect(); to
>     achieve this, refactor the devmap and cpumap code to get rid the 
> bitmap for
>     selecting which devices to flush.
> v2:
>   - For patch 1, make it clear that the change works for any map type.
>   - For patch 2, just use the new BPF_F_RDONLY_PROG flag to make the 
> return
>     value read-only.
Acked-by: Jonathan Lemon <jonathan.lemon@...il.com>