public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] scsi: core: Fix error handler encryption support
@ 2025-12-02  1:15 Po-Wen Kao
  2025-12-02  1:33 ` Bart Van Assche
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Po-Wen Kao @ 2025-12-02  1:15 UTC (permalink / raw)
  Cc: Brian Kao, James E.J. Bottomley, Martin K. Petersen,
	open list:SCSI SUBSYSTEM, open list

From: Brian Kao <powenkao@google.com>

Some low-level drivers (LLD) access block layer crypto fields, such as
rq->crypt_keyslot and rq->crypt_ctx within `struct request`, to
configure hardware for inline encryption.
However, SCSI Error Handling (EH) commands (e.g., TEST UNIT READY,
START STOP UNIT) should not involve any encryption setup.

To prevent drivers from erroneously applying crypto settings during EH,
this patch saves the original values of rq->crypt_keyslot and
rq->crypt_ctx before an EH command is prepared via scsi_eh_prep_cmnd().
These fields in the `struct request` are then set to NULL.
The original values are restored in scsi_eh_restore_cmnd() after the EH
command completes.

This ensures that the block layer crypto context does not leak into
EH command execution.

Tested-by: Brian Kao <powenkao@google.com>
Signed-off-by: Brian Kao <powenkao@google.com>
---
 drivers/scsi/scsi_error.c | 16 ++++++++++++++++
 include/scsi/scsi_eh.h    |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 10154d78e3360..2d0df74df703a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1040,6 +1040,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 			unsigned char *cmnd, int cmnd_size, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
+	struct request *rq = scsi_cmd_to_rq(scmd);
 
 	/*
 	 * We need saved copies of a number of fields - this is because
@@ -1091,6 +1092,16 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 		scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
 			(sdev->lun << 5 & 0xe0);
 
+	/*
+	 * Encryption must be disabled for the commands submitted by the error handler.
+	 * Hence, clear the encryption context information.
+	 */
+	ses->rq_crypt_keyslot = rq->crypt_keyslot;
+	ses->rq_crypt_ctx = rq->crypt_ctx;
+
+	rq->crypt_keyslot = NULL;
+	rq->crypt_ctx = NULL;
+
 	/*
 	 * Zero the sense buffer.  The scsi spec mandates that any
 	 * untransferred sense data should be interpreted as being zero.
@@ -1108,6 +1119,8 @@ EXPORT_SYMBOL(scsi_eh_prep_cmnd);
  */
 void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 {
+	struct request *rq = scsi_cmd_to_rq(scmd);
+
 	/*
 	 * Restore original data
 	 */
@@ -1120,6 +1133,9 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 	scmd->underflow = ses->underflow;
 	scmd->prot_op = ses->prot_op;
 	scmd->eh_eflags = ses->eh_eflags;
+
+	rq->crypt_keyslot = ses->rq_crypt_keyslot;
+	rq->crypt_ctx = ses->rq_crypt_ctx;
 }
 EXPORT_SYMBOL(scsi_eh_restore_cmnd);
 
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 1ae08e81339fa..9ef97f7820886 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -41,6 +41,10 @@ struct scsi_eh_save {
 	unsigned char cmnd[32];
 	struct scsi_data_buffer sdb;
 	struct scatterlist sense_sgl;
+
+	/* struct request fields */
+	struct bio_crypt_ctx *rq_crypt_ctx;
+	struct blk_crypto_keyslot *rq_crypt_keyslot;
 };
 
 extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
-- 
2.52.0.177.g9f829587af-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] scsi: core: Fix error handler encryption support
  2025-12-02  1:15 [PATCH 1/1] scsi: core: Fix error handler encryption support Po-Wen Kao
@ 2025-12-02  1:33 ` Bart Van Assche
  2025-12-02 16:42 ` kernel test robot
  2025-12-02 19:20 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2025-12-02  1:33 UTC (permalink / raw)
  To: Po-Wen Kao
  Cc: James E.J. Bottomley, Martin K. Petersen,
	open list:SCSI SUBSYSTEM, open list, Eric Biggers

On 12/1/25 5:15 PM, Po-Wen Kao wrote:

(+Eric Biggers)

> Some low-level drivers (LLD) access block layer crypto fields, such as
> rq->crypt_keyslot and rq->crypt_ctx within `struct request`, to
> configure hardware for inline encryption.
> However, SCSI Error Handling (EH) commands (e.g., TEST UNIT READY,
> START STOP UNIT) should not involve any encryption setup.
> 
> To prevent drivers from erroneously applying crypto settings during EH,
> this patch saves the original values of rq->crypt_keyslot and
> rq->crypt_ctx before an EH command is prepared via scsi_eh_prep_cmnd().
> These fields in the `struct request` are then set to NULL.
> The original values are restored in scsi_eh_restore_cmnd() after the EH
> command completes.
> 
> This ensures that the block layer crypto context does not leak into
> EH command execution.
Since I suggested this approach I think it is appropriate to add the
following:

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] scsi: core: Fix error handler encryption support
  2025-12-02  1:15 [PATCH 1/1] scsi: core: Fix error handler encryption support Po-Wen Kao
  2025-12-02  1:33 ` Bart Van Assche
@ 2025-12-02 16:42 ` kernel test robot
  2025-12-02 19:20 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-12-02 16:42 UTC (permalink / raw)
  To: Po-Wen Kao
  Cc: oe-kbuild-all, Brian Kao, James E.J. Bottomley,
	Martin K. Petersen, open list:SCSI SUBSYSTEM, open list

Hi Po-Wen,

kernel test robot noticed the following build errors:

[auto build test ERROR on jejb-scsi/for-next]
[also build test ERROR on mkp-scsi/for-next linus/master v6.18 next-20251202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Po-Wen-Kao/scsi-core-Fix-error-handler-encryption-support/20251202-091809
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
patch link:    https://lore.kernel.org/r/20251202011529.73738-1-powenkao%40google.com
patch subject: [PATCH 1/1] scsi: core: Fix error handler encryption support
config: x86_64-randconfig-161-20251202 (https://download.01.org/0day-ci/archive/20251203/202512030031.cPHecpfo-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251203/202512030031.cPHecpfo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512030031.cPHecpfo-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/scsi/scsi_error.c:1122:30: error: no member named 'crypt_keyslot' in 'struct request'
    1122 |         ses->rq_crypt_keyslot = rq->crypt_keyslot;
         |                                 ~~  ^
>> drivers/scsi/scsi_error.c:1123:26: error: no member named 'crypt_ctx' in 'struct request'
    1123 |         ses->rq_crypt_ctx = rq->crypt_ctx;
         |                             ~~  ^
   drivers/scsi/scsi_error.c:1125:6: error: no member named 'crypt_keyslot' in 'struct request'
    1125 |         rq->crypt_keyslot = NULL;
         |         ~~  ^
   drivers/scsi/scsi_error.c:1126:6: error: no member named 'crypt_ctx' in 'struct request'
    1126 |         rq->crypt_ctx = NULL;
         |         ~~  ^
   drivers/scsi/scsi_error.c:1160:6: error: no member named 'crypt_keyslot' in 'struct request'
    1160 |         rq->crypt_keyslot = ses->rq_crypt_keyslot;
         |         ~~  ^
   drivers/scsi/scsi_error.c:1161:6: error: no member named 'crypt_ctx' in 'struct request'
    1161 |         rq->crypt_ctx = ses->rq_crypt_ctx;
         |         ~~  ^
   6 errors generated.


vim +1122 drivers/scsi/scsi_error.c

  1047	
  1048	/**
  1049	 * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recovery
  1050	 * @scmd:       SCSI command structure to hijack
  1051	 * @ses:        structure to save restore information
  1052	 * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  1053	 * @cmnd_size:  size in bytes of @cmnd (must be <= MAX_COMMAND_SIZE)
  1054	 * @sense_bytes: size of sense data to copy. or 0 (if != 0 @cmnd is ignored)
  1055	 *
  1056	 * This function is used to save a scsi command information before re-execution
  1057	 * as part of the error recovery process.  If @sense_bytes is 0 the command
  1058	 * sent must be one that does not transfer any data.  If @sense_bytes != 0
  1059	 * @cmnd is ignored and this functions sets up a REQUEST_SENSE command
  1060	 * and cmnd buffers to read @sense_bytes into @scmd->sense_buffer.
  1061	 */
  1062	void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
  1063				unsigned char *cmnd, int cmnd_size, unsigned sense_bytes)
  1064	{
  1065		struct scsi_device *sdev = scmd->device;
  1066		struct request *rq = scsi_cmd_to_rq(scmd);
  1067	
  1068		/*
  1069		 * We need saved copies of a number of fields - this is because
  1070		 * error handling may need to overwrite these with different values
  1071		 * to run different commands, and once error handling is complete,
  1072		 * we will need to restore these values prior to running the actual
  1073		 * command.
  1074		 */
  1075		ses->cmd_len = scmd->cmd_len;
  1076		ses->data_direction = scmd->sc_data_direction;
  1077		ses->sdb = scmd->sdb;
  1078		ses->result = scmd->result;
  1079		ses->resid_len = scmd->resid_len;
  1080		ses->underflow = scmd->underflow;
  1081		ses->prot_op = scmd->prot_op;
  1082		ses->eh_eflags = scmd->eh_eflags;
  1083	
  1084		scmd->prot_op = SCSI_PROT_NORMAL;
  1085		scmd->eh_eflags = 0;
  1086		memcpy(ses->cmnd, scmd->cmnd, sizeof(ses->cmnd));
  1087		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
  1088		memset(&scmd->sdb, 0, sizeof(scmd->sdb));
  1089		scmd->result = 0;
  1090		scmd->resid_len = 0;
  1091	
  1092		if (sense_bytes) {
  1093			scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
  1094						 sense_bytes);
  1095			sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
  1096				    scmd->sdb.length);
  1097			scmd->sdb.table.sgl = &ses->sense_sgl;
  1098			scmd->sc_data_direction = DMA_FROM_DEVICE;
  1099			scmd->sdb.table.nents = scmd->sdb.table.orig_nents = 1;
  1100			scmd->cmnd[0] = REQUEST_SENSE;
  1101			scmd->cmnd[4] = scmd->sdb.length;
  1102			scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
  1103		} else {
  1104			scmd->sc_data_direction = DMA_NONE;
  1105			if (cmnd) {
  1106				BUG_ON(cmnd_size > sizeof(scmd->cmnd));
  1107				memcpy(scmd->cmnd, cmnd, cmnd_size);
  1108				scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
  1109			}
  1110		}
  1111	
  1112		scmd->underflow = 0;
  1113	
  1114		if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
  1115			scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
  1116				(sdev->lun << 5 & 0xe0);
  1117	
  1118		/*
  1119		 * Encryption must be disabled for the commands submitted by the error handler.
  1120		 * Hence, clear the encryption context information.
  1121		 */
> 1122		ses->rq_crypt_keyslot = rq->crypt_keyslot;
> 1123		ses->rq_crypt_ctx = rq->crypt_ctx;
  1124	
  1125		rq->crypt_keyslot = NULL;
  1126		rq->crypt_ctx = NULL;
  1127	
  1128		/*
  1129		 * Zero the sense buffer.  The scsi spec mandates that any
  1130		 * untransferred sense data should be interpreted as being zero.
  1131		 */
  1132		memset(scmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
  1133	}
  1134	EXPORT_SYMBOL(scsi_eh_prep_cmnd);
  1135	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] scsi: core: Fix error handler encryption support
  2025-12-02  1:15 [PATCH 1/1] scsi: core: Fix error handler encryption support Po-Wen Kao
  2025-12-02  1:33 ` Bart Van Assche
  2025-12-02 16:42 ` kernel test robot
@ 2025-12-02 19:20 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-12-02 19:20 UTC (permalink / raw)
  To: Po-Wen Kao
  Cc: oe-kbuild-all, Brian Kao, James E.J. Bottomley,
	Martin K. Petersen, open list:SCSI SUBSYSTEM, open list

Hi Po-Wen,

kernel test robot noticed the following build errors:

[auto build test ERROR on jejb-scsi/for-next]
[also build test ERROR on mkp-scsi/for-next linus/master v6.18 next-20251202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Po-Wen-Kao/scsi-core-Fix-error-handler-encryption-support/20251202-091809
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
patch link:    https://lore.kernel.org/r/20251202011529.73738-1-powenkao%40google.com
patch subject: [PATCH 1/1] scsi: core: Fix error handler encryption support
config: i386-randconfig-013-20251202 (https://download.01.org/0day-ci/archive/20251203/202512030247.AllzCFdb-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251203/202512030247.AllzCFdb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512030247.AllzCFdb-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/scsi/scsi_error.c: In function 'scsi_eh_prep_cmnd':
>> drivers/scsi/scsi_error.c:1122:35: error: 'struct request' has no member named 'crypt_keyslot'
    1122 |         ses->rq_crypt_keyslot = rq->crypt_keyslot;
         |                                   ^~
>> drivers/scsi/scsi_error.c:1123:31: error: 'struct request' has no member named 'crypt_ctx'
    1123 |         ses->rq_crypt_ctx = rq->crypt_ctx;
         |                               ^~
   drivers/scsi/scsi_error.c:1125:11: error: 'struct request' has no member named 'crypt_keyslot'
    1125 |         rq->crypt_keyslot = NULL;
         |           ^~
   drivers/scsi/scsi_error.c:1126:11: error: 'struct request' has no member named 'crypt_ctx'
    1126 |         rq->crypt_ctx = NULL;
         |           ^~
   drivers/scsi/scsi_error.c: In function 'scsi_eh_restore_cmnd':
   drivers/scsi/scsi_error.c:1160:11: error: 'struct request' has no member named 'crypt_keyslot'
    1160 |         rq->crypt_keyslot = ses->rq_crypt_keyslot;
         |           ^~
   drivers/scsi/scsi_error.c:1161:11: error: 'struct request' has no member named 'crypt_ctx'
    1161 |         rq->crypt_ctx = ses->rq_crypt_ctx;
         |           ^~


vim +1122 drivers/scsi/scsi_error.c

  1047	
  1048	/**
  1049	 * scsi_eh_prep_cmnd  - Save a scsi command info as part of error recovery
  1050	 * @scmd:       SCSI command structure to hijack
  1051	 * @ses:        structure to save restore information
  1052	 * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
  1053	 * @cmnd_size:  size in bytes of @cmnd (must be <= MAX_COMMAND_SIZE)
  1054	 * @sense_bytes: size of sense data to copy. or 0 (if != 0 @cmnd is ignored)
  1055	 *
  1056	 * This function is used to save a scsi command information before re-execution
  1057	 * as part of the error recovery process.  If @sense_bytes is 0 the command
  1058	 * sent must be one that does not transfer any data.  If @sense_bytes != 0
  1059	 * @cmnd is ignored and this functions sets up a REQUEST_SENSE command
  1060	 * and cmnd buffers to read @sense_bytes into @scmd->sense_buffer.
  1061	 */
  1062	void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
  1063				unsigned char *cmnd, int cmnd_size, unsigned sense_bytes)
  1064	{
  1065		struct scsi_device *sdev = scmd->device;
  1066		struct request *rq = scsi_cmd_to_rq(scmd);
  1067	
  1068		/*
  1069		 * We need saved copies of a number of fields - this is because
  1070		 * error handling may need to overwrite these with different values
  1071		 * to run different commands, and once error handling is complete,
  1072		 * we will need to restore these values prior to running the actual
  1073		 * command.
  1074		 */
  1075		ses->cmd_len = scmd->cmd_len;
  1076		ses->data_direction = scmd->sc_data_direction;
  1077		ses->sdb = scmd->sdb;
  1078		ses->result = scmd->result;
  1079		ses->resid_len = scmd->resid_len;
  1080		ses->underflow = scmd->underflow;
  1081		ses->prot_op = scmd->prot_op;
  1082		ses->eh_eflags = scmd->eh_eflags;
  1083	
  1084		scmd->prot_op = SCSI_PROT_NORMAL;
  1085		scmd->eh_eflags = 0;
  1086		memcpy(ses->cmnd, scmd->cmnd, sizeof(ses->cmnd));
  1087		memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
  1088		memset(&scmd->sdb, 0, sizeof(scmd->sdb));
  1089		scmd->result = 0;
  1090		scmd->resid_len = 0;
  1091	
  1092		if (sense_bytes) {
  1093			scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
  1094						 sense_bytes);
  1095			sg_init_one(&ses->sense_sgl, scmd->sense_buffer,
  1096				    scmd->sdb.length);
  1097			scmd->sdb.table.sgl = &ses->sense_sgl;
  1098			scmd->sc_data_direction = DMA_FROM_DEVICE;
  1099			scmd->sdb.table.nents = scmd->sdb.table.orig_nents = 1;
  1100			scmd->cmnd[0] = REQUEST_SENSE;
  1101			scmd->cmnd[4] = scmd->sdb.length;
  1102			scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
  1103		} else {
  1104			scmd->sc_data_direction = DMA_NONE;
  1105			if (cmnd) {
  1106				BUG_ON(cmnd_size > sizeof(scmd->cmnd));
  1107				memcpy(scmd->cmnd, cmnd, cmnd_size);
  1108				scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
  1109			}
  1110		}
  1111	
  1112		scmd->underflow = 0;
  1113	
  1114		if (sdev->scsi_level <= SCSI_2 && sdev->scsi_level != SCSI_UNKNOWN)
  1115			scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
  1116				(sdev->lun << 5 & 0xe0);
  1117	
  1118		/*
  1119		 * Encryption must be disabled for the commands submitted by the error handler.
  1120		 * Hence, clear the encryption context information.
  1121		 */
> 1122		ses->rq_crypt_keyslot = rq->crypt_keyslot;
> 1123		ses->rq_crypt_ctx = rq->crypt_ctx;
  1124	
  1125		rq->crypt_keyslot = NULL;
  1126		rq->crypt_ctx = NULL;
  1127	
  1128		/*
  1129		 * Zero the sense buffer.  The scsi spec mandates that any
  1130		 * untransferred sense data should be interpreted as being zero.
  1131		 */
  1132		memset(scmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
  1133	}
  1134	EXPORT_SYMBOL(scsi_eh_prep_cmnd);
  1135	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-02 19:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02  1:15 [PATCH 1/1] scsi: core: Fix error handler encryption support Po-Wen Kao
2025-12-02  1:33 ` Bart Van Assche
2025-12-02 16:42 ` kernel test robot
2025-12-02 19:20 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox