Re: [PATCH net 3/6] net: enetc: fix use-after-free in mailbox cleanup on interrupt race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Reordering teardown leaves hardware interrupts enabled after unregistering the OS handler, causing pending MSI-X interrupts that crash the kernel on re-initialization.
- [High] DMA memory Use-After-Free in mailbox cleanup because buffers are freed before clearing hardware receive registers.
- [High] Initialization order bug in `enetc_msg_psi_init()` leading to kernel panic on pending interrupts.
--

commit 7775e7cd27db0383dcec0aee22ae84d6c2378ee2
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: fix use-after-free in mailbox cleanup on interrupt race
    
This commit fixes a use-after-free regression in the PF mailbox teardown path
by reordering the shutdown sequence to prevent work requeuing.

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> index 40d22ebe9224f..fe9680a97b0e7 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> @@ -151,14 +151,14 @@ void enetc_msg_psi_free(struct enetc_pf *pf)
>  	struct enetc_si *si = pf->si;
>  	int i;
>  
> -	cancel_work_sync(&pf->msg_task);
> +	/* de-register message passing interrupt handler */
> +	free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
>  
> -	/* disable MR interrupts */
> -	enetc_msg_disable_mr_int(&si->hw);
> +	cancel_work_sync(&pf->msg_task);

Could moving free_irq() before enetc_msg_disable_mr_int() leave hardware
interrupts enabled after the OS handler is unregistered?

Because VFs are still active at this stage, a VF can send a message.
cancel_work_sync() waits for the worker, which processes existing messages
but then unconditionally re-arms the MR interrupt before exiting.

If a VF sends a message now, the hardware asserts the interrupt. Since
the OS handler is removed and the vector is masked, it is recorded as pending
in the MSI-X Pending Bit Array. enetc_msg_disable_mr_int() is called later
but does not clear this array.

Upon the next SR-IOV enablement, request_irq() unmasks the vector, and
the pending interrupt fires immediately, leading to a crash.

This regression appears to be mitigated later in the patch series by
commit 20bd14a48fb81 ("net: enetc: avoid VF->PF mailbox timeout during
SR-IOV teardown") and commit fa9d7e3799c54 ("net: enetc: fix initialization
order to prevent use of uninitialized resources").

>  	for (i = 0; i < pf->num_vfs; i++)
>  		enetc_msg_free_mbx(si, i);

This isn't a regression introduced by this patch, but does the current
implementation of enetc_msg_free_mbx() have a use-after-free risk for the
mailbox memory?

It calls dma_free_coherent() to free the mailbox memory before clearing the
hardware DMA address registers:

enetc_msg_free_mbx() {
	...
	dma_free_coherent(&si->pdev->dev, msg->size, msg->vaddr, msg->dma);
	memset(msg, 0, sizeof(*msg));

	enetc_wr(hw, ENETC_PSIVMSGRCVAR0(idx), 0);
	enetc_wr(hw, ENETC_PSIVMSGRCVAR1(idx), 0);
}

Since VFs are still active during SR-IOV teardown, a VF could send a message
during this window. This would cause the PF hardware to perform a DMA write
using the uncleared receive address into the just-freed memory.

This regression appears to be mitigated later in the patch series by
commit 20bd14a48fb81 ("net: enetc: avoid VF->PF mailbox timeout during
SR-IOV teardown"), which disables SR-IOV before mailbox cleanup.

> -	/* de-register message passing interrupt handler */
> -	free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
> +	/* disable MR interrupts */
> +	enetc_msg_disable_mr_int(&si->hw);
>  }

Additionally, while not introduced by this patch, is there an initialization
order regression in enetc_msg_psi_init()?

If request_irq() is called to register enetc_msg_psi_msix() before INIT_WORK()
and enetc_msg_alloc_mbx() are executed, an immediate interrupt could cause the
handler to call schedule_work() on an uninitialized work structure or lead
to a NULL pointer dereference.

This regression appears to be fixed later in the patch series by
commit fa9d7e3799c54 ("net: enetc: fix initialization order to prevent use
of uninitialized resources").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513103021.2190593-1-wei.fang@nxp.com?part=3