linux-kernel - Re: vhost-net

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:   Tue, 17 Jan 2023 12:26:57 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Jonas Bonn <jonas@...rbonn.se>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: vhost-net

On Mon, Jan 16, 2023 at 4:59 PM Jonas Bonn <jonas@...rbonn.se> wrote:
>
> Hi,
>
> I have a question about using vhost-net with an IFF_TUN device.  I'm
> uncertain about the behaviour I'm seeing which is:
>
> i)  on RX, the descriptor contains no Ethernet header, which is what I
> was expecting
> ii)  on TX, the first 14 bytes of the transmitted _IP_ packet are lost;
> if I prepend an extra 14 bytes (zeroes) before the IP packet and extend
> the packet length accordingly, then things appear to work as expected.
>
> In vhost_net_build_xdp() it appears that the userspace packet data is
> copied verbatim to an XDP packet structure that assumes the presence of
> an ethernet header; as such, the IP header is copied into the ethernet
> header area.  I think this accounts for losing the first 14 bytes of the
> IP header...
>
> If I set SO_SNDBUF to something less than INT_MAX, then the XDP path is
> bypassed and transmission of IP packets works.  This means that knowing
> the value of SO_SNDBUF becomes important in the userspace application in
> order to know whether an extra 14 bytes needs to be prepended to the IP
> packet... which is awkward, at best.

It's a bug.

>
> For an IFF_TUN device, should vhost-net not be adding an implicit
> ethernet header in _build_xdp()?

Probably.

Actually, this makes me think that we should disable XDP for TUN?

> Can this be done without backward
> compatibility implications?
>

The path is used by vhost-net only, so I think we are fine.

Patch is more than welcomed.

Thanks

> Thanks,
> Jonas
>