From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Axtens Subject: Re: [PATCH 0/3] Check gso_size of packets when forwarding Date: Fri, 19 Jan 2018 00:17:30 +1100 Message-ID: <87fu735ms5.fsf@linkitivity.dja.id.au> References: <20180116020920.20232-1-dja@axtens.net> <7bd81028-34bc-0784-3100-f30c8c9dbcf9@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: Linux Kernel Network Developers , Manish.Chopra@cavium.com, ovs dev To: Jason Wang , Pravin Shelar Return-path: Received: from mail-pg0-f65.google.com ([74.125.83.65]:43336 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755875AbeARNRf (ORCPT ); Thu, 18 Jan 2018 08:17:35 -0500 Received: by mail-pg0-f65.google.com with SMTP id n17so6576588pgf.10 for ; Thu, 18 Jan 2018 05:17:35 -0800 (PST) In-Reply-To: <7bd81028-34bc-0784-3100-f30c8c9dbcf9@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: Jason Wang writes: > On 2018=E5=B9=B401=E6=9C=8818=E6=97=A5 16:28, Pravin Shelar wrote: >> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens wrote: >>> When regular packets are forwarded, we validate their size against the >>> MTU of the destination device. However, when GSO packets are >>> forwarded, we do not validate their size against the MTU. We >>> implicitly assume that when they are segmented, the resultant packets >>> will be correctly sized. >>> >>> This is not always the case. >>> >>> We observed a case where a packet received on an ibmveth device had a >>> GSO size of around 10kB. This was forwarded by Open vSwitch to a bnx2x >>> device, where it caused a firmware assert. This is described in detail >>> at [0] and was the genesis of this series. Rather than fixing it in >>> the driver, this series fixes the forwarding path. >>> >> Are there any other possible forwarding path in networking stack? TC >> is one subsystem that could forward such a packet to the bnx2x device, >> how is that handled ? > > Yes, so it looks to me we should do the check in e.g netif_needs_gso()=20 > before passing it to hardware. And bnx2x needs to set its gso_max_size=20 > correctly. I don't think gso_max_size is quite the same. If I understand net/ipv4/tcp.c correctly, gso_max_size is supposed to cap the total length of the skb, not the length of each segmented packet. The problem for the bnx2x card is the length of the segment, not the overall length. > > Btw, looks like this could be triggered from a guest which is a DOS. We=20 > need request a CVE for this. > You are correct about how this can be triggered: in fact it came to my attention because a network packet from one LPAR (PowerVM virtual machine) brought down the card attached to a different LPAR. It didn't occur to me that it was potentially a security issue. I am talking with the security team at Canonical regarding this. Regards, Daniel > Thanks > >> >>> To fix this: >>> >>> - Move a helper in patch 1. >>> >>> - Validate GSO segment lengths in is_skb_forwardable() in the GSO >>> case, rather than assuming all will be well. This fixes bridges. >>> This is patch 2. >>> >>> - Open vSwitch uses its own slightly specialised algorithm for >>> checking lengths. Wire up checking for that in patch 3. >>> >>> [0]: https://patchwork.ozlabs.org/patch/859410/ >>> >>> Cc: Manish.Chopra@cavium.com >>> Cc: dev@openvswitch.org >>> >>> Daniel Axtens (3): >>> net: move skb_gso_mac_seglen to skbuff.h >>> net: is_skb_forwardable: validate length of GSO packet segments >>> openvswitch: drop GSO packets that are too large >>> >>> include/linux/skbuff.h | 16 ++++++++++++++++ >>> net/core/dev.c | 7 ++++--- >>> net/core/skbuff.c | 34 ++++++++++++++++++++++++++++++++++ >>> net/openvswitch/vport.c | 37 ++++++++++++++++++++++++++++++------- >>> net/sched/sch_tbf.c | 10 ---------- >>> 5 files changed, 84 insertions(+), 20 deletions(-) >>> >>> -- >>> 2.14.1 >>>