mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: "Daniel Borkmann" <daniel@iogearbox.net>,
	"Lena Wang (王娜)" <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>,
	"Shiming Cheng (成诗明)" <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>
Subject: Re: [PATCH net] udp: fix segmentation crash for GRO packet without fraglist
Date: Mon, 29 Apr 2024 17:14:58 -0400	[thread overview]
Message-ID: <66300dd27bc48_2f15ff2944a@willemb.c.googlers.com.notmuch> (raw)
In-Reply-To: <880367ab-e9a2-d9b4-c6d6-9e2efdf04a0f@iogearbox.net>

> >> 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.

  reply	other threads:[~2024-04-29 21:14 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 15:01 shiming.cheng
2024-04-15 20:53 ` Willem de Bruijn
2024-04-16  2:14   ` Lena Wang (王娜)
2024-04-16  2:53     ` Maciej Żenczykowski
2024-04-16 17:16       ` Willem de Bruijn
2024-04-16 17:51         ` Maciej Żenczykowski
2024-04-16 17:57           ` Maciej Żenczykowski
2024-04-16 23:14             ` Willem de Bruijn
2024-04-17  7:19               ` Lena Wang (王娜)
2024-04-17 19:48                 ` Willem de Bruijn
2024-04-18  2:52                   ` Lena Wang (王娜)
2024-04-18  4:15                     ` Maciej Żenczykowski
2024-04-19  8:36                       ` Lena Wang (王娜)
2024-04-19 14:17                         ` Willem de Bruijn
2024-04-19 17:29                           ` Maciej Żenczykowski
2024-04-19 17:41                             ` Willem de Bruijn
2024-04-23 14:47                               ` Lena Wang (王娜)
2024-04-23 18:35                                 ` Willem de Bruijn
2024-04-24 12:22                                   ` Lena Wang (王娜)
2024-04-24 14:28                                     ` Willem de Bruijn
2024-04-25  4:32                                       ` Lena Wang (王娜)
2024-04-25 14:07                                         ` Willem de Bruijn
2024-04-26  9:52                                           ` Lena Wang (王娜)
2024-04-26 21:08                                             ` Daniel Borkmann
2024-04-27 13:28                                               ` Willem de Bruijn
2024-04-28  7:48                                                 ` Lena Wang (王娜)
2024-04-28 13:19                                                   ` Willem de Bruijn
2024-04-29 10:15                                                   ` Daniel Borkmann
2024-04-29 11:45                                                     ` Lena Wang (王娜)
2024-04-29 15:11                                                       ` Daniel Borkmann
2024-04-29 21:14                                                         ` Willem de Bruijn [this message]
2024-04-26  0:16                                         ` Maciej Żenczykowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=66300dd27bc48_2f15ff2944a@willemb.c.googlers.com.notmuch \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=Lena.Wang@mediatek.com \
    --cc=Shiming.Cheng@mediatek.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=maze@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=steffen.klassert@secunet.com \
    --cc=yan@cloudflare.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®