Skip to contentSkip to conversation

net: lan966x: fix page_pool error handling and error paths

PATCH Applied David Carlier 2 participants 5 messages Apr 5 – Apr 9

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v3 cover net: lan966x: fix page_pool error handling and error paths

This series fixes error handling around the lan966x page pool:

1/3 adds the missing IS_ERR check after page_pool_create(), preventing
a kernel oops when the error pointer flows into xdp_rxq_info_reg_mem_model().

2/3 plugs page pool leaks in the lan966x_fdma_rx_alloc() and
lan966x_fdma_init() error paths, now reachable after 1/3.

3/3 fixes a use-after-free and page pool leak in the
lan966x_fdma_reload() restore path, where the hardware could resume DMA into pages already returned to the page pool.

David Carlier (3):
net: lan966x: fix page_pool error handling in

    lan966x_fdma_rx_alloc_page_pool()

net: lan966x: fix page pool leak in error paths
net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()

 .../ethernet/microchip/lan966x/lan966x_fdma.c | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)
Signature · 10 lines
v2 -> v3:                                                                                                                                                             
    - 1/3: remove blank line between page_pool_create() and IS_ERR check (Jakub)
    - 2/3: drop rx->page_pool = NULL for consistency with lan966x_fdma_init()                                                                                           
           cleanup; update commit message accordingly (Jakub)                                                                                                           
    - 3/3: remove blank line between kmemdup() and !old_pages check (Jakub)                                                                                             
  v1 -> v2: address caller error paths raised by Jakub's review; add patches                                                                                            
           2/3 and 3/3                                                                                                                                                  
  v1: https://lore.kernel.org/netdev/[email protected]                                                                           
  v2: https://lore.kernel.org/netdev/[email protected]
2.53.0
#1 lore raw

PATCH v3 1/3 net: lan966x: fix page_pool error handling in lan966x_fdma_rx_alloc_page_pool()

page_pool_create() can return an ERR_PTR on failure. The return value is used unconditionally in the loop that follows, passing the error pointer through xdp_rxq_info_reg_mem_model() into page_pool_use_xdp_mem(), which dereferences it, causing a kernel oops.

Add an IS_ERR check after page_pool_create() to return early on failure.

Fixes 11871aba1974 ("net: lan96x: Use page_pool API")
---
 drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c +2 -0
index 7b6369e43451..74851c63e46a 100644
@@ -91,6 +91,8 @@ static int lan966x_fdma_rx_alloc_page_pool(struct lan966x_rx *rx)
 		pp_params.dma_dir = DMA_BIDIRECTIONAL;
 
 	rx->page_pool = page_pool_create(&pp_params);
