Re: [PATCH v3 63/88] scsi: qla2xxx: Zero mailbox struct in qla2x00_get_firmware_state()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized kernel stack memory disclosure vulnerability remains unfixed in the sysfs handler `qla2x00_fw_state_show()` on the success path.
--

--- Patch [63]: [PATCH v3 63/88] scsi: qla2xxx: Zero mailbox struct in qla2x00_get_firmware_state() ---
commit 9d1cc31f70b4b1282c00f58b1e61ed39b581f84d
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Zero mailbox struct in qla2x00_get_firmware_state()

This commit zero-initializes the mailbox command structure in
qla2x00_get_firmware_state() to prevent leaking uninitialized kernel stack
memory on failure paths. It ensures that when early returns occur, userspace
receives deterministic zeroed state rather than stack contents.

> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index ba4a4764de1fb..ab5648eb5f206 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -2276,6 +2276,8 @@ qla2x00_get_firmware_state(scsi_qla_host_t *vha, uint16_t *states)
>  	if (!ha->flags.fw_started)
>  		return QLA_FUNCTION_FAILED;
>  
> +	memset(&mc, 0, sizeof(mc));
> +
>  	mcp->mb[0] = MBC_GET_FIRMWARE_STATE;
>  	mcp->out_mb = MBX_0;
>  	if (IS_FWI2_CAPABLE(vha->hw))

[Severity: High]
This is a pre-existing issue, but does this still leak uninitialized stack
memory on the success path for adapters that are not FWI2 capable?

Looking at the caller qla2x00_fw_state_show() in
drivers/scsi/qla2xxx/qla_attr.c, an uninitialized array is declared on the
stack and passed to qla2x00_get_firmware_state():

drivers/scsi/qla2xxx/qla_attr.c:qla2x00_fw_state_show() {
	uint16_t state[16];
        ...
	rval = qla2x00_get_firmware_state(vha, state);
        ...
}

When qla2x00_get_firmware_state() completes successfully, it only populates
states[1] through states[5] if the adapter is FWI2 capable:

drivers/scsi/qla2xxx/qla_mbx.c:qla2x00_get_firmware_state() {
	states[0] = mcp->mb[1];
	if (IS_FWI2_CAPABLE(vha->hw)) {
		states[1] = mcp->mb[2];
		states[2] = mcp->mb[3];  /* SFP info */
		states[3] = mcp->mb[4];
		states[4] = mcp->mb[5];
		states[5] = mcp->mb[6];  /* DPORT status */
        ...
}

However, qla2x00_fw_state_show() unconditionally formats all six elements into
the sysfs output buffer, which could expose uninitialized stack memory to
userspace for non-FWI2 adapters:

drivers/scsi/qla2xxx/qla_attr.c:qla2x00_fw_state_show() {
        ...
	return scnprintf(buf, PAGE_SIZE, "0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n",
	    state[0], state[1], state[2], state[3], state[4], state[5]);
}

Should the state array be zero-initialized in qla2x00_fw_state_show(), or
should qla2x00_get_firmware_state() guarantee all returned values are
initialized regardless of the adapter capability?

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