Re: [PATCH v6 06/56] scsi: qla2xxx: Remove redundant VPD flash read in sysfs read path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unprotected flash hardware register access in qla2x00_sysfs_read_vpd().
--

--- Patch [6]: [PATCH v6 06/56] scsi: qla2xxx: Remove redundant VPD flash read in sysfs read path ---
commit e9338dabc7b53d75f2d4ddaf069664fa8b72258d
Author: Manish Rangankar <mrangankar@marvell.com>
Subject: scsi: qla2xxx: Remove redundant VPD flash read in sysfs read path

This patch removes a redundant and unprotected call to read_optrom() in 
qla2x00_sysfs_read_vpd(). By dropping the second call, it prevents exposing 
flash accesses to concurrent optrom operations while optrom_mutex is not held.

> diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
> index 6a05ce195aa05..800751ab562a1 100644
> --- a/drivers/scsi/qla2xxx/qla_attr.c
> +++ b/drivers/scsi/qla2xxx/qla_attr.c
> @@ -580,7 +580,6 @@ qla2x00_sysfs_read_vpd(struct file *filp, struct kobject *kobj,

[Severity: High]
This is a pre-existing issue, but does calling qla28xx_get_aux_images() earlier
in qla2x00_sysfs_read_vpd() without holding optrom_mutex risk corrupting the
flash hardware state?

While this patch successfully addresses the unprotected read_optrom() call below,
qla28xx_get_aux_images() is still called before the lock is acquired:

qla2x00_sysfs_read_vpd() {
    ...
    if (IS_QLA28XX(ha)) {
        qla28xx_get_aux_images(vha, &active_regions);
        ...
    }

    mutex_lock(&ha->optrom_mutex);
    ...
}

Because qla28xx_get_aux_images() eventually performs raw MMIO accesses to the
flash hardware registers, could concurrent sysfs reads to the vpd attribute
interleave these writes and reads (e.g., to reg->flash_addr and reg->flash_data)
and corrupt the ISP flash hardware state machine?

>  	ha->isp_ops->read_optrom(vha, ha->vpd, faddr, ha->vpd_size);
>  	mutex_unlock(&ha->optrom_mutex);
>  
> -	ha->isp_ops->read_optrom(vha, ha->vpd, faddr, ha->vpd_size);
>  skip:
>  	return memory_read_from_buffer(buf, count, &off, ha->vpd, ha->vpd_size);
>  }

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