netdev - Re: [PATCH RFC v1] net: xdp: allow for layer 3 packets in generic skb handler

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:   Mon, 27 Apr 2020 09:20:34 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>, netdev@...r.kernel.org
Subject: Re: [PATCH RFC v1] net: xdp: allow for layer 3 packets in generic skb handler

"Jason A. Donenfeld" <Jason@...c4.com> writes:

> A user reported a few days ago that packets from wireguard were possibly
> ignored by XDP [1]. We haven't heard back from the original reporter to
> receive more info, so this here is mostly speculative. Successfully nerd
> sniped, Toke and I started poking around. Toke noticed that the generic
> skb xdp handler path seems to assume that packets will always have an
> ethernet header, which really isn't always the case for layer 3 packets,
> which are produced by multiple drivers. This patch is untested, but I
> wanted to gauge interest in this approach: if the mac_len is 0, then we
> assume that it's a layer 3 packet, and figure out skb->protocol from
> looking at the IP header. This patch also adds some stricter testing
> around mac_len before we assume that it's an ethhdr.

While your patch will fix the header pointer mangling for the skb, it
unfortunately won't fix generic XDP for Wireguard: The assumption that
there's an Ethernet header present is made for compatibility with native
XDP, so you might say it's deliberate. I.e., the eBPF programs running
in the XDP hook expect to see an Ethernet header as part of the packet
data (and parses the packet like in [0]).

So, to make XDP generic work for Wireguard (or other IP-header-only
devices) we'd need to either (1) introduce a new XDP sub-type that
assumes L4 packets, or (2) make Wireguard add a fake Ethernet header to
the head of the packet and set the skb mac_header accordingly.

We've discussed (1) before in other contexts (specifically, adding a
802.11 sub-type), but IIRC we decided that there wasn't enough interest.
I wonder if the same wouldn't be the case for an IP sub-type, since
users would have to re-write their XDP programs to fit that hook type,
and it would only be usable for generic XDP on certain tunnel interface
types. Not sure about the feasibility of (2).

-Toke

[0] https://github.com/xdp-project/xdp-tutorial/blob/master/packet01-parsing/xdp_prog_kern.c