+	if (unlikely(IS_ERR(rx->page_pool)))
+		return PTR_ERR(rx->page_pool);
 
 	for (int i = 0; i < lan966x->num_phys_ports; ++i) {
 		struct lan966x_port *port;
Signature · 1 line
2.53.0
#2 lore raw

PATCH v3 2/3 net: lan966x: fix page pool leak in error paths

In reply to David Carlier

lan966x_fdma_rx_alloc() creates a page pool but does not destroy it if the subsequent fdma_alloc_coherent() call fails, leaking the pool.

Similarly, lan966x_fdma_init() frees the coherent DMA memory when lan966x_fdma_tx_alloc() fails but does not destroy the page pool that was successfully created by lan966x_fdma_rx_alloc(), leaking it.

Add the missing page_pool_destroy() calls in both error paths.

Fixes 11871aba1974 ("net: lan96x: Use page_pool API")
---
 drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c +4 -1
index 74851c63e46a..10773fe93d4d 100644
@@ -119,8 +119,10 @@ static int lan966x_fdma_rx_alloc(struct lan966x_rx *rx)
 		return PTR_ERR(rx->page_pool);
 
 	err = fdma_alloc_coherent(lan966x->dev, fdma);
-	if (err)
+	if (err) {
+		page_pool_destroy(rx->page_pool);
 		return err;
+	}
 
 	fdma_dcbs_init(fdma, FDMA_DCB_INFO_DATAL(fdma->db_size),
 		       FDMA_DCB_STATUS_INTR);
@@ -957,6 +959,7 @@ int lan966x_fdma_init(struct lan966x *lan966x)
 	err = lan966x_fdma_tx_alloc(&lan966x->tx);
 	if (err) {
 		fdma_free_coherent(lan966x->dev, &lan966x->rx.fdma);
+		page_pool_destroy(lan966x->rx.page_pool);
 		return err;
 	}
 
Signature · 1 line
2.53.0
#3 lore raw

PATCH v3 3/3 net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()

In reply to David Carlier

When lan966x_fdma_reload() fails to allocate new RX buffers, the restore path restarts DMA using old descriptors whose pages were already freed via lan966x_fdma_rx_free_pages(). Since page_pool_put_full_page() can release pages back to the buddy allocator, the hardware may DMA into memory now owned by other kernel subsystems.

Additionally, on the restore path, the newly created page pool (if allocation partially succeeded) is overwritten without being destroyed, leaking it.

Fix both issues by deferring the release of old pages until after the new allocation succeeds. Save the old page array before the allocation so old pages can be freed on the success path. On the failure path, the old descriptors, pages and page pool are all still valid, making the restore safe. Also ensure the restore path re-enables NAPI and wakes the netdev, matching the success path.

Fixes 89ba464fcf54 ("net: lan966x: refactor buffer reload function")
---
 .../ethernet/microchip/lan966x/lan966x_fdma.c | 21 ++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c +18 -3
index 10773fe93d4d..f8ce735a7fc0 100644
@@ -812,9 +812,15 @@ static int lan966x_qsys_sw_status(struct lan966x *lan966x)
 
 static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
 {
+	struct page *(*old_pages)[FDMA_RX_DCB_MAX_DBS];
 	struct page_pool *page_pool;
 	struct fdma fdma_rx_old;
-	int err;
+	int err, i, j;
+
+	old_pages = kmemdup(lan966x->rx.page, sizeof(lan966x->rx.page),
+			   GFP_KERNEL);
+	if (!old_pages)
+		return -ENOMEM;
 
 	/* Store these for later to free them */
 	memcpy(&fdma_rx_old, &lan966x->rx.fdma, sizeof(struct fdma));
@@ -825,7 +831,6 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
 	lan966x_fdma_stop_netdev(lan966x);
 
 	lan966x_fdma_rx_disable(&lan966x->rx);
-	lan966x_fdma_rx_free_pages(&lan966x->rx);
 	lan966x->rx.page_order = round_up(new_mtu, PAGE_SIZE) / PAGE_SIZE - 1;
 	lan966x->rx.max_mtu = new_mtu;
 	err = lan966x_fdma_rx_alloc(&lan966x->rx);
@@ -833,6 +838,11 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
 		goto restore;
 	lan966x_fdma_rx_start(&lan966x->rx);
 
+	for (i = 0; i < fdma_rx_old.n_dcbs; ++i)
+		for (j = 0; j < fdma_rx_old.n_dbs; ++j)
+			page_pool_put_full_page(page_pool,
+						old_pages[i][j], false);
+
 	fdma_free_coherent(lan966x->dev, &fdma_rx_old);
 
 	page_pool_destroy(page_pool);
@@ -840,12 +850,17 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
 	lan966x_fdma_wakeup_netdev(lan966x);
 	napi_enable(&lan966x->napi);
 
-	return err;
+	kfree(old_pages);
+	return 0;
 restore:
 	lan966x->rx.page_pool = page_pool;
 	memcpy(&lan966x->rx.fdma, &fdma_rx_old, sizeof(struct fdma));
 	lan966x_fdma_rx_start(&lan966x->rx);
 
+	lan966x_fdma_wakeup_netdev(lan966x);
+	napi_enable(&lan966x->napi);
+
+	kfree(old_pages);
 	return err;
 }
 
Signature · 1 line
2.53.0
#4 lore raw
In reply to David Carlier
This series was applied to netdev/net.git (main) by Paolo Abeni <[email protected]>:

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <[email protected]>:

Quoted 10 lines · On Sun, 5 Apr 2026 06:52:38 +0100 you wrote:

This series fixes error handling around the lan966x page pool:

1/3 adds the missing IS_ERR check after page_pool_create(), preventing
a kernel oops when the error pointer flows into xdp_rxq_info_reg_mem_model().

2/3 plugs page pool leaks in the lan966x_fdma_rx_alloc() and
lan966x_fdma_init() error paths, now reachable after 1/3.

[...]

Here is the summary with links:
- [net,v3,v3,1/3] net: lan966x: fix page_pool error handling in lan966x_fdma_rx_alloc_page_pool()

    https://git.kernel.org/netdev/net/c/3fd0da4fd885
  - [net,v3,v3,2/3] net: lan966x: fix page pool leak in error paths
    https://git.kernel.org/netdev/net/c/076344a6ad9d
  - [net,v3,v3,3/3] net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()
    https://git.kernel.org/netdev/net/c/59c3d55a946c

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html