From mboxrd@z Thu Jan 1 00:00:00 1970 From: Willem de Bruijn Subject: Re: [PATCH net] net: validate untrusted gso packets Date: Thu, 18 Jan 2018 19:53:39 -0500 Message-ID: References: <20180116202914.205914-1-willemdebruijn.kernel@gmail.com> <39e93ed0-3e2c-20bd-ebb1-deeb37940175@redhat.com> <28e14b63-5756-3384-ab16-f70911779f2a@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Network Development , David Miller , Eric Dumazet , Willem de Bruijn To: Jason Wang Return-path: Received: from mail-oi0-f53.google.com ([209.85.218.53]:42392 "EHLO mail-oi0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750784AbeASAyU (ORCPT ); Thu, 18 Jan 2018 19:54:20 -0500 Received: by mail-oi0-f53.google.com with SMTP id o64so53058oia.9 for ; Thu, 18 Jan 2018 16:54:20 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: >>> This implements subset of function for codes which was removed by the >>> commit >>> I mentioned below. >> >> No, as I explain above, it performs a different check. >> >>>>> > > [...] Clearly I was wrong, sorry. Thanks for pointing out that commit and 576a30eb6453 ("[NET]: Added GSO header verification"). >>>>> For performance reason. I think we should delay the check or >>>>> segmentation >>>>> as >>>>> much as possible until it was really needed. >>>> >>>> Going through segmentation is probably as expensive as flow dissector, >>>> if not more so because of the indirect branches. >>> >>> I think we don't even need to care about this consider the evil packet >>> should be rare. >> >> How does frequency matter when a single packet can crash a host? > > > I mean consider we had fix the crash, we don't care how expensive do we spot > this. > >> >>> And what you propose here is just a very small subset of the >>> necessary checking, more comes at gso header checking. So even if we care >>> performance, it only help for some specific case. >> >> It also fixed the bug that Eric sent a separate patch for, as that did >> not dissect as a valid TCP packet, either. > > > I may miss something but how did this patch protects an evil thoff? Actually, it blocked that specific reproducer because the ip protocol did not match. I think that __skb_flow_dissect_tcp should return a boolean, causing dissection return FLOW_DISSECT_RET_OUT_BAD if the tcph is bad. That would be needed to really catch it with flow dissection at the source.