netdev - Re: [PATCH v3 1/5] net: add header len parameter to tun_get_socket(), tap_get

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CA+FuTSecOyH_k-jmLm_Ux4V9w0LOfWfVf6kuKfhOPU5DyD-wCw@mail.gmail.com>
Date:   Fri, 25 Jun 2021 14:13:28 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     David Woodhouse <dwmw2@...radead.org>
Cc:     netdev@...r.kernel.org, Jason Wang <jasowang@...hat.com>,
        Eugenio Pérez <eperezma@...hat.com>
Subject: Re: [PATCH v3 1/5] net: add header len parameter to tun_get_socket(), tap_get_socket()

On Thu, Jun 24, 2021 at 8:30 AM David Woodhouse <dwmw2@...radead.org> wrote:
>
> From: David Woodhouse <dwmw@...zon.co.uk>
>
> The vhost-net driver was making wild assumptions about the header length

If respinning, please more concretely describe which configuration is
currently broken.

IFF_NO_PI + IFF_VNET_HDR, if I understand correctly. But I got that
from the discussion, not the commit message.

> of the underlying tun/tap socket. Then it was discarding packets if
> the number of bytes it got from sock_recvmsg() didn't precisely match
> its guess.
>
> Fix it to get the correct information along with the socket itself.
> As a side-effect, this means that tun_get_socket() won't work if the
> tun file isn't actually connected to a device, since there's no 'tun'
> yet in that case to get the information from.
>
> On the receive side, where the tun device generates the virtio_net_hdr
> but VIRITO_NET_F_MSG_RXBUF was negotiated and vhost-net needs to fill

Nit: VIRTIO_NET_F_MSG_RXBUF

> in the 'num_buffers' field on top of the existing virtio_net_hdr, fix
> that to use 'sock_hlen - 2' as the location, which means that it goes

Please use sizeof(hdr.num_buffers) instead of a raw constant 2, to
self document the code.

Should this be an independent one-line fix?