netdev - RE: [RFC] AF_PACKET transport

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: <AM6PR05MB58796304392E387C82248DEED1BB0@AM6PR05MB5879.eurprd05.prod.outlook.com>
Date:   Mon, 24 Dec 2018 08:33:18 +0000
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Jason Wang <jasowang@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: RE: [RFC] AF_PACKET transport_offset fix

Thanks for the feedback!

> This sounds good to me.
>
> With the caveat that we have to make sure that no path depends on the
> transport header always being set, and that this change in
> skb_probe_transport_header does not cause unintended side effects
> for the other users (tun/tap/xen). But I expect that to be fine.

I also hope that no code should depend on transport header being set to a wrong
value :). Regarding tun, tap and xen, I looked at their code where
skb_probe_transport_header is used, and they all set skb->protocol before
calling it, so it should succeed, unless there is no L3 in the packet. So, we'll
only have transport_header == ~0U when there is none, which looks correct.

> Optionally, as a separate follow-up patch we could add a value -2U that
> denotes transport header is unset and already parsed. To avoid additional
> flow dissection attempts. But this will have to be cleared on various
> operations, such as cross-netns forwarding or packet mangling. So
> probably not worth the effort.

I also had this idea, but IMO it's better to avoid extra dissection attempts by
just not having multiple calls to __skb_flow_dissect in the path, which should
be doable at first sight. And if we need to reset this special value at some
points, it can indeed negate the effort, so I agree that it is not needed for
now.