Re: [PATCH v2 40/60] scsi: qla2xxx: Add size check for extended VP report ID entry

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Dangling pointer `fcport` is accessed after dropping `sess_lock`, and list traversal is performed entirely locklessly for Format 2 reports.
--

commit 4be1e7bdd8412a09686257ce5b4981203d404668
Author: Nilesh Javali <njavali@marvell.com>
scsi: qla2xxx: Add size check for extended VP report ID entry

This patch adds padding to the vp_rpt_id_entry_24xx_ext structure to match
the hardware IOCB stride. It also updates qla24xx_report_id_acquisition()
to extract vp_idx and vp_status from the extended structure on 29xx adapters.

> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -4094,15 +4094,18 @@ qla2x00_set_idma_speed(scsi_qla_host_t *vha, uint16_t loop_id,
>  }
>  
>  void
> -qla24xx_report_id_acquisition(scsi_qla_host_t *vha,
> -	struct vp_rpt_id_entry_24xx *rptid_entry)
> +qla24xx_report_id_acquisition(scsi_qla_host_t *vha, void *pkt)
>  {

[Severity: High]
This is a pre-existing issue, but can this function trigger a use-after-free
or list corruption?

In the format 1 path, the sess_lock is dropped right before modifying the
fcport structure without acquiring a reference:

    spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
    ...
    fcport = qla2x00_find_fcport_by_wwpn(vha, rptid_entry->u.f1.port_name, 1);
    spin_unlock_irqrestore(&vha->hw->tgt.sess_lock, flags);

    if (fcport) {
        fcport->plogi_nack_done_deadline = jiffies + HZ;
        ...

Without a reference count being taken, could a concurrent thread remove
and free this fcport before it is modified here?

Additionally, in the format 2 path, the vha->vp_fcports list is iterated
locklessly, and then fcport is searched and dereferenced locklessly:

    list_for_each_entry(fcport, &vha->vp_fcports, list) {
        fcport->scan_state = QLA_FCPORT_SCAN;
        fcport->n2n_flag = 0;
    }

    fcport = qla2x00_find_fcport_by_wwpn(vha, rptid_entry->u.f2.port_name, 1);

    if (fcport) {
        fcport->login_retry = vha->hw->login_retry_count;
        ...

Could concurrent modifications to this list by the DPC thread or sysfs cause
list corruption panics or a use-after-free here?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612095333.1666592-1-njavali@marvell.com?part=40