Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp8079147pxb; Fri, 19 Feb 2021 06:59:12 -0800 (PST) X-Google-Smtp-Source: ABdhPJz3ZAeViFgvwl/+fdSjI9SoqWxvHwAWeMTjvw6LhhvPVIqkX+BY0eVxsGGgLGp0bS53iNYH X-Received: by 2002:a17:906:fb14:: with SMTP id lz20mr9112261ejb.391.1613746752668; Fri, 19 Feb 2021 06:59:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1613746752; cv=none; d=google.com; s=arc-20160816; b=M5rYPIhOsIRDVGRvfBJEBoyYQlDAxirjizmiTel2reyquQSHucSYQYGS9Q4027grxt PgrzEOwpQnHuNiypJPKAB/RjeJinQfxuBzvDrQw3kZPoSnDmd7CiSqgPMX1xzLpKU51Q uEE49UUqxKnmmTaxaJqvbUnSKgPL2jzpul2nn40g5A0EwrnnMtDvAncDvfborPFPHsJR s0lvbmu+UcVJWRJhzYIQP55EW93ZEc2IdC+7nokfg7YfrGilg/3HJsp48Nvcj9cw2tNX 3pt8SKspCgErB4gnHqSeAVL6qR0vTOBMkgqlzzFsi+GMcFAgbWbr32S00T6SDrQH12nM 2SJA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=9KW+1lIwPV9FPrnZg5vNSqypATsMZae7KiNb0eudmHM=; b=Tk3dcd4S+ZqJUfmsmANrBDtvaPk2Y5yXWzmJoMmCM61ItDwWbHSsdVsL7PCYEGJgpA MiMO/uq3NejzLGn25TGroXtZiATOq/JMUT30ml8bHiHmNbytAcAC29tZLiJ5khp3ugCe N42NZamHTA0piyy2LdgmoeZt43UEhtqhsWKTqkWEhNqQAtnuRL2x7GqOaUGh/mYUAjsH qUXFiM6k8B+uvAKrXDcdJY3tzEYuc6fgDw2AaT/EuYe7HoQnoujXjI4AIylDdRhD5UJu 99M8E57WTB3vXCbQqrkRag8POxaAFx/ptsn4W1r09B/cOn+xediclz4btDHTOrsbiDYU ZGZw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=fWlizlEu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r16si6410539ejo.467.2021.02.19.06.58.38; Fri, 19 Feb 2021 06:59:12 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=fWlizlEu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229755AbhBSO4x (ORCPT + 99 others); Fri, 19 Feb 2021 09:56:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49254 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229620AbhBSO4v (ORCPT ); Fri, 19 Feb 2021 09:56:51 -0500 Received: from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com [IPv6:2a00:1450:4864:20::52a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EB554C061574 for ; Fri, 19 Feb 2021 06:56:09 -0800 (PST) Received: by mail-ed1-x52a.google.com with SMTP id d2so10404164edq.10 for ; Fri, 19 Feb 2021 06:56:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=9KW+1lIwPV9FPrnZg5vNSqypATsMZae7KiNb0eudmHM=; b=fWlizlEuP6fQFXcSfr57FExVLI3CPIV2N2m+q43FPhwTzlg8qFqVtR2ot7crvh/QW2 Qs8fcmRxZoRaSthNGmlfJA5Rv1YBC4S9VvZN9TBF+pu585Yd1tSv+dganVhM4259iily Wk8X2dC0CiMSntdm2q50kxGpwkPvEF2mKeZbx3Rd9w0aV1hEjvzOeSYK91djuqD/9uRz IybFBgnlTsOuU0j8UxqZHZhzAmejyXt6wOt88B/zjfNk8g3b8pKf0IPPwq3gTBJNFb6m sZQFyr10ifWODIk6m67nbAeupaz3rX9483lUva4ErH/Z1+sVoqOuBKu+W0aPvF0RE7bF LqFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=9KW+1lIwPV9FPrnZg5vNSqypATsMZae7KiNb0eudmHM=; b=OhBxO+VbPtVLSzRz8VqPIRkbRDiGzVntowa8afxdxOeLAN4gFz//u4eJLnu5yXOCSZ GOCmHVYfXSvOi9lCygQTD6mlWrHYAny50AVlgNrwDH9yrHWWjAEqmzXoL5c7odGmBPo9 OtrzZXiSYVUmzWqSWy/m4+i1IQrc9MCLiEiisuqpOGkT3MwLEVTFWlzgODkRAjboyia8 nyKrB5oGlcUpuKdHYxiyGBpsl6jrfemtwPzbFvh2N+o0WxyEd3G+aupSxkv5qx7dndq2 vD69e0ItImUZRSUyA5obbOh57WTKWcbD3Xv5Ajnj/bwxDl9TKiGHBCNvm9VWj5dAwjVZ cdjQ== X-Gm-Message-State: AOAM533iyYVm9dGZHe+YeHiHTBpdH4gN4IGNJZc7hGI0k1lmWh4NWmx+ xTQHFlxQS4xi9QFjfG4ZU2Q5/3OlD2k= X-Received: by 2002:a50:be8b:: with SMTP id b11mr9547463edk.145.1613746568138; Fri, 19 Feb 2021 06:56:08 -0800 (PST) Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com. [209.85.221.49]) by smtp.gmail.com with ESMTPSA id r6sm5331879edm.23.2021.02.19.06.56.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Feb 2021 06:56:06 -0800 (PST) Received: by mail-wr1-f49.google.com with SMTP id n4so8962758wrx.1 for ; Fri, 19 Feb 2021 06:56:06 -0800 (PST) X-Received: by 2002:a5d:4488:: with SMTP id j8mr8312106wrq.12.1613746565793; Fri, 19 Feb 2021 06:56:05 -0800 (PST) MIME-Version: 1.0 References: <5e910d11a14da17c41317417fc41d3a9d472c6e7.1613659844.git.bnemeth@redhat.com> <2cc06597-8005-7be8-4094-b20f525afde8@redhat.com> In-Reply-To: <2cc06597-8005-7be8-4094-b20f525afde8@redhat.com> From: Willem de Bruijn Date: Fri, 19 Feb 2021 09:55:28 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] net: check if protocol extracted by virtio_net_hdr_set_proto is correct To: Jason Wang Cc: Balazs Nemeth , Network Development , linux-kernel , "Michael S. Tsirkin" , David Miller , virtualization@lists.linux-foundation.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 19, 2021 at 3:53 AM Jason Wang wrote: > > > On 2021/2/18 11:50 =E4=B8=8B=E5=8D=88, Willem de Bruijn wrote: > > On Thu, Feb 18, 2021 at 10:01 AM Balazs Nemeth wro= te: > >> For gso packets, virtio_net_hdr_set_proto sets the protocol (if it isn= 't > >> set) based on the type in the virtio net hdr, but the skb could contai= n > >> anything since it could come from packet_snd through a raw socket. If > >> there is a mismatch between what virtio_net_hdr_set_proto sets and > >> the actual protocol, then the skb could be handled incorrectly later > >> on by gso. > >> > >> The network header of gso packets starts at 14 bytes, but a specially > >> crafted packet could fool the call to skb_flow_dissect_flow_keys_basic > >> as the network header offset in the skb could be incorrect. > >> Consequently, EINVAL is not returned. > >> > >> There are even packets that can cause an infinite loop. For example, a > >> packet with ethernet type ETH_P_MPLS_UC (which is unnoticed by > >> virtio_net_hdr_to_skb) that is sent to a geneve interface will be > >> handled by geneve_build_skb. In turn, it calls > >> udp_tunnel_handle_offloads which then calls skb_reset_inner_headers. > >> After that, the packet gets passed to mpls_gso_segment. That function > >> calculates the mpls header length by taking the difference between > >> network_header and inner_network_header. Since the two are equal > >> (due to the earlier call to skb_reset_inner_headers), it will calculat= e > >> a header of length 0, and it will not pull any headers. Then, it will > >> call skb_mac_gso_segment which will again call mpls_gso_segment, etc..= . > >> This leads to the infinite loop. > > > I remember kernel will validate dodgy gso packets in gso ops. I wonder > why not do the check there? The reason is that virtio/TUN is not the > only source for those packets. It is? All other GSO packets are generated by the stack itself, either locally or through GRO. But indeed some checks are better performed in the GSO layer. Such as likely the 0-byte mpls header length. If we cannot trust virtio_net_hdr.gso_type passed from userspace, then we can also not trust the eth.h_proto coming from the same source. But it makes sense to require them to be consistent. There is a dev_parse_header_protocol that may return the link layer type in a more generic fashion than casting to skb_eth_hdr. Question remains what to do for the link layer types that do not implement header_ops->parse_protocol, and so we cannot validate the packet's network protocol. Drop will cause false positives, accepts will leave a potential path, just closes it for Ethernet. This might call for multiple fixes, both on first ingest and inside the sta= ck?