netdev - [PATCH] drivers: net: mvpp2: attempt to refill rx before allocating skb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250514173417.276731-1-mmakassikis@freebox.fr>
Date: Wed, 14 May 2025 19:34:17 +0200
From: Marios Makassikis <mmakassikis@...ebox.fr>
To: netdev@...r.kernel.org
Cc: marcin.s.wojtas@...il.com,
	linux@...linux.org.uk,
	andrew+netdev@...n.ch,
	davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	linux-kernel@...r.kernel.org,
	Marios Makassikis <mmakassikis@...ebox.fr>
Subject: [PATCH] drivers: net: mvpp2: attempt to refill rx before allocating skb

on mvpp2_rx_refill() failure, the freshly allocated skb is freed,
the rx error counter is incremented and the descriptor currently
being processed is rearmed through mvpp2_bm_pool_put().

the logic is that the system is low on memory so it's not possible
to allocate both a rx descriptor and an skb, so we might as well
drop the skb and return the descriptor to the rx pool to avoid
draining it (and preventing any future packet reception).

the skb freeing is unfortunate, as build_skb() takes ownership
of the 'data' buffer:
 - build_skb() calls  __finalize_skb_around() which sets skb->head
 and skb->data to point to 'data'
 - dev_free_skb_any() may call skb_free_frag() on skb->head

thus, the final mvpp2_bm_pool_put() rearms a descriptor that was
just freed.

call mvpp2_rx_refill() first, so there's no skb to free.

incidentally, doing rx refill prior to skb allocation is what is
done in marvell's mvneta driver for armada 370 (mvneta_rx_hwbm() in
mvneta.c)

Signed-off-by: Marios Makassikis <mmakassikis@...ebox.fr>
Fixes: d6526926de739 ("net: mvpp2: fix memory leak in mvpp2_rx")
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 416a926a8281..e13055ec4483 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4003,6 +4003,12 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 			metasize = xdp.data - xdp.data_meta;
 		}
 
+		err = mvpp2_rx_refill(port, bm_pool, pp, pool);
+		if (err) {
+			netdev_err(port->dev, "failed to refill BM pools\n");
+			goto err_drop_frame;
+		}
+
 		if (frag_size)
 			skb = build_skb(data, frag_size);
 		else
@@ -4021,13 +4027,6 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
 					 skb_hwtstamps(skb));
 		}
 
-		err = mvpp2_rx_refill(port, bm_pool, pp, pool);
-		if (err) {
-			netdev_err(port->dev, "failed to refill BM pools\n");
-			dev_kfree_skb_any(skb);
-			goto err_drop_frame;
-		}
-
 		if (pp)
 			skb_mark_for_recycle(skb);
 		else
-- 
2.49.0