netdev - Re: [PATCH net-next v3 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to

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]

Message-ID: <CA+FuTSdEF7dONWZWR3t9EZ5VU3XrfWTb0CmWKe7pQBL-tje0WA@mail.gmail.com>
Date:   Thu, 3 Jun 2021 23:51:27 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     Tanner Love <tannerlove.kernel@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Petar Penkov <ppenkov@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Tanner Love <tannerlove@...gle.com>,
        "Michael S. Tsirkin" <mst@...hat.com>
Subject: Re: [PATCH net-next v3 0/3] virtio_net: add optional flow dissection
 in virtio_net_hdr_to_skb

On Thu, Jun 3, 2021 at 10:55 PM Jason Wang <jasowang@...hat.com> wrote:
>
>
> 在 2021/6/2 上午6:18, Tanner Love 写道:
> > From: Tanner Love <tannerlove@...gle.com>
> >
> > First patch extends the flow dissector BPF program type to accept
> > virtio-net header members.
> >
> > Second patch uses this feature to add optional flow dissection in
> > virtio_net_hdr_to_skb(). This allows admins to define permitted
> > packets more strictly, for example dropping deprecated UDP_UFO
> > packets.
> >
> > Third patch extends kselftest to cover this feature.
>
>
> I wonder why virtio maintainers is not copied in this series.

Sorry, an oversight.

> Several questions:
>
> 1) having bpf core to know about virito-net header seems like a layer
> violation, it doesn't scale as we may add new fields, actually there's
> already fields that is not implemented in the spec but not Linux right now.

struct virtio_net_hdr is used by multiple interfaces, not just virtio.
The interface as is will remain, regardless of additional extensions.

If the interface is extended, the validation can be extended with it.

Just curious: can you share what extra fields may be in the pipeline?
The struct has historically not seen (m)any changes.

> 2) virtio_net_hdr_to_skb() is not the single entry point, packet could
> go via XDP

Do you mean AF_XDP? As far as I know, vnet_hdr is the only injection
interface for complex packets that include offload instructions (GSO,
csum) -- which are the ones mostly implicated in bug reports.

> 3) I wonder whether we can simply use XDP to solve this issue (metadata
> probably but I don't have a deep thought)
> 4) If I understand the code correctly, it should deal with all dodgy
> packets instead of just for virtio

Yes. Some callers of virtio_net_hdr_to_skb, such as tun_get_user and
virtio receive_buf, pass all packets to it. Others, like tap_get_user
and packet_snd, only call it if a virtio_net_hdr is passed. Once we
have a validation hook, ideally all packets need to pass it. Modifying
callers like tap_get_user can be a simple follow-on.