summaryrefslogtreecommitdiff
path: root/drivers/net/ethernet
diff options
context:
space:
mode:
authorWei Fang <wei.fang@nxp.com>2026-05-20 14:44:20 +0800
committerJakub Kicinski <kuba@kernel.org>2026-05-21 08:48:59 -0700
commit54362b0176080b905dbd0651ee3dbb295da41541 (patch)
treec289870991105875375e53fccc4bc2b294cbd8b8 /drivers/net/ethernet
parentf8ae63de2a872fa3b68c287c35379f6d73d38a5d (diff)
downloadlwn-54362b0176080b905dbd0651ee3dbb295da41541.tar.gz
lwn-54362b0176080b905dbd0651ee3dbb295da41541.zip
net: enetc: fix init and teardown order to prevent use of unsafe resources
Sashiko reported a potential issue in enetc_msg_psi_init() where the IRQ handler is registered before DMA resources are fully initialized [1]. The current initialization sequence is: 1. request_irq(enetc_msg_psi_msix) <- IRQ handler registered 2. INIT_WORK(&pf->msg_task, ...) <- work_struct initialized 3. enetc_msg_alloc_mbx() <- mailbox DMA allocated This ordering is unsafe because if a spurious interrupt or pending interrupt from a previous device state fires immediately after request_irq() returns, the registered ISR enetc_msg_psi_msix() will execute and unconditionally call: schedule_work(&pf->msg_task) At this point, pf->msg_task has not been initialized by INIT_WORK(), so the work_struct contains garbage values in its internal linked list pointers (work_struct->entry). Passing an uninitialized work_struct to schedule_work() could corrupt the kernel's workqueue linked lists, potentially leading to: - Kernel panic in __queue_work() - Memory corruption in workqueue data structures - System deadlock or undefined behavior Additionally, even if the work_struct was initialized, the mailbox DMA buffers (pf->rxmsg[]) may not yet be allocated when the work handler enetc_msg_task() runs, resulting in NULL pointer dereference. Fix by reordering the initialization sequence to ensure all resources are properly initialized before the interrupt handler can execute: 1. enetc_msg_alloc_mbx() <- Allocate all mailboxes 2. INIT_WORK(&pf->msg_task, ...) <- Initialize work first 3. request_irq(enetc_msg_psi_msix) <- Register IRQ last 4. Configure hardware & enable MR interrupts This guarantees that when enetc_msg_psi_msix() runs: - pf->msg_task is properly initialized (safe for schedule_work) - pf->rxmsg[] buffers are allocated (safe for work handler access) - Hardware is configured appropriately As the inverse of enetc_msg_psi_init(), enetc_msg_psi_free() also has similar problems. For example, if a pending interrupt fires between enetc_msg_free_mbx() and free_irq(), the ISR enetc_msg_psi_msix() may schedule the work handler again via schedule_work(), which could then access already-freed DMA buffers (pf->rxmsg[]), leading to use-after-free and potential memory corruption. Therefore, the order of enetc_msg_psi_free() is adjusted: 1. enetc_msg_disable_mr_int() <- Stop new interrupts first 2. free_irq() <- Ensure no IRQ handler can run 3. cancel_work_sync() <- Wait for any pending work 4. enetc_msg_disable_mr_int() <- Re-disable in case work re-enabled it 5. enetc_msg_free_mbx() <- Safe to free DMA buffers now Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1 Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support") Signed-off-by: Wei Fang <wei.fang@nxp.com> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com> Link: https://patch.msgid.link/20260520064421.91569-9-wei.fang@nxp.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Diffstat (limited to 'drivers/net/ethernet')
-rw-r--r--drivers/net/ethernet/freescale/enetc/enetc_msg.c35
1 files changed, 18 insertions, 17 deletions
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
index 3136e8321e4d..c09635e7eb3d 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
@@ -118,6 +118,15 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
struct enetc_si *si = pf->si;
int vector, i, err;
+ for (i = 0; i < pf->num_vfs; i++) {
+ err = enetc_msg_alloc_mbx(si, i);
+ if (err)
+ goto free_mbx;
+ }
+
+ /* initialize PSI mailbox */
+ INIT_WORK(&pf->msg_task, enetc_msg_task);
+
/* register message passing interrupt handler */
snprintf(pf->msg_int_name, sizeof(pf->msg_int_name), "%s-vfmsg",
si->ndev->name);
@@ -126,32 +135,21 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
if (err) {
dev_err(&si->pdev->dev,
"PSI messaging: request_irq() failed!\n");
- return err;
+ goto free_mbx;
}
/* set one IRQ entry for PSI message receive notification (SI int) */
enetc_wr(&si->hw, ENETC_SIMSIVR, ENETC_SI_INT_IDX);
- /* initialize PSI mailbox */
- INIT_WORK(&pf->msg_task, enetc_msg_task);
-
- for (i = 0; i < pf->num_vfs; i++) {
- err = enetc_msg_alloc_mbx(si, i);
- if (err)
- goto err_init_mbx;
- }
-
/* enable MR interrupts */
enetc_msg_enable_mr_int(pf);
return 0;
-err_init_mbx:
+free_mbx:
for (i--; i >= 0; i--)
enetc_msg_free_mbx(si, i);
- free_irq(vector, si);
-
return err;
}
@@ -160,14 +158,17 @@ void enetc_msg_psi_free(struct enetc_pf *pf)
struct enetc_si *si = pf->si;
int i;
+ /* disable MR interrupts */
+ enetc_msg_disable_mr_int(pf);
+
+ /* de-register message passing interrupt handler */
+ free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
+
cancel_work_sync(&pf->msg_task);
- /* disable MR interrupts */
+ /* MR interrupts may be re-enabled by workqueue */
enetc_msg_disable_mr_int(pf);
for (i = 0; i < pf->num_vfs; i++)
enetc_msg_free_mbx(si, i);
-
- /* de-register message passing interrupt handler */
- free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
}