From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1934EC4338F for ; Fri, 6 Aug 2021 23:22:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DFAC661163 for ; Fri, 6 Aug 2021 23:22:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244764AbhHFXWx (ORCPT ); Fri, 6 Aug 2021 19:22:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:46268 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230280AbhHFXWw (ORCPT ); Fri, 6 Aug 2021 19:22:52 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 96B2061158; Fri, 6 Aug 2021 23:22:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628292155; bh=C6ywIFcePa4Rg5+pKYASIbZl4SqTp+oziLoHdx7Hrco=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Q9iCWQcw5s6RiIsyZ847VZIMq8WVPEEgek58P0pI7hRWotpKq5TH348eaO7wzJIZk O0d6J96F5tu097KlybmyZxdHWtf+jr+0Ce/GmMRC3pCVa9uwGS1IJxdg96En9eHCRq NfH0XP0cHulSofFC67dX3reaAS/VhrejllTBUfpMZ2IU8M7JSNY9prpfT6UubJdoux GZCaUDAl3ocFCa6U+TSQVX1ILxczc6GX5pcemzojLzIOSS8TIM22tMbLiupUO1WSJO cglm765PkDh1NMiJXQYbalZiT+TdykHdRFg4d384MfWbRIuYBvHz3sTDSQDwv2PxKG U1eOgSKWQgaBA== Date: Fri, 6 Aug 2021 16:22:34 -0700 From: Jakub Kicinski To: Guillaume Nault Cc: David Miller , netdev@vger.kernel.org, Martin Varghese , Willem de Bruijn Subject: Re: [PATCH net] bareudp: Fix invalid read beyond skb's linear data Message-ID: <20210806162234.69334902@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: <7741c46545c6ef02e70c80a9b32814b22d9616b3.1628264975.git.gnault@redhat.com> References: <7741c46545c6ef02e70c80a9b32814b22d9616b3.1628264975.git.gnault@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, 6 Aug 2021 17:52:06 +0200 Guillaume Nault wrote: > Data beyond the UDP header might not be part of the skb's linear data. > Use skb_copy_bits() instead of direct access to skb->data+X, so that > we read the correct bytes even on a fragmented skb. > > Fixes: 4b5f67232d95 ("net: Special handling for IP & MPLS.") > Signed-off-by: Guillaume Nault > --- > drivers/net/bareudp.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c > index a7ee0af1af90..54e321a695ce 100644 > --- a/drivers/net/bareudp.c > +++ b/drivers/net/bareudp.c > @@ -71,12 +71,18 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) > family = AF_INET6; > > if (bareudp->ethertype == htons(ETH_P_IP)) { > - struct iphdr *iphdr; > + __u8 ipversion; > > - iphdr = (struct iphdr *)(skb->data + BAREUDP_BASE_HLEN); > - if (iphdr->version == 4) { > - proto = bareudp->ethertype; > - } else if (bareudp->multi_proto_mode && (iphdr->version == 6)) { > + if (skb_copy_bits(skb, BAREUDP_BASE_HLEN, &ipversion, > + sizeof(ipversion))) { No preference just curious - could skb_header_pointer() be better suited? > + bareudp->dev->stats.rx_dropped++; > + goto drop; > + } > + ipversion >>= 4; > + > + if (ipversion == 4) { > + proto = htons(ETH_P_IP); > + } else if (ipversion == 6 && bareudp->multi_proto_mode) { > proto = htons(ETH_P_IPV6); > } else { > bareudp->dev->stats.rx_dropped++;