Re: [PATCH v6 22/40] scsi: myrX: 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] The REQUEST_SENSE command intercepts the CDB but returns without calling scsi_done(), causing the command to hang.
--

commit b860b6810bf948611bc8eabd52fcae5e876d0483
Author: Damien Le Moal <dlemoal@kernel.org>
scsi: myrX: use 16-bits defined sense codes

Refactor the Mylex RAID controller driver to use scsi_set_sense() and the
16-bits sense_code field of struct scsi_sense_hdr and replace all
hard-coded additional sense codes and additional sense code qualifiers
with the enum values defined in include/scsi/scsi_sense.h.

> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
> index 3678b66310ed0..95df6e5345f7a 100644
> --- a/drivers/scsi/myrb.c
> +++ b/drivers/scsi/myrb.c

[ ... ]

> @@ -1493,8 +1495,8 @@ static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
>  		return 0;

[Severity: High]
This is a pre-existing issue, but does this missing scsi_done() cause the
REQUEST_SENSE command to hang indefinitely?

Looking at myrb_ldev_queuecommand(), the REQUEST_SENSE command is intercepted
and returns without calling scsi_done():

	case REQUEST_SENSE:
		myrb_request_sense(cb, scmd);
		scmd->result = (DID_OK << 16);
		return 0;

Because this command is never completed, could it hang indefinitely and
eventually trigger the SCSI midlayer error handler?

Since userspace applications with access to the SCSI generic device
can send arbitrary SCSI commands via the SG_IO ioctl, could this be reached
from userspace and trigger a local denial of service by forcing SCSI error
recovery?

>  	case SEND_DIAGNOSTIC:
>  		if (scmd->cmnd[1] != 0x04) {
> -			/* Illegal request, invalid field in CDB */
> -			scsi_build_sense(scmd, 0, ILLEGAL_REQUEST, 0x24, 0);
> +			scsi_set_sense(scmd, 0, ILLEGAL_REQUEST,
> +				       INVALID_FIELD_IN_CDB);
>  		} else {
>  			/* Assume good status */
>  			scmd->result = (DID_OK << 16);

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