Re: [PATCH v5 16/40] scsi: device_handlers: alua: use 16-bits defined sense codes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized stack memory `sense_hdr` is unconditionally evaluated in `alua_tur()` if `scsi_test_unit_ready()` fails locally.
--

commit 8e77c706432e156d81e3b3bd7cdc9af0c2b4cc23
Author: Damien Le Moal <dlemoal@kernel.org>

scsi: device_handlers: alua: use 16-bits defined sense codes

This commit refactors the SCSI ALUA device handler driver to use the 16-bit
sense_code field of struct scsi_sense_hdr. It replaces hard-coded ASC and
ASCQ values with enum constants from include/scsi/scsi_sense.h to improve
code clarity. No functional change is intended.

> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index d81ec47a71402..87ac0a6d63e54 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c

[ ... ]

> @@ -517,12 +499,12 @@ static int alua_tur(struct scsi_device *sdev)
>  				      ALUA_FAILOVER_RETRIES, &sense_hdr);
>  	if ((sense_hdr.sense_key == NOT_READY ||
>  	     sense_hdr.sense_key == UNIT_ATTENTION) &&
> -	    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a)
> +	    sense_hdr.sense_code ==
> +	    LU_NOT_ACCESSIBLE_ASYMMETRIC_ACCESS_STATE_TRANSITION)

[Severity: High]
This is a pre-existing issue, but does this evaluate uninitialized stack
memory if scsi_test_unit_ready() fails locally?

Looking at alua_tur() in drivers/scsi/device_handler/scsi_dh_alua.c:

static int alua_tur(struct scsi_device *sdev)
{
        struct scsi_sense_hdr sense_hdr;
        int retval;

        retval = scsi_test_unit_ready(sdev, ALUA_FAILOVER_TIMEOUT * HZ,
                                      ALUA_FAILOVER_RETRIES, &sense_hdr);

The sense_hdr variable is declared on the stack without initialization. If the
underlying request allocation fails or the queue is dying,
scsi_test_unit_ready() will return a negative error code (e.g., -ENODEV or
-ENOMEM) without zeroing or populating the sense_hdr buffer.

This condition then unconditionally evaluates sense_hdr.sense_key and
sense_hdr.sense_code before checking if retval indicates a local failure.

If alua_tur() is called from a multipath workqueue thread immediately after a
previous successful ALUA state transition check on another LUN, the stack
memory will identically retain the previous struct's values. This
uninitialized read could match the condition perfectly, causing alua_tur() to
wrongly return SCSI_DH_RETRY instead of the expected SCSI_DH_IO for a local
error, severely delaying multipath failover.

>  		return SCSI_DH_RETRY;
> -	else if (retval)
> +	if (retval)
>  		return SCSI_DH_IO;
> -	else
> -		return SCSI_DH_OK;
> +	return SCSI_DH_OK;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907024343.1046387-1-dlemoal@kernel.org?part=16