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

[Paolo Abeni <pabeni@redhat.com>]

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.

> @@ -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