Received-SPF: pass (google.com: domain of linux-kernel+bounces-163061-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 29 Apr 2024 17:14:58 -0400
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Daniel Borkmann <daniel@iogearbox.net>, 
 =?UTF-8?B?TGVuYSBXYW5nICjnjovlqJwp?= <Lena.Wang@mediatek.com>, 
 "maze@google.com" <maze@google.com>, 
 "willemdebruijn.kernel@gmail.com" <willemdebruijn.kernel@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
 "bpf@vger.kernel.org" <bpf@vger.kernel.org>, 
 "steffen.klassert@secunet.com" <steffen.klassert@secunet.com>, 
 "kuba@kernel.org" <kuba@kernel.org>, 
 =?UTF-8?B?U2hpbWluZyBDaGVuZyAo5oiQ6K+X5piOKQ==?= <Shiming.Cheng@mediatek.com>, 
 "pabeni@redhat.com" <pabeni@redhat.com>, 
 "edumazet@google.com" <edumazet@google.com>, 
 "netdev@vger.kernel.org" <netdev@vger.kernel.org>, 
 "matthias.bgg@gmail.com" <matthias.bgg@gmail.com>, 
 "davem@davemloft.net" <davem@davemloft.net>, 
 "yan@cloudflare.com" <yan@cloudflare.com>
Message-ID: <66300dd27bc48_2f15ff2944a@willemb.c.googlers.com.notmuch>
In-Reply-To: <880367ab-e9a2-d9b4-c6d6-9e2efdf04a0f@iogearbox.net>
References: <20240415150103.23316-1-shiming.cheng@mediatek.com>
 <66227ce6c1898_116a9b294be@willemb.c.googlers.com.notmuch>
 <CANP3RGfxeKDUmGwSsZrAs88Fmzk50XxN+-MtaJZTp641aOhotA@mail.gmail.com>
 <6622acdd22168_122c5b2945@willemb.c.googlers.com.notmuch>
 <9f097bcafc5bacead23c769df4c3f63a80dcbad5.camel@mediatek.com>
 <6627ff5432c3a_1759e929467@willemb.c.googlers.com.notmuch>
 <274c7e9837e5bbe468d19aba7718cc1cf0f9a6eb.camel@mediatek.com>
 <66291716bcaed_1a760729446@willemb.c.googlers.com.notmuch>
 <c28a5c635f38a47f1be266c4328e5fbba44ff084.camel@mediatek.com>
 <662a63aeee385_1de39b294fd@willemb.c.googlers.com.notmuch>
 <752468b66d2f5766ea16381a0c5d7b82ab77c5c4.camel@mediatek.com>
 <ae0ba22a-049a-49c1-d791-d0e953625904@iogearbox.net>
 <662cfd6db06df_28b9852949a@willemb.c.googlers.com.notmuch>
 <afa6e302244a87c2a834fcc31d48b377e19a34a2.camel@mediatek.com>
 <5cc1c662-1cec-101c-8184-c32c210eeadc@iogearbox.net>
 <bd9d5fef2fa6154e162e963f5d669ff618b95229.camel@mediatek.com>
 <880367ab-e9a2-d9b4-c6d6-9e2efdf04a0f@iogearbox.net>
Subject: Re: [PATCH net] udp: fix segmentation crash for GRO packet without
 fraglist
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit

> >> The 'write_len > skb_headlen(skb)' test is redundant, no ?
> >>
> >> It is covered by the earlier test :
> >>
> >>           if (likely(len <= skb_headlen(skb)))
> >>                   return SKB_NOT_DROPPED_YET;
> >>
> > Daniel, it is not redundant. The bpf pulls a len between
> > skb_headlen(skb) and skb->len that results in error. Here it will stop
> > this operation. For other skbs(not SKB_GSO_FRAGLIST) it could be a
> > normal behaviour and will continue to do next pulling.
> 
> I meant something like the below. The len <= skb_headlen(skb) case you
> already return earlier with SKB_NOT_DROPPED_YET. Willem, do you see a
> case where this should not live in pskb_may_pull_reason() but rather
> specifically in skb_ensure_writable()?

Yes. pskb_may_pull is called all over the hot path. All in locations
that are known safe, because they only pull header bytes. I prefer to
limit the branch to the few (user configurable) locations that are in
scope.