diff mbox series

[v2,22/40] scsi: myrX: use 16-bits defined sense codes

Message ID 20260903034201.112211-23-dlemoal@kernel.org
State New
Headers show
Series Use defined 16-bits ASC/ASCQ combinations | expand

Commit Message

Damien Le Moal Sept. 3, 2026, 3:41 a.m. UTC 
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. This helps with
code clarity as the sense codes being processed are easier to test and
self-documented.

No functional change intended.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/scsi/myrb.c | 67 +++++++++++++++++++++++----------------------
 drivers/scsi/myrs.c | 23 ++++++++++------
 2 files changed, 48 insertions(+), 42 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 3, 2026, 4:01 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Data Corruption: Incorrect offset used for writing the SCSI mode parameter block descriptor corrupts the 24-bit block length.
- [High] Command Timeout: The REQUEST_SENSE command in myrb_ldev_queuecommand returns without calling scsi_done(), leading to a hanging command.
- [High] Resource Leak: Returning SCSI_MLQUEUE_HOST_BUSY after a successful scsi_dma_map() without unmapping causes a DMA mapping leak.
- [High] Memory Leak: DMA pool allocations are leaked on the SG mapping error path in myrs_queuecommand.
--

commit 3eaecf993e28ec2a9d1c5ee8b1fc650d068a56f8
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. This helps with
code clarity as the sense codes being processed are easier to test and
self-documented.

> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
> index 3678b66310ed0..e7d796add9864 100644
> --- a/drivers/scsi/myrb.c
> +++ b/drivers/scsi/myrb.c
[ ... ]
> @@ -1398,7 +1400,7 @@ myrb_mode_sense(struct myrb_hba *cb, struct scsi_cmnd *scmd,
>  static void myrb_request_sense(struct myrb_hba *cb,
>  		struct scsi_cmnd *scmd)
>  {
> -	scsi_build_sense(scmd, 0, NO_SENSE, 0, 0);
> +	scsi_set_sense(scmd, 0, NO_SENSE, NO_ADDITIONAL_SENSE_INFORMATION);
>  	scsi_sg_copy_from_buffer(scmd, scmd->sense_buffer,
>  				 SCSI_SENSE_BUFFERSIZE);
>  }

[Severity: High]
This is a pre-existing issue, but looking at how myrb_request_sense() is used
in myrb_ldev_queuecommand() in drivers/scsi/myrb.c:

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

Because returning 0 indicates to the SCSI midlayer that the driver has
accepted the command and will complete it asynchronously, does failing to call
scsi_done(scmd) here cause the command to hang indefinitely and eventually
trigger a SCSI timeout?

[ ... ]
> @@ -1461,8 +1463,8 @@ static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
>  	case MODE_SENSE:
>  		if ((scmd->cmnd[2] & 0x3F) != 0x3F &&
>  		    (scmd->cmnd[2] & 0x3F) != 0x08) {
> -			/* 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 {
>  			myrb_mode_sense(cb, scmd, ldev_info);

[Severity: Critical]
This is a pre-existing issue, but I noticed myrb_mode_sense() in
drivers/scsi/myrb.c writes the block size to the mode page block descriptor
using put_unaligned_be32():

static void myrb_mode_sense(struct myrb_hba *cb, struct scsi_cmnd *scmd,
		struct myrb_ldev_info *ldev_info)
{
...
		put_unaligned_be32(cb->ldev_block_size, &block_desc[5]);

Since the block length in the descriptor is a 24-bit field located at offsets
5, 6, and 7, won't writing a 32-bit value at offset 5 overflow the LSB into
offset 8 (the start of the mode page header)? For a typical 512 byte block
size, it seems this writes 2 instead of 512, completely corrupting capacity
reporting.

>  			scmd->result = (DID_OK << 16);
[ ... ]
> @@ -1544,8 +1546,8 @@ static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
>  		block_cnt = get_unaligned_be32(&scmd->cmnd[6]);
>  		break;
>  	default:
> -		/* Illegal request, invalid opcode */
> -		scsi_build_sense(scmd, 0, ILLEGAL_REQUEST, 0x20, 0);
> +		scsi_set_sense(scmd, 0, ILLEGAL_REQUEST,
> +			       INVALID_COMMAND_OP_CODE);
>  		scsi_done(scmd);
>  		return 0;
>  	}

[Severity: High]
This is a pre-existing issue, but later in myrb_ldev_queuecommand() in
drivers/scsi/myrb.c, after the switch statement, the function maps DMA memory:

	nsge = scsi_dma_map(scmd);
    ...
	} else {
		struct myrb_sge *hw_sgl;
		dma_addr_t hw_sgl_addr;
		int i;

		hw_sgl = dma_pool_alloc(cb->sg_pool, GFP_ATOMIC, &hw_sgl_addr);
		if (!hw_sgl)
			return SCSI_MLQUEUE_HOST_BUSY;

Does returning SCSI_MLQUEUE_HOST_BUSY without calling scsi_dma_unmap() cause
a DMA mapping and IOMMU resource leak when the midlayer retries the command
and maps it again?

> diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
> index afd68225221a6..1192b01fab8ec 100644
> --- a/drivers/scsi/myrs.c
> +++ b/drivers/scsi/myrs.c
[ ... ]
> @@ -1603,7 +1607,8 @@ static enum scsi_qc_status myrs_queuecommand(struct Scsi_Host *shost,
>  
>  	switch (scmd->cmnd[0]) {
>  	case REPORT_LUNS:
> -		scsi_build_sense(scmd, 0, ILLEGAL_REQUEST, 0x20, 0x0);
> +		scsi_set_sense(scmd, 0, ILLEGAL_REQUEST,
> +			       INVALID_COMMAND_OP_CODE);
>  		scsi_done(scmd);
>  		return 0;
>  	case MODE_SENSE:
> @@ -1612,8 +1617,8 @@ static enum scsi_qc_status myrs_queuecommand(struct Scsi_Host *shost,
>  
>  			if ((scmd->cmnd[2] & 0x3F) != 0x3F &&
>  			    (scmd->cmnd[2] & 0x3F) != 0x08) {
> -				/* 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 {
>  				myrs_mode_sense(cs, scmd, ldev_info);

[Severity: Critical]
This is a pre-existing issue, but myrs_mode_sense() in drivers/scsi/myrs.c
has the identical bug as myrb_mode_sense() when writing the mode page block
descriptor:

static void myrs_mode_sense(struct myrs_hba *cs, struct scsi_cmnd *scmd,
		struct myrs_ldev_info *ldev_info)
{
...
		put_unaligned_be32(ldev_info->devsize_bytes, &block_desc[5]);

Does this similarly corrupt the 24-bit block length by writing a 32-bit value?

[Severity: High]
Additionally, this is another pre-existing issue in myrs_queuecommand() in
drivers/scsi/myrs.c. When processing commands with scatterlists, if
dma_pool_alloc() fails it returns SCSI_MLQUEUE_HOST_BUSY:

			hw_sgl = dma_pool_alloc(cs->sg_pool, GFP_ATOMIC,
						&hw_sgl_addr);
			if (WARN_ON(!hw_sgl)) {
				if (cmd_blk->dcdb) {
					dma_pool_free(cs->dcdb_pool,
						      cmd_blk->dcdb,
						      cmd_blk->dcdb_dma);
...
				return SCSI_MLQUEUE_HOST_BUSY;
			}

Since scsi_dma_map() was already called earlier in the function, does
returning here without calling scsi_dma_unmap() leak the DMA mapping?

[Severity: High]
Finally, in the same scatterlist loop in myrs_queuecommand(), there is a
sanity check on hw_sgl:

		scsi_for_each_sg(scmd, sgl, nsge, i) {
			if (WARN_ON(!hw_sgl)) {
				scsi_dma_unmap(scmd);
				scmd->result = (DID_ERROR << 16);
				scsi_done(scmd);
				return 0;
			}

When taking this error path, it calls scsi_done(scmd) but bypasses the normal
completion routine myrs_handle_scsi(). Does this permanently leak the
dma_pool_alloc() blocks for cmd_blk->sense and cmd_blk->dcdb?
Hannes Reinecke Sept. 7, 2026, 12:34 p.m. UTC | #2 
On 9/3/26 5:41 AM, Damien Le Moal wrote:
> 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. This helps with
> code clarity as the sense codes being processed are easier to test and
> self-documented.
> 
> No functional change intended.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/scsi/myrb.c | 67 +++++++++++++++++++++++----------------------
>   drivers/scsi/myrs.c | 23 ++++++++++------
>   2 files changed, 48 insertions(+), 42 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
diff mbox series

Patch

diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index 3678b66310ed..e7d796add986 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -314,17 +314,19 @@  static void myrb_get_event(struct myrb_hba *cb, unsigned int event)
 		scsi_normalize_sense(ev_buf->sense, 32, &sshdr);
 
 		if (sshdr.sense_key == VENDOR_SPECIFIC &&
-		    sshdr.asc == 0x80 &&
-		    sshdr.ascq < ARRAY_SIZE(myrb_event_msg))
+		    scsi_sense_asc(&sshdr) == 0x80 &&
+		    scsi_sense_ascq(&sshdr) < ARRAY_SIZE(myrb_event_msg))
 			shost_printk(KERN_CRIT, cb->host,
 				     "Physical drive %d:%d: %s\n",
 				     ev_buf->channel, ev_buf->target,
-				     myrb_event_msg[sshdr.ascq]);
+				     myrb_event_msg[scsi_sense_ascq(&sshdr)]);
 		else
 			shost_printk(KERN_CRIT, cb->host,
 				     "Physical drive %d:%d: Sense: %X/%02X/%02X\n",
 				     ev_buf->channel, ev_buf->target,
-				     sshdr.sense_key, sshdr.asc, sshdr.ascq);
+				     sshdr.sense_key,
+				     scsi_sense_asc(&sshdr),
+				     scsi_sense_ascq(&sshdr));
 	}
 
 	dma_free_coherent(&cb->pdev->dev, sizeof(struct myrb_log_entry),
@@ -1398,7 +1400,7 @@  myrb_mode_sense(struct myrb_hba *cb, struct scsi_cmnd *scmd,
 static void myrb_request_sense(struct myrb_hba *cb,
 		struct scsi_cmnd *scmd)
 {
-	scsi_build_sense(scmd, 0, NO_SENSE, 0, 0);
+	scsi_set_sense(scmd, 0, NO_SENSE, NO_ADDITIONAL_SENSE_INFORMATION);
 	scsi_sg_copy_from_buffer(scmd, scmd->sense_buffer,
 				 SCSI_SENSE_BUFFERSIZE);
 }
@@ -1446,8 +1448,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 		return 0;
 	case INQUIRY:
 		if (scmd->cmnd[1] & 1) {
-			/* 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 {
 			myrb_inquiry(cb, scmd);
 			scmd->result = (DID_OK << 16);
@@ -1461,8 +1463,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 	case MODE_SENSE:
 		if ((scmd->cmnd[2] & 0x3F) != 0x3F &&
 		    (scmd->cmnd[2] & 0x3F) != 0x08) {
-			/* 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 {
 			myrb_mode_sense(cb, scmd, ldev_info);
 			scmd->result = (DID_OK << 16);
@@ -1472,15 +1474,15 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 	case READ_CAPACITY:
 		if ((scmd->cmnd[1] & 1) ||
 		    (scmd->cmnd[8] & 1)) {
-			/* 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);
 			scsi_done(scmd);
 			return 0;
 		}
 		lba = get_unaligned_be32(&scmd->cmnd[2]);
 		if (lba) {
-			/* 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);
 			scsi_done(scmd);
 			return 0;
 		}
@@ -1493,8 +1495,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 		return 0;
 	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);
@@ -1503,8 +1505,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 		return 0;
 	case READ_6:
 		if (ldev_info->state == MYRB_DEVICE_WO) {
-			/* Data protect, attempt to read invalid data */
-			scsi_build_sense(scmd, 0, DATA_PROTECT, 0x21, 0x06);
+			scsi_set_sense(scmd, 0, DATA_PROTECT,
+				       ATTEMPT_TO_READ_INVALID_DATA);
 			scsi_done(scmd);
 			return 0;
 		}
@@ -1517,8 +1519,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 		break;
 	case READ_10:
 		if (ldev_info->state == MYRB_DEVICE_WO) {
-			/* Data protect, attempt to read invalid data */
-			scsi_build_sense(scmd, 0, DATA_PROTECT, 0x21, 0x06);
+			scsi_set_sense(scmd, 0, DATA_PROTECT,
+				       ATTEMPT_TO_READ_INVALID_DATA);
 			scsi_done(scmd);
 			return 0;
 		}
@@ -1531,8 +1533,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 		break;
 	case READ_12:
 		if (ldev_info->state == MYRB_DEVICE_WO) {
-			/* Data protect, attempt to read invalid data */
-			scsi_build_sense(scmd, 0, DATA_PROTECT, 0x21, 0x06);
+			scsi_set_sense(scmd, 0, DATA_PROTECT,
+				       ATTEMPT_TO_READ_INVALID_DATA);
 			scsi_done(scmd);
 			return 0;
 		}
@@ -1544,8 +1546,8 @@  static enum scsi_qc_status myrb_ldev_queuecommand(struct Scsi_Host *shost,
 		block_cnt = get_unaligned_be32(&scmd->cmnd[6]);
 		break;
 	default:
-		/* Illegal request, invalid opcode */
-		scsi_build_sense(scmd, 0, ILLEGAL_REQUEST, 0x20, 0);
+		scsi_set_sense(scmd, 0, ILLEGAL_REQUEST,
+			       INVALID_COMMAND_OP_CODE);
 		scsi_done(scmd);
 		return 0;
 	}
@@ -2328,20 +2330,19 @@  static void myrb_handle_scsi(struct myrb_hba *cb, struct myrb_cmdblk *cmd_blk,
 		dev_dbg(&scmd->device->sdev_gendev,
 			"Bad Data Encountered\n");
 		if (scmd->sc_data_direction == DMA_FROM_DEVICE)
-			/* Unrecovered read error */
-			scsi_build_sense(scmd, 0, MEDIUM_ERROR, 0x11, 0);
+			scsi_set_sense(scmd, 0, MEDIUM_ERROR,
+					 UNRECOVERED_READ_ERROR);
 		else
-			/* Write error */
-			scsi_build_sense(scmd, 0, MEDIUM_ERROR, 0x0C, 0);
+			scsi_set_sense(scmd, 0, MEDIUM_ERROR, WRITE_ERROR);
 		break;
 	case MYRB_STATUS_IRRECOVERABLE_DATA_ERROR:
 		scmd_printk(KERN_ERR, scmd, "Irrecoverable Data Error\n");
 		if (scmd->sc_data_direction == DMA_FROM_DEVICE)
-			/* Unrecovered read error, auto-reallocation failed */
-			scsi_build_sense(scmd, 0, MEDIUM_ERROR, 0x11, 0x04);
+			scsi_set_sense(scmd, 0, MEDIUM_ERROR,
+				UNRECOVERED_READ_ERROR_AUTO_REALLOCATE_FAILED);
 		else
-			/* Write error, auto-reallocation failed */
-			scsi_build_sense(scmd, 0, MEDIUM_ERROR, 0x0C, 0x02);
+			scsi_set_sense(scmd, 0, MEDIUM_ERROR,
+				       WRITE_ERROR_AUTO_REALLOCATION_FAILED);
 		break;
 	case MYRB_STATUS_LDRV_NONEXISTENT_OR_OFFLINE:
 		dev_dbg(&scmd->device->sdev_gendev,
@@ -2351,8 +2352,8 @@  static void myrb_handle_scsi(struct myrb_hba *cb, struct myrb_cmdblk *cmd_blk,
 	case MYRB_STATUS_ACCESS_BEYOND_END_OF_LDRV:
 		dev_dbg(&scmd->device->sdev_gendev,
 			    "Attempt to Access Beyond End of Logical Drive");
-		/* Logical block address out of range */
-		scsi_build_sense(scmd, 0, NOT_READY, 0x21, 0);
+		scsi_set_sense(scmd, 0, NOT_READY,
+			       LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE);
 		break;
 	case MYRB_STATUS_DEVICE_NONRESPONSIVE:
 		dev_dbg(&scmd->device->sdev_gendev, "Device nonresponsive\n");
diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
index afd68225221a..1192b01fab8e 100644
--- a/drivers/scsi/myrs.c
+++ b/drivers/scsi/myrs.c
@@ -816,7 +816,7 @@  static void myrs_log_event(struct myrs_hba *cs, struct myrs_event *ev)
 	struct Scsi_Host *shost = cs->host;
 	struct scsi_device *sdev;
 	struct scsi_sense_hdr sshdr = {0};
-	unsigned char sense_info[4];
+	unsigned char asc, sense_info[4];
 	unsigned char cmd_specific[4];
 
 	if (ev->ev_code == 0x1C) {
@@ -829,9 +829,11 @@  static void myrs_log_event(struct myrs_hba *cs, struct myrs_event *ev)
 			memcpy(cmd_specific, &ev->sense_data[7], 4);
 		}
 	}
+	asc = scsi_sense_asc(&sshdr);
 	if (sshdr.sense_key == VENDOR_SPECIFIC &&
-	    (sshdr.asc == 0x80 || sshdr.asc == 0x81))
-		ev->ev_code = ((sshdr.asc - 0x80) << 8 | sshdr.ascq);
+	    (asc == 0x80 || asc == 0x81))
+		ev->ev_code = scsi_sense_code(asc - 0x80,
+					      scsi_sense_ascq(&sshdr));
 	while (true) {
 		ev_code = myrs_ev_list[ev_idx].ev_code;
 		if (ev_code == ev->ev_code || ev_code == 0)
@@ -891,8 +893,9 @@  static void myrs_log_event(struct myrs_hba *cs, struct myrs_event *ev)
 	case 'S':
 		if (sshdr.sense_key == NO_SENSE ||
 		    (sshdr.sense_key == NOT_READY &&
-		     sshdr.asc == 0x04 && (sshdr.ascq == 0x01 ||
-					    sshdr.ascq == 0x02)))
+		     (sshdr.sense_code == LU_IS_IN_PROCESS_OF_BECOMING_READY ||
+		      sshdr.sense_code ==
+		      LU_NOT_READY_INITIALIZING_COMMAND_REQUIRED)))
 			break;
 		shost_printk(KERN_INFO, shost,
 			     "event %d: Physical Device %d:%d %s\n",
@@ -900,7 +903,8 @@  static void myrs_log_event(struct myrs_hba *cs, struct myrs_event *ev)
 		shost_printk(KERN_INFO, shost,
 			     "Physical Device %d:%d Sense Key = %X, ASC = %02X, ASCQ = %02X\n",
 			     ev->channel, ev->target,
-			     sshdr.sense_key, sshdr.asc, sshdr.ascq);
+			     sshdr.sense_key, scsi_sense_asc(&sshdr),
+			     scsi_sense_ascq(&sshdr));
 		shost_printk(KERN_INFO, shost,
 			     "Physical Device %d:%d Sense Information = %02X%02X%02X%02X %02X%02X%02X%02X\n",
 			     ev->channel, ev->target,
@@ -1603,7 +1607,8 @@  static enum scsi_qc_status myrs_queuecommand(struct Scsi_Host *shost,
 
 	switch (scmd->cmnd[0]) {
 	case REPORT_LUNS:
-		scsi_build_sense(scmd, 0, ILLEGAL_REQUEST, 0x20, 0x0);
+		scsi_set_sense(scmd, 0, ILLEGAL_REQUEST,
+			       INVALID_COMMAND_OP_CODE);
 		scsi_done(scmd);
 		return 0;
 	case MODE_SENSE:
@@ -1612,8 +1617,8 @@  static enum scsi_qc_status myrs_queuecommand(struct Scsi_Host *shost,
 
 			if ((scmd->cmnd[2] & 0x3F) != 0x3F &&
 			    (scmd->cmnd[2] & 0x3F) != 0x08) {
-				/* 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 {
 				myrs_mode_sense(cs, scmd, ldev_info);
 				scmd->result = (DID_OK << 16);