netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	"Network Development" <netdev@vger.kernel.org>,
	"Karlsson, Magnus" <magnus.karlsson@intel.com>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Stanislav Fomichev" <stfomichev@gmail.com>,
	"Alexander Lobakin" <aleksander.lobakin@intel.com>,
	syzbot+ff145014d6b0ce64a173@syzkaller.appspotmail.com,
	"Ihor Solodrai" <ihor.solodrai@linux.dev>,
	"Octavian Purdila" <tavip@google.com>
Subject: Re: [PATCH bpf 1/2] xdp: update xdp_rxq_info's mem type in XDP generic hook
Date: Tue, 7 Oct 2025 21:39:24 +0200	[thread overview]
Message-ID: <aOVsbMAkfc5gv0vO@boxer> (raw)
In-Reply-To: <CAADnVQLGocfOT224=9_nJZ6093QDh1M_EDLQ3cNVQZKEDnjwog@mail.gmail.com>

On Fri, Oct 03, 2025 at 10:29:08AM -0700, Alexei Starovoitov wrote:
> On Fri, Oct 3, 2025 at 7:03 AM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > Currently, generic XDP hook uses xdp_rxq_info from netstack Rx queues
> > which do not have its XDP memory model registered. There is a case when
> > XDP program calls bpf_xdp_adjust_tail() BPF helper that releases
> > underlying memory. This happens when it consumes enough amount of bytes
> > and when XDP buffer has fragments. For this action the memory model
> > knowledge passed to XDP program is crucial so that core can call
> > suitable function for freeing/recycling the page.
> >
> > For netstack queues it defaults to MEM_TYPE_PAGE_SHARED (0) due to lack
> > of mem model registration. The problem we're fixing here is when kernel
> > copied the skb to new buffer backed by system's page_pool and XDP buffer
> > is built around it. Then when bpf_xdp_adjust_tail() calls
> > __xdp_return(), it acts incorrectly due to mem type not being set to
> > MEM_TYPE_PAGE_POOL and causes a page leak.
> >
> > For this purpose introduce a small helper, xdp_update_mem_type(), that
> > could be used on other callsites such as veth which are open to this
> > problem as well. Here we call it right before executing XDP program in
> > generic XDP hook.
> >
> > This problem was triggered by syzbot as well as AF_XDP test suite which
> > is about to be integrated to BPF CI.
> >
> > Reported-by: syzbot+ff145014d6b0ce64a173@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/netdev/6756c37b.050a0220.a30f1.019a.GAE@google.com/
> > Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
> > Tested-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> > Co-developed-by: Octavian Purdila <tavip@google.com>
> > Signed-off-by: Octavian Purdila <tavip@google.com> # whole analysis, testing, initiating a fix
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> # commit msg and proposed more robust fix
> > ---
> >  include/net/xdp.h | 7 +++++++
> >  net/core/dev.c    | 2 ++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/include/net/xdp.h b/include/net/xdp.h
> > index f288c348a6c1..5568e41cc191 100644
> > --- a/include/net/xdp.h
> > +++ b/include/net/xdp.h
> > @@ -336,6 +336,13 @@ xdp_update_skb_shared_info(struct sk_buff *skb, u8 nr_frags,
> >         skb->pfmemalloc |= pfmemalloc;
> >  }
> >
> > +static inline void
> > +xdp_update_mem_type(struct xdp_buff *xdp)
> > +{
> > +       xdp->rxq->mem.type = page_pool_page_is_pp(virt_to_page(xdp->data)) ?
> > +               MEM_TYPE_PAGE_POOL : MEM_TYPE_PAGE_SHARED;
> > +}
> 
> AI says that it's racy and I think it's onto something.
> See
> https://github.com/kernel-patches/bpf/actions/runs/18224704286/job/51892959919
> and
> https://github.com/kernel-patches/bpf/actions/runs/18224704286/job/51892959937

How do we respond to this in future?
On first one AI missed the fact we run under napi so two cpus won't play
with same rx queue concurrently. The second is valid thing to be fixed.

> 
> click on 'Check review-inline.txt'

  reply	other threads:[~2025-10-07 19:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-03 14:02 [PATCH bpf 0/2] xdp: fix page_pool leaks Maciej Fijalkowski
2025-10-03 14:02 ` [PATCH bpf 1/2] xdp: update xdp_rxq_info's mem type in XDP generic hook Maciej Fijalkowski
2025-10-03 17:29   ` Alexei Starovoitov
2025-10-07 19:39     ` Maciej Fijalkowski [this message]
2025-10-07 20:18       ` Alexei Starovoitov
2025-10-03 14:02 ` [PATCH bpf 2/2] veth: update mem type in xdp_buff Maciej Fijalkowski
2025-10-03 23:10   ` Jakub Kicinski
2025-10-07 14:59     ` Maciej Fijalkowski
2025-10-08  1:11       ` Jakub Kicinski
2025-10-08  9:22         ` Maciej Fijalkowski
2025-10-08 10:37           ` Maciej Fijalkowski
2025-10-13 23:24             ` Jakub Kicinski
2025-10-14 16:53               ` Toke Høiland-Jørgensen

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=aOVsbMAkfc5gv0vO@boxer \
    --to=maciej.fijalkowski@intel.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=hawk@kernel.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=ilias.apalodimas@linaro.org \
    --cc=lorenzo@kernel.org \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=stfomichev@gmail.com \
    --cc=syzbot+ff145014d6b0ce64a173@syzkaller.appspotmail.com \
    --cc=tavip@google.com \
    --cc=toke@redhat.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;
as well as URLs for NNTP newsgroup(s).