net: lan966x: restore XDP RXQ page pool on reload failure

[PATCH] net: lan966x: restore XDP RXQ page pool on reload failure

Posted by Guangshuo Li 3 days, 21 hours ago

lan966x_fdma_rx_alloc_page_pool() registers the newly allocated page pool
with each port's XDP RXQ. During an FDMA reload, this registration is
performed before fdma_alloc_coherent() is called.

If fdma_alloc_coherent() fails, lan966x_fdma_rx_alloc() destroys the new
page pool and returns an error. The reload rollback path restores
rx->page_pool and rx->fdma to their old values, but it does not restore
the per-port XDP RXQ mem model registration. As a result, the XDP RXQ
state can remain associated with the page pool from the failed allocation
attempt while the RX state has been rolled back to the old page pool.

Restore the old page pool registration for each port's XDP RXQ in the
reload rollback path before RX is started again, so the XDP RXQ state
matches the restored RX state.

Fixes: 59c3d55a946c ("net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 .../net/ethernet/microchip/lan966x/lan966x_fdma.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
index f8ce735a7fc0..76654b44baf2 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
@@ -855,6 +855,21 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
 restore:
 	lan966x->rx.page_pool = page_pool;
 	memcpy(&lan966x->rx.fdma, &fdma_rx_old, sizeof(struct fdma));
+	/*
+	 * lan966x_fdma_rx_alloc_page_pool() registered the new pool with
+	 * each port's XDP RXQ before the allocation failed. The new pool is
+	 * destroyed by lan966x_fdma_rx_alloc(), so restore the old pool's
+	 * registration before restarting RX.
+	 */
+	for (int i = 0; i < lan966x->num_phys_ports; i++) {
+		if (!lan966x->ports[i])
+			continue;
+
+		xdp_rxq_info_unreg_mem_model(&lan966x->ports[i]->xdp_rxq);
+		xdp_rxq_info_reg_mem_model(&lan966x->ports[i]->xdp_rxq,
+					   MEM_TYPE_PAGE_POOL, page_pool);
+	}
+
 	lan966x_fdma_rx_start(&lan966x->rx);
 
 	lan966x_fdma_wakeup_netdev(lan966x);
-- 
2.43.0

Re: [PATCH] net: lan966x: restore XDP RXQ page pool on reload failure

Posted by David CARLIER 3 days, 18 hours ago

On Thu, 4 Jun 2026 at 12:31, Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> lan966x_fdma_rx_alloc_page_pool() registers the newly allocated page pool
> with each port's XDP RXQ. During an FDMA reload, this registration is
> performed before fdma_alloc_coherent() is called.
>
> If fdma_alloc_coherent() fails, lan966x_fdma_rx_alloc() destroys the new
> page pool and returns an error. The reload rollback path restores
> rx->page_pool and rx->fdma to their old values, but it does not restore
> the per-port XDP RXQ mem model registration. As a result, the XDP RXQ
> state can remain associated with the page pool from the failed allocation
> attempt while the RX state has been rolled back to the old page pool.
>
> Restore the old page pool registration for each port's XDP RXQ in the
> reload rollback path before RX is started again, so the XDP RXQ state
> matches the restored RX state.
>
> Fixes: 59c3d55a946c ("net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  .../net/ethernet/microchip/lan966x/lan966x_fdma.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> index f8ce735a7fc0..76654b44baf2 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> @@ -855,6 +855,21 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
>  restore:
>         lan966x->rx.page_pool = page_pool;
>         memcpy(&lan966x->rx.fdma, &fdma_rx_old, sizeof(struct fdma));
> +       /*
> +        * lan966x_fdma_rx_alloc_page_pool() registered the new pool with
> +        * each port's XDP RXQ before the allocation failed. The new pool is
> +        * destroyed by lan966x_fdma_rx_alloc(), so restore the old pool's
> +        * registration before restarting RX.
> +        */
> +       for (int i = 0; i < lan966x->num_phys_ports; i++) {
> +               if (!lan966x->ports[i])
> +                       continue;
> +
> +               xdp_rxq_info_unreg_mem_model(&lan966x->ports[i]->xdp_rxq);
> +               xdp_rxq_info_reg_mem_model(&lan966x->ports[i]->xdp_rxq,
> +                                          MEM_TYPE_PAGE_POOL, page_pool);
> +       }
> +
>         lan966x_fdma_rx_start(&lan966x->rx);
>
>         lan966x_fdma_wakeup_netdev(lan966x);
> --
> 2.43.0
>

Hi Guangshuo,

Just one remark. The re-registration fix looks right, but the same
restore path has a related gap.

  Just above the failing alloc we set:

  lan966x->rx.page_order = round_up(new_mtu, PAGE_SIZE) / PAGE_SIZE - 1;
  lan966x->rx.max_mtu = new_mtu;

  These aren't rolled back in restore:, so after a failed reload
page_order/max_mtu are new while pages/fdma/page_pool are old.
lan966x_xdp_run() then calls
  xdp_init_buff() with PAGE_SIZE << page_order, advertising a
too-large frame_sz — for a jumbo MTU that bumps page_order,
bpf_xdp_adjust_tail() could grow past the real
  page.

  Could you save and restore the old page_order/max_mtu too, so the
restored RX state is fully consistent?

Cheers.

Re: [PATCH] net: lan966x: restore XDP RXQ page pool on reload failure

Posted by Guangshuo Li 17 hours ago

Hi David,

Thank you for the review.

On Thu, 4 Jun 2026 at 21:46, David CARLIER <devnexen@gmail.com> wrote:
>
> On Thu, 4 Jun 2026 at 12:31, Guangshuo Li <lgs201920130244@gmail.com> wrote:
> >
> > lan966x_fdma_rx_alloc_page_pool() registers the newly allocated page pool
> > with each port's XDP RXQ. During an FDMA reload, this registration is
> > performed before fdma_alloc_coherent() is called.
> >
> > If fdma_alloc_coherent() fails, lan966x_fdma_rx_alloc() destroys the new
> > page pool and returns an error. The reload rollback path restores
> > rx->page_pool and rx->fdma to their old values, but it does not restore
> > the per-port XDP RXQ mem model registration. As a result, the XDP RXQ
> > state can remain associated with the page pool from the failed allocation
> > attempt while the RX state has been rolled back to the old page pool.
> >
> > Restore the old page pool registration for each port's XDP RXQ in the
> > reload rollback path before RX is started again, so the XDP RXQ state
> > matches the restored RX state.
> >
> > Fixes: 59c3d55a946c ("net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()")
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> >  .../net/ethernet/microchip/lan966x/lan966x_fdma.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> > index f8ce735a7fc0..76654b44baf2 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> > @@ -855,6 +855,21 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
> >  restore:
> >         lan966x->rx.page_pool = page_pool;
> >         memcpy(&lan966x->rx.fdma, &fdma_rx_old, sizeof(struct fdma));
> > +       /*
> > +        * lan966x_fdma_rx_alloc_page_pool() registered the new pool with
> > +        * each port's XDP RXQ before the allocation failed. The new pool is
> > +        * destroyed by lan966x_fdma_rx_alloc(), so restore the old pool's
> > +        * registration before restarting RX.
> > +        */
> > +       for (int i = 0; i < lan966x->num_phys_ports; i++) {
> > +               if (!lan966x->ports[i])
> > +                       continue;
> > +
> > +               xdp_rxq_info_unreg_mem_model(&lan966x->ports[i]->xdp_rxq);
> > +               xdp_rxq_info_reg_mem_model(&lan966x->ports[i]->xdp_rxq,
> > +                                          MEM_TYPE_PAGE_POOL, page_pool);
> > +       }
> > +
> >         lan966x_fdma_rx_start(&lan966x->rx);
> >
> >         lan966x_fdma_wakeup_netdev(lan966x);
> > --
> > 2.43.0
> >
>
> Hi Guangshuo,
>
> Just one remark. The re-registration fix looks right, but the same
> restore path has a related gap.
>
>   Just above the failing alloc we set:
>
>   lan966x->rx.page_order = round_up(new_mtu, PAGE_SIZE) / PAGE_SIZE - 1;
>   lan966x->rx.max_mtu = new_mtu;
>
>   These aren't rolled back in restore:, so after a failed reload
> page_order/max_mtu are new while pages/fdma/page_pool are old.
> lan966x_xdp_run() then calls
>   xdp_init_buff() with PAGE_SIZE << page_order, advertising a
> too-large frame_sz — for a jumbo MTU that bumps page_order,
> bpf_xdp_adjust_tail() could grow past the real
>   page.
>
>   Could you save and restore the old page_order/max_mtu too, so the
> restored RX state is fully consistent?
>
> Cheers.

Yes, that makes sense. I missed that page_order and max_mtu are also
updated before the failing allocation and need to be restored together
with the old RX state.

I will save and restore the old page_order/max_mtu in the reload rollback
path, and send a v2 with that fixed.

Thanks,
Guangshuo