netdev - Re: [PATCH bpf-next] bpf, xdp: add bpf_redirect{,_map}() leaf node detection and optimization

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: <1f3efb08-498c-7e77-040d-5551e8237d17@intel.com>
Date:   Wed, 2 Dec 2020 09:01:41 +0100
From:   Björn Töpel <bjorn.topel@...el.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Björn Töpel <bjorn.topel@...il.com>
Cc:     ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        bpf@...r.kernel.org, kafai@...com, songliubraving@...com,
        yhs@...com, andrii@...nel.org, john.fastabend@...il.com,
        hawk@...nel.org, kuba@...nel.org, magnus.karlsson@...el.com,
        maciej.fijalkowski@...el.com
Subject: Re: [PATCH bpf-next] bpf, xdp: add bpf_redirect{,_map}() leaf node
 detection and optimization

On 2020-12-02 05:46, Alexei Starovoitov wrote:
[...]
> 
> Sorry I don't like this check at all. It's too fragile.
> It will work for one hard coded program.
> It may work for something more real, but will break with minimal
> changes to the prog or llvm changes.
> How are we going to explain that fragility to users?
>

[...]

> 
> I haven't looked through all possible paths, but it feels very dangerous.
> The stack growth is big. Calling xsk_rcv from preempt_disabled
> and recursively calling into another bpf prog?
> That violates all stack checks we have in the verifier.
>

Fair points, and thanks for pointing them out.

If the robustness (your first point) is improved, say via proper
indirect jump support, the stack usage will still be a concern.


> I see plenty of cons and not a single pro in this patch.
> 5% improvement for micro benchmark? That's hardly a justification.
> 

It's indeed a ubench, and something that is mostly beneficial to AF_XDP.
I'll go back to the drawing board and make sure the cons/pro balance is
improved.

Thanks for the feedback!


Björn