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
| ||
|
Message-ID: <AM6PR05MB5879232068A16CF20F9E450ED17F0@AM6PR05MB5879.eurprd05.prod.outlook.com> Date: Fri, 22 Feb 2019 12:30:15 +0000 From: Maxim Mikityanskiy <maximmi@...lanox.com> To: Willem de Bruijn <willemdebruijn.kernel@...il.com> CC: "David S. Miller" <davem@...emloft.net>, Saeed Mahameed <saeedm@...lanox.com>, Willem de Bruijn <willemb@...gle.com>, Jason Wang <jasowang@...hat.com>, Eric Dumazet <edumazet@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Eran Ben Elisha <eranbe@...lanox.com>, Tariq Toukan <tariqt@...lanox.com> Subject: RE: [PATCH net-next v2 1/7] net: Don't set transport offset to invalid value > -----Original Message----- > From: Willem de Bruijn <willemdebruijn.kernel@...il.com> > Sent: 21 February, 2019 19:28 > To: Maxim Mikityanskiy <maximmi@...lanox.com> > Cc: David S. Miller <davem@...emloft.net>; Saeed Mahameed > <saeedm@...lanox.com>; Willem de Bruijn <willemb@...gle.com>; Jason Wang > <jasowang@...hat.com>; Eric Dumazet <edumazet@...gle.com>; > netdev@...r.kernel.org; Eran Ben Elisha <eranbe@...lanox.com>; Tariq Toukan > <tariqt@...lanox.com> > Subject: Re: [PATCH net-next v2 1/7] net: Don't set transport offset to > invalid value > > On Thu, Feb 21, 2019 at 7:40 AM Maxim Mikityanskiy <maximmi@...lanox.com> > wrote: > > > > If the socket was created with socket(AF_PACKET, SOCK_RAW, 0), > > skb->protocol will be unset, __skb_flow_dissect() will fail, and > > skb_probe_transport_header() will fall back to the offset_hint, making > > the resulting skb_transport_offset incorrect. > > > > If, however, there is no transport header in the packet, > > transport_header shouldn't be set to an arbitrary value. > > > > Fix it by leaving the transport offset unset if it couldn't be found, to > > be explicit rather than to fill it with some wrong value. It changes the > > behavior, but if some code relied on the old behavior, it would be > > broken anyway, as the old one is incorrect. > > > > Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com> > > qdisc_pkt_len_init also expects skb_transport_header(skb) to always be > set for gso packets. > > Once net is merged into net-next, commit d5be7f632bad ("net: validate This commit is already in net-next, isn't it? > untrusted gso packets without csum offload") will ensure that packets > that fail flow dissection do not make it into the stack. But we have > to skip dissection in some cases, like tun [1]. OK, got you. However, is everything OK with patch [1]? It fixes false positives, when a packet was dropped because network_header had not been set yet for dissection to succeed, but what about evil packets that have no network_offset at the moment of calling virtio_net_hdr_to_skb? Why are all of them considered valid? > I think we need to add a check in qdisc_pkt_len_init to skip the gso > size estimation branch if !skb_transport_header_was_set(skb). > > Otherwise this patch set looks good to me. To avoid resubmitting > everything we can fix up the qdisc_pkt_len_init in a follow-up, in > which case I'm happy to add my Acked-by to this series. I'll add this check and submit the patch soon. Thanks for reviewing! > [1] http://patchwork.ozlabs.org/patch/1044429/
Powered by blists - more mailing lists