Skip to content

Commit 54362b0

Browse files
WeiFang-NXPkuba-moo
authored andcommitted
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: beb74ac ("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>
1 parent f8ae63d commit 54362b0

1 file changed

Lines changed: 18 additions & 17 deletions

File tree

drivers/net/ethernet/freescale/enetc/enetc_msg.c

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,15 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
118118
struct enetc_si *si = pf->si;
119119
int vector, i, err;
120120

121+
for (i = 0; i < pf->num_vfs; i++) {
122+
err = enetc_msg_alloc_mbx(si, i);
123+
if (err)
124+
goto free_mbx;
125+
}
126+
127+
/* initialize PSI mailbox */
128+
INIT_WORK(&pf->msg_task, enetc_msg_task);
129+
121130
/* register message passing interrupt handler */
122131
snprintf(pf->msg_int_name, sizeof(pf->msg_int_name), "%s-vfmsg",
123132
si->ndev->name);
@@ -126,32 +135,21 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
126135
if (err) {
127136
dev_err(&si->pdev->dev,
128137
"PSI messaging: request_irq() failed!\n");
129-
return err;
138+
goto free_mbx;
130139
}
131140

132141
/* set one IRQ entry for PSI message receive notification (SI int) */
133142
enetc_wr(&si->hw, ENETC_SIMSIVR, ENETC_SI_INT_IDX);
134143

135-
/* initialize PSI mailbox */
136-
INIT_WORK(&pf->msg_task, enetc_msg_task);
137-
138-
for (i = 0; i < pf->num_vfs; i++) {
139-
err = enetc_msg_alloc_mbx(si, i);
140-
if (err)
141-
goto err_init_mbx;
142-
}
143-
144144
/* enable MR interrupts */
145145
enetc_msg_enable_mr_int(pf);
146146

147147
return 0;
148148

149-
err_init_mbx:
149+
free_mbx:
150150
for (i--; i >= 0; i--)
151151
enetc_msg_free_mbx(si, i);
152152

153-
free_irq(vector, si);
154-
155153
return err;
156154
}
157155

@@ -160,14 +158,17 @@ void enetc_msg_psi_free(struct enetc_pf *pf)
160158
struct enetc_si *si = pf->si;
161159
int i;
162160

161+
/* disable MR interrupts */
162+
enetc_msg_disable_mr_int(pf);
163+
164+
/* de-register message passing interrupt handler */
165+
free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
166+
163167
cancel_work_sync(&pf->msg_task);
164168

165-
/* disable MR interrupts */
169+
/* MR interrupts may be re-enabled by workqueue */
166170
enetc_msg_disable_mr_int(pf);
167171

168172
for (i = 0; i < pf->num_vfs; i++)
169173
enetc_msg_free_mbx(si, i);
170-
171-
/* de-register message passing interrupt handler */
172-
free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
173174
}

0 commit comments

Comments
 (0)