Re: [PATCH v3 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Reporting Option 3 fails to ignore the REQUESTED SERVICE ACTION field for commands that do not have service actions, causing supported commands to be reported as unsupported.
--

commit dc113e618a2abb6f7c7b971f3095bb3e3f08a158
Author: Damien Le Moal <[email protected]>

ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES

Add support for the reporting options value 2 of the in REPORT SUPPORTED
OPERATION CODES command in ata_scsi_report_supported_opcodes(). With this
reporting option, the specified opcode to check must have a service
action. Use ata_scsi_supported_cmd_has_sa() to check for this and fill the
command reply buffer using the one command format based on the command
support.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 4d4cdf3047405..5dd76579327e9 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3774,6 +3774,13 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev,
>  			return 0;
>  		}
>  		break;
> +	case 2:
> +		/* One command format, must have sa. */
> +		if (!ata_scsi_supported_cmd_use_sa(cdb[3])) {
> +			ata_scsi_set_invalid_field(dev, cmd, 3, 0xff);
> +			return 0;
> +		}
> +		fallthrough;
>  	case 3:
>  		/* One command format */
>  		sa = get_unaligned_be16(&cdb[4]);

[Severity: Medium]
Does this code incorrectly fail to ignore the REQUESTED SERVICE ACTION field
for commands without service actions under Option 3?

If a legitimate SCSI query uses Option 3 with a non-zero service action field
for a command that does not use service actions, this block extracts the
non-zero sa value from the CDB.

Later in ata_scsi_get_supported_cmd(), the code enforces a strict match:

	for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) {
		cmd = &ata_supported_cmds[i];
		if (cmd->op == op && cmd->sa == sa)
			return cmd;
	}

Because cmd->sa is 0 for commands without service actions, this strict match
will fail. Will this cause valid commands (like READ_10) to be incorrectly
reported as unsupported?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4