[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aOVsbMAkfc5gv0vO@boxer>
Date: Tue, 7 Oct 2025 21:39:24 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
CC: bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, "Daniel
Borkmann" <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>, Toke
Høiland-Jørgensen <toke@...hat.com>, Lorenzo Bianconi
<lorenzo@...nel.org>, Network Development <netdev@...r.kernel.org>,
"Karlsson, Magnus" <magnus.karlsson@...el.com>, Andrii Nakryiko
<andrii@...nel.org>, Stanislav Fomichev <stfomichev@...il.com>, "Alexander
Lobakin" <aleksander.lobakin@...el.com>,
<syzbot+ff145014d6b0ce64a173@...kaller.appspotmail.com>, Ihor Solodrai
<ihor.solodrai@...ux.dev>, Octavian Purdila <tavip@...gle.com>
Subject: Re: [PATCH bpf 1/2] xdp: update xdp_rxq_info's mem type in XDP
generic hook
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@...el.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@...kaller.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@...ux.dev>
> > Co-developed-by: Octavian Purdila <tavip@...gle.com>
> > Signed-off-by: Octavian Purdila <tavip@...gle.com> # whole analysis, testing, initiating a fix
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.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'
Powered by blists - more mailing lists