Received: by 2002:a05:6500:1b45:b0:1f5:f2ab:c469 with SMTP id cz5csp285960lqb; Tue, 16 Apr 2024 16:14:58 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCUGKzqcvUmQUjG3NmtKjKcRErBQOSFZuGt9FIqsse2IC1cezT6X1ig5wRFS1Qpm4gDnK6Z08oiIIJGD59O6JS/c6sawVeVJHxfe9JZgfg== X-Google-Smtp-Source: AGHT+IGeeEYUbhBUOv6Vsvk6aHBwf1BR3Z87xkqX391dRJmy7BgQHFznkFmUzCCNOXqjRV/H+/da X-Received: by 2002:ac8:588c:0:b0:436:7870:575 with SMTP id t12-20020ac8588c000000b0043678700575mr17277925qta.15.1713309297825; Tue, 16 Apr 2024 16:14:57 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1713309297; cv=pass; d=google.com; s=arc-20160816; b=iyHAy+l0tnIkXI4EUIzf3XCvI6izfwWx9UnwXObiPBPHKTuq9l9XSSC17i/bwZ4PcZ Uv3bHRA3wknPekezBUSzAJ1wS30Z9KNifIGNoHDk/0nFiDteBddipctoVPE67eIYW1xE 7QegKqYnnU6rUdzF+S9um3kJGN3lSmGVkawkf2flU5k8ux4MsJryNUlHRPvtLv1yTq1X hjxSQtRv2IqRie64G9NK/rH2fs+akx8l8Pj8wNeEeJfztNZ6NkV/oxDObMVov6z1OWNd WH6nQBDuJaeoWCzjXqQm6R8ToIRhSsRC63m22PKHjtE89KvOO/LYioQ+BtOdLkeYG4Mk ZTpQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:subject:references:in-reply-to :message-id:cc:to:from:date:dkim-signature; bh=GjNK5/6z8Zr/0UT33xznjKY1QmMGq1le/CtfMG5Gbxc=; fh=e78okDoA8HcmvEaXEsWvv0p+rsVMEwojqfxus5vSMrA=; b=Aort85k4UCnMMlMQBTYxKzlVCkykMKEXP1KDByUZs0YlCg+PdoRh6ygXt82nINhl5y x6PtCMPuLpLnSYdWX9Qcy4rgcSC5WtDuUS+J58Ks0GyElNsJeUkFiGyIQBYdtbgsgqdN DCRZZLD0GMtXNB0wLsl4YMcnE4Mtvp/ey9u8Jfjkx6zGABDyjqa3MKYNeLceD0P/RNZv vtOcGh+XynkQERLgeDZlbfkOUIFxvmfCAOO4++qIyRH4kvFre5h1uY63AazMFSZHp90e L0hBvTQDUNYevdTGBqLdc8/S0D71b3kx6+etOvggcWYFd3k/VQ5ihEgKqXeflS/Yw8SL rCHw==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=Nyqi2uS+; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-147745-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-147745-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id v15-20020a05622a188f00b00434e08bb1d6si14095287qtc.595.2024.04.16.16.14.57 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Apr 2024 16:14:57 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-147745-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=Nyqi2uS+; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-147745-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-147745-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 295631C20BD3 for ; Tue, 16 Apr 2024 23:14:57 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id CB95613A890; Tue, 16 Apr 2024 23:14:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Nyqi2uS+" Received: from mail-qt1-f175.google.com (mail-qt1-f175.google.com [209.85.160.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B8E14AEE9; Tue, 16 Apr 2024 23:14:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.175 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713309283; cv=none; b=m3IGXiZqUi3ZoN+d4v2R/EFHt3LyfaOISo+YeXyLo6aKHZl66hJuwmoqrmtJNofKtLC+TmS0OFoaKwWLRiVUgWyc+4WmwfP1stVCg89WGUsWtZQvPkUqSVJ5JGTixVNukGQAxwdz/Zo64l3q0Tu9H7jae99rKQhJEO9F+0BqGAQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713309283; c=relaxed/simple; bh=Sw2FFGAUztuJzFb3aL9I8gL4fK/+KtIu9O7CvmghvDg=; h=Date:From:To:Cc:Message-ID:In-Reply-To:References:Subject: Mime-Version:Content-Type; b=uyxfaw4GsUS6T2ynvs+AlEAOiMaXYqndINziyQSN59VXvqeA1tYqczbPKlb8zWQdj6AB8skV35Acnl8CN1DtdK7+EmMlKTwtCIIS2FBHAn2b0raoT5gEVaPVrmIXTOkJp3bfSPkukbRVy3+43wLFrDyQyZ945M7+i+aJMSYizBo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Nyqi2uS+; arc=none smtp.client-ip=209.85.160.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-qt1-f175.google.com with SMTP id d75a77b69052e-4375b8b4997so4271571cf.2; Tue, 16 Apr 2024 16:14:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713309281; x=1713914081; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=GjNK5/6z8Zr/0UT33xznjKY1QmMGq1le/CtfMG5Gbxc=; b=Nyqi2uS+ZdLv2TXSmKC1amxx09asVwwuB5kS6LtYSwrXFXKr2AsmBYpwmp0BTxusk0 ZWGeLHY5xj11qb0vk55KEJ2KPPAP1Qk8Gd37ovvhxszqG/WiYDcxPnMopCl5y6PmHGlT w2PB47iY80BTCH9Vjo6QNnL1pVWhwi7260ib3rQhVJ5gMdnMRhwjiDBnbSiWSm72l/U1 MjQ7leEl/DuFNvi1N2X8EK1P0DawlwGah/+FV7np6tG1uvWn5BDR7DhhwvfAGFV4dVA7 uhky5bN3qR2oCHPW6hcKJpAWPsKQ8jLB9Kj8PdC8qSv2UsSFbzbuKXfR6GkRkoDXnAr8 b8Ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713309281; x=1713914081; h=content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=GjNK5/6z8Zr/0UT33xznjKY1QmMGq1le/CtfMG5Gbxc=; b=G83XzBD7GQS74cr2hWyRLuDLlvlh3R0/BIguH2ZVbeGj1bQBpCk5uaw5xfA9wdCmhQ 8IU/XrEVbKlAXqacpfibcBHRbJufXiAmSN4AkC7ShwA7HWOMo/GV5byS6e3OG7A/VTxm fvNL3j0rzjrspYm7J4tajmIO7kpOQaWLmEOTnzA08QYo1Ixg/iW+BoJob/agxz2o+44L y5WUlYN7VtX+8sNsmW4+85JJRDYl0bwdEl7ReNSMXPkIpfubY9m2RkScodjPUZhYK8Xd ftOaYHpdvd3rQpaXkBpBCkd1bc3GFzeylZSdwUjeqEan/ROOL+FTHnUQ9NB9TWQKjvhr MVVQ== X-Forwarded-Encrypted: i=1; AJvYcCUVNmJjD9e5cyfrV03Ejzh7yVtqBw1nFor2qmiaN9P2/yukKaHi4FdpqwIC0ykz2dDuo7gf3OzQB33RIp+PFb34o0uosUiDAdA8GWOpjqvYy0Ye8O6QqZIJCoQdOZFyhzw4UHVi71JkzpprUSvEaKN4qhyOIj1uV8AK X-Gm-Message-State: AOJu0YyqrWUPvk34B0cAMtog3ce2Yk/3Yed6vT/gSAY89lebuDZwpV8e SXldA2G0lqyP3LzHImXe20BszNHsbu9E9O9fFfiuYnAjwDW4bldp X-Received: by 2002:a05:622a:507:b0:434:3358:8990 with SMTP id l7-20020a05622a050700b0043433588990mr19527861qtx.21.1713309281086; Tue, 16 Apr 2024 16:14:41 -0700 (PDT) Received: from localhost (73.84.86.34.bc.googleusercontent.com. [34.86.84.73]) by smtp.gmail.com with ESMTPSA id a19-20020a05622a065300b004367cc4ab01sm6711814qtb.15.2024.04.16.16.14.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Apr 2024 16:14:40 -0700 (PDT) Date: Tue, 16 Apr 2024 19:14:40 -0400 From: Willem de Bruijn To: =?UTF-8?B?TWFjaWVqIMW7ZW5jenlrb3dza2k=?= , Willem de Bruijn Cc: =?UTF-8?B?TGVuYSBXYW5nICjnjovlqJwp?= , "steffen.klassert@secunet.com" , "kuba@kernel.org" , =?UTF-8?B?U2hpbWluZyBDaGVuZyAo5oiQ6K+X5piOKQ==?= , "pabeni@redhat.com" , "edumazet@google.com" , "matthias.bgg@gmail.com" , "davem@davemloft.net" , "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , bpf Message-ID: <661f066060ab4_7a39f2945d@willemb.c.googlers.com.notmuch> In-Reply-To: References: <20240415150103.23316-1-shiming.cheng@mediatek.com> <661d93b4e3ec3_3010129482@willemb.c.googlers.com.notmuch> <65e3e88a53d466cf5bad04e5c7bc3f1648b82fd7.camel@mediatek.com> <661eb25eeb09e_6672129490@willemb.c.googlers.com.notmuch> Subject: Re: [PATCH net] udp: fix segmentation crash for GRO packet without fraglist Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > > > > Personally, I think bpf_skb_pull_data() should have automatically= > > > > (ie. in kernel code) reduced how much it pulls so that it would p= ull > > > > headers only, > > > > > > That would be a helper that parses headers to discover header lengt= h. > > > > Does it actually need to? Presumably the bpf pull function could > > notice that it is > > a packet flagged as being of type X (UDP GSO FRAGLIST) and reduce the= pull > > accordingly so that it doesn't pull anything from the non-linear > > fraglist portion??? > > > > I know only the generic overview of what udp gso is, not any details,= so I am > > assuming here that there's some sort of guarantee to how these packet= s > > are structured... But I imagine there must be or we wouldn't be hitt= ing these > > issues deeper in the stack? > = > Perhaps for a packet of this type we're already guaranteed the headers > are in the linear portion, > and the pull should simply be ignored? > = > > > > > Parsing is better left to the BPF program. I do prefer adding sanity checks to the BPF helpers, over having to add then in the net hot path only to protect against dangerous BPF programs. In this case, it would be detecting this GSO type and failing the operation if exceeding skb_headlen(). > > > > > > > and not packet content. > > > > (This is assuming the rest of the code isn't ready to deal with a= longer pull, > > > > which I think is the case atm. Pulling too much, and then crashi= ng or forcing > > > > the stack to drop packets because of them being malformed seems w= rong...) > > > > > > > > In general it would be nice if there was a way to just say pull a= ll headers... > > > > (or possibly all L2/L3/L4 headers) > > > > You in general need to pull stuff *before* you've even looked at = the packet, > > > > so that you can look at the packet, > > > > so it's relatively hard/annoying to pull the correct length from = bpf > > > > code itself. > > > > > > > > > > > BPF needs to modify a proper length to do pull data. Howeve= r kernel > > > > > > > should also improve the flow to avoid crash from a bpf func= tion > > > > > > call. > > > > > > > As there is no split flow and app may not decode the merged= UDP > > > > > > packet, > > > > > > > we should drop the packet without fraglist in skb_segment_l= ist > > > > > > here. > > > > > > > > > > > > > > Fixes: 3a1296a38d0c ("net: Support GRO/GSO fraglist chainin= g.") > > > > > > > Signed-off-by: Shiming Cheng > > > > > > > Signed-off-by: Lena Wang > > > > > > > --- > > > > > > > net/core/skbuff.c | 3 +++ > > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > > > > > > > index b99127712e67..f68f2679b086 100644 > > > > > > > --- a/net/core/skbuff.c > > > > > > > +++ b/net/core/skbuff.c > > > > > > > @@ -4504,6 +4504,9 @@ struct sk_buff *skb_segment_list(stru= ct > > > > > > sk_buff *skb, > > > > > > > if (err) > > > > > > > goto err_linearize; > > > > > > > > > > > > > > +if (!list_skb) > > > > > > > +goto err_linearize; > > > > > > > + > > > > > > This would catch the case where the entire data frag_list is > > > linearized, but not a pskb_may_pull that only pulls in part of the > > > list. > > > > > > Even with BPF being privileged, the kernel should not crash if BPF > > > pulls a FRAGLIST GSO skb. > > > > > > But the check needs to be refined a bit. For a UDP GSO packet, I > > > think gso_size is still valid, so if the head_skb length does not > > > match gso_size, it has been messed with and should be dropped. > > > > > > For a GSO_BY_FRAGS skb, there is no single gso_size, and this pull > > > may be entirely undetectable as long as frag_list !=3D NULL? > > > > > > > > > > > > > skb_shinfo(skb)->frag_list =3D NULL; > > > > > > > > > > > > In absense of plugging the issue in BPF, dropping here is the= best > > > > > > we can do indeed, I think. > = > -- > Maciej =C5=BBenczykowski, Kernel Networking Developer @ Google