bpf_xdp_shrink_data() frees a released frag via __xdp_return() using
xdp->rxq->mem.type, but that type is wrong for skb-backed XDP: the skb is
cow'd into page_pool memory while the rxq still says MEM_TYPE_PAGE_SHARED,
so the page_pool page is freed with page_frag_free() and we hit
"Bad page state ... page_pool leak".

Both generic XDP and veth are affected. A non-linear skb is cow'd into
page_pool memory (skb_cow_data_for_xdp() -> skb_pp_cow_data() for generic
XDP, veth_convert_skb_to_xdp_buff() for veth), so its frags become
page_pool pages while the rxq keeps MEM_TYPE_PAGE_SHARED.

We can't just fix rxq->mem.type in place:
- generic XDP: xdp->rxq is dev->_rx[queue].xdp_rxq (see
  bpf_prog_run_generic_xdp()), a shared rxq that other CPUs may access in
  parallel, so we must not write to it.
- veth: rq->xdp_rxq.mem is shared per-queue state that veth resets on XDP
  teardown, and with GRO that reset runs without stopping in-flight NAPI,
  so a type stashed there can be clobbered under a packet still in flight.

Adding a check in __xdp_return() or bpf_xdp_shrink_data() itself is not an
option either: without recording it somewhere, both can only guess the
frag's memory type, which quickly gets confusing.

So record it in the xdp_buff. Add a XDP_FLAGS_FRAGS_PAGE_POOL flag; the two
skb-cow sites set it, and bpf_xdp_shrink_data() frees the frag to the
page_pool when it is set, otherwise it keeps falling back to
xdp->rxq->mem.type unchanged. No other path changes behaviour.

Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic 
mode")
Fixes: 0ebab78cbcbf ("net: veth: add page_pool for page recycling")
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=237bbeed8dfe0699b7f5
Signed-off-by: Jiayuan Chen <[email protected]>
---
 drivers/net/veth.c |  5 +++++
 include/net/xdp.h  | 14 ++++++++++++++
 net/core/dev.c     |  5 +++++
 net/core/filter.c  |  7 +++++++
 4 files changed, 31 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 6ed3ee81153f..0afa0661ada1 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -775,6 +775,11 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
        if (skb_shinfo(skb)->nr_frags) {
                skb_shinfo(skb)->xdp_frags_size = skb->data_len;
                xdp_buff_set_frags_flag(xdp);
+               /* A nonlinear skb was cow'd into rq->page_pool above, so the
+                * frags must be freed to that pool, not via the rxq's
+                * MEM_TYPE_PAGE_SHARED.
+                */
+               xdp_buff_set_frag_pp(xdp);
        } else {
                xdp_buff_clear_frags_flag(xdp);
        }
diff --git a/include/net/xdp.h b/include/net/xdp.h
index aa742f413c35..b389dc527adc 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -81,6 +81,10 @@ enum xdp_buff_flags {
         * XDP program is not attached.
         */
        XDP_FLAGS_FRAGS_UNREADABLE      = BIT(2),
+       /* frags are page_pool memory even though rxq->mem.type is not: a
+        * skb-backed XDP buff (generic XDP, veth) is cow'd into a page_pool.
+        */
+       XDP_FLAGS_FRAGS_PAGE_POOL       = BIT(3),
 };
 
 struct xdp_buff {
@@ -131,6 +135,16 @@ static __always_inline void 
xdp_buff_set_frag_unreadable(struct xdp_buff *xdp)
        xdp->flags |= XDP_FLAGS_FRAGS_UNREADABLE;
 }
 
+static __always_inline void xdp_buff_set_frag_pp(struct xdp_buff *xdp)
+{
+       xdp->flags |= XDP_FLAGS_FRAGS_PAGE_POOL;
+}
+
+static __always_inline bool xdp_buff_is_frag_pp(const struct xdp_buff *xdp)
+{
+       return !!(xdp->flags & XDP_FLAGS_FRAGS_PAGE_POOL);
+}
+
 static __always_inline u32 xdp_buff_get_skb_flags(const struct xdp_buff *xdp)
 {
        return xdp->flags;
diff --git a/net/core/dev.c b/net/core/dev.c
index 38336858c168..be36020484b6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5532,6 +5532,11 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct 
xdp_buff *xdp,
        if (skb_is_nonlinear(skb)) {
                skb_shinfo(skb)->xdp_frags_size = skb->data_len;
                xdp_buff_set_frags_flag(xdp);
+               /* A nonlinear skb was cow'd into page_pool memory by
+                * skb_cow_data_for_xdp() before we got here, so the frags must
+                * be freed to that pool, not via the rxq's 
MEM_TYPE_PAGE_SHARED.
+                */
+               xdp_buff_set_frag_pp(xdp);
        } else {
                xdp_buff_clear_frags_flag(xdp);
        }
diff --git a/net/core/filter.c b/net/core/filter.c
index 61940e753552..d34ba56d79d8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4378,6 +4378,13 @@ static bool bpf_xdp_shrink_data(struct xdp_buff *xdp, 
skb_frag_t *frag,
        if (mem_type == MEM_TYPE_XSK_BUFF_POOL) {
                netmem = 0;
                zc_frag = bpf_xdp_shrink_data_zc(xdp, shrink, tail, release);
+       } else if (xdp_buff_is_frag_pp(xdp)) {
+               /*
+                * Skb-backed XDP (generic XDP, veth) cow's the frags into a
+                * page_pool while the rxq stays MEM_TYPE_PAGE_SHARED, so free
+                * the frag to the pool, not via page_frag_free().
+                */
+               mem_type = MEM_TYPE_PAGE_POOL;
        }
 
        if (release) {
-- 
2.43.0


Reply via email to