Re: [PATCH v2] scsi: lpfc: drain all CQ work before releasing queue dependencies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] scsi: lpfc: UAF on cq->pring in lpfc_sli4_queue_unset() before
  CQ work drain
- [High] scsi: lpfc: incomplete IRQ and CQ work drain in
  lpfc_sli4_queue_destroy()
--

commit a82bf087615a1eaf204b24247167186d3c756921
Author: Hongyan Xu <getshell@seu.edu.cn>

scsi: lpfc: drain all CQ work before releasing queue dependencies

This commit disables delayed CQ work items and flushes the HBA workqueue
before freeing queues in lpfc_sli4_queue_destroy(), and moves interrupt
teardown ahead of queue destruction in lpfc_sli4_prep_dev_for_reset().

Link: https://lore.kernel.org/all/20260928134154.1595-1-getshell@seu.edu.cn/

> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index 6df5f4e7c5d65..0724dd8dcc02f 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -10808,6 +10808,37 @@ lpfc_sli4_release_queues(struct lpfc_queue ***qs, int max)
>  	*qs = NULL;
>  }
>  
> +static void lpfc_sli4_disable_cq_work(struct lpfc_queue *cq)
> +{
> +	if (!cq)
> +		return;
> +
> +	disable_delayed_work_sync(&cq->sched_irqwork);
> +	disable_delayed_work_sync(&cq->sched_spwork);
> +}
> +
> +static void lpfc_sli4_drain_cq_work(struct lpfc_hba *phba)
> +{
> +	struct lpfc_sli4_hba *sli4 = &phba->sli4_hba;
> +	unsigned int i;
> +
> +	if (sli4->hdwq)
> +		for (i = 0; i < phba->cfg_hdw_queue; i++)
> +			lpfc_sli4_disable_cq_work(sli4->hdwq[i].io_cq);
> +
> +	if (sli4->nvmet_cqset)
> +		for (i = 0; i < phba->cfg_nvmet_mrq; i++)
> +			lpfc_sli4_disable_cq_work(sli4->nvmet_cqset[i]);
> +
> +	lpfc_sli4_disable_cq_work(sli4->els_cq);
> +	lpfc_sli4_disable_cq_work(sli4->nvmels_cq);
> +	lpfc_sli4_disable_cq_work(sli4->mbx_cq);
> +
> +	/* Direct CQ work cannot rearm the disabled delayed work. */
> +	if (phba->wq)
> +		flush_workqueue(phba->wq);
> +}

[Severity: High]
Can a threaded IRQ handler still access or queue CQ work after
lpfc_sli4_drain_cq_work() returns?

For MSI-X, lpfc_sli4_hba_intr_handler_th() runs in a dedicated IRQ kthread
outside phba->wq, calls __lpfc_sli4_hba_process_cq() directly for fast-path
CQs, and queues cq->spwork onto phba->wq for slow-path CQs.  Callers such as
lpfc_sli4_hba_setup() (on failure after lpfc_sli4_arm_cqeq_intr()) and
lpfc_sli_brdrestart_s4() invoke lpfc_sli4_queue_destroy() without disabling or
synchronizing interrupts first, and lpfc_sli4_disable_cq_work() does not call
disable_work_sync() on cq->irqwork or cq->spwork:

CPU0                                  CPU1 (irq kthread)
lpfc_sli4_hba_setup()
  lpfc_sli4_arm_cqeq_intr(phba)
  ...
  lpfc_sli4_queue_destroy(phba)
    lpfc_sli4_drain_cq_work(phba)
      flush_workqueue(phba->wq)
                                      lpfc_sli4_hba_intr_handler_th()
                                        lpfc_sli4_process_eq()
                                          __lpfc_sli4_hba_process_cq(cq)
    lpfc_sli4_release_hdwq(phba)
      lpfc_sli4_queue_free(cq)
        kfree(cq)
                                            ...  <-- use-after-free

Synchronizing active interrupt handlers before draining and calling
disable_work_sync() on cq->irqwork and cq->spwork in
lpfc_sli4_disable_cq_work() would prevent this race.

>  
>  static inline void
>  lpfc_sli4_release_hdwq(struct lpfc_hba *phba)
>  {
> @@ -10868,6 +10899,7 @@ lpfc_sli4_queue_destroy(struct lpfc_hba *phba)
>  	spin_unlock_irq(&phba->hbalock);
>  
>  	lpfc_sli4_cleanup_poll_list(phba);
> +	lpfc_sli4_drain_cq_work(phba);
>  
>  	/* Release HBA eqs */
>  	if (phba->sli4_hba.hdwq)

[Severity: High]
Does draining CQ work only inside lpfc_sli4_queue_destroy() leave a
use-after-free window on cq->pring during lpfc_sli4_queue_unset()?

In lpfc_sli4_hba_unset(), lpfc_sli_brdrestart_s4(), and the
lpfc_sli4_hba_setup() error path, lpfc_sli4_queue_unset() runs before
lpfc_sli4_queue_destroy() while delayed CQ timers (cq->sched_irqwork and
cq->sched_spwork) can still be pending.  Inside lpfc_sli4_queue_unset(),
lpfc_wq_destroy() frees wq->pring, which cq->pring aliases:

drivers/scsi/lpfc/lpfc_sli.c:lpfc_wq_destroy() {
    ...
	list_del_init(&wq->list);
	kfree(wq->pring);
	wq->pring = NULL;
    ...
}

CPU0                                  CPU1 (kworker)
lpfc_sli4_hba_unset()
  lpfc_sli4_queue_unset(phba)
    lpfc_wq_destroy(phba, wq)
      kfree(wq->pring)
                                      lpfc_sli4_dly_hba_process_cq()
                                        ...
                                          lpfc_sli4_fp_handle_fcp_wcqe()
                                            pring = cq->pring;
                                            spin_lock_irqsave(&pring->ring_lock)
                                              <-- use-after-free
  lpfc_sli4_queue_destroy(phba)
    lpfc_sli4_drain_cq_work(phba)

Draining CQ work before lpfc_sli4_queue_unset() tears down WQ state and rings
would close this window.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179474327480.103840.7747335244166055751@seu.edu.cn?part=1