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