Thread (7 messages) 7 messages, 4 authors, 4d ago

Re: [PATCH net v2 2/2] pds_core: fix use-after-free on workqueue during remove

From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-07-02 08:13:10

On 6/29/26 10:03 PM, Nikhil P. Rao wrote:
In pdsc_remove(), the workqueue is destroyed before pdsc_teardown()
is called. This ordering allows two paths to queue work on the
destroyed workqueue:

1. If pdsc_teardown() -> pdsc_devcmd_reset() times out, the error
   path in pdsc_devcmd_locked() queues health_work.

2. A NotifyQ event can trigger the ISR and queue work before free_irq()
   is called in pdsc_teardown().
I think this should be 2 separate patches.
quoted hunk ↗ jump to hunk
@@ -121,10 +122,16 @@ void pdsc_process_adminq(struct pdsc_qcq *qcq)
 	qcq->accum_work += aq_work;
 
 credits:
-	/* Return the interrupt credits, one for each completion */
-	pds_core_intr_credits(&pdsc->intr_ctrl[qcq->intx],
-			      nq_work + aq_work,
-			      PDS_CORE_INTR_CRED_REARM);
+	/* Return the interrupt credits, one for each completion.
+	 * Use READ_ONCE to get a single consistent copy of intx since it can
+	 * be set to PDS_CORE_INTR_INDEX_NOT_ASSIGNED concurrently during
+	 * teardown, and skip the credits if so.
+	 */
+	intx = READ_ONCE(qcq->intx);
+	if (intx != PDS_CORE_INTR_INDEX_NOT_ASSIGNED)
+		pds_core_intr_credits(&pdsc->intr_ctrl[intx],
+				      nq_work + aq_work,
+				      PDS_CORE_INTR_CRED_REARM);
AFAICS this does not look safe.

A concurrent pdsc_qcq_free()/pdsc_qcq_intr_free() may free
`pdsc->intr_ctrl` before setting PDS_CORE_INTR_INDEX_NOT_ASSIGNED.

I think the teardown should:

- disable the IRQ
- cancel the work
- free the structs

in the above sequence.

/P
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help