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]
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.

Powered by blists - more mailing lists