[PATCH v2 1/1] scsi: core: Fix error handler encryption support

[PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Po-Wen Kao]

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>
---

Changes in v2:
- Guard inline encryption fields with CONFIG_BLK_INLINE_ENCRYPTION to
  fix build errors when the feature is disabled.

 drivers/scsi/scsi_error.c | 24 ++++++++++++++++++++++++
 include/scsi/scsi_eh.h    |  6 ++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index f869108fd969..eebca96c1fc1 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1063,6 +1063,9 @@ 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;
+#ifdef CONFIG_BLK_INLINE_ENCRYPTION
+	struct request *rq = scsi_cmd_to_rq(scmd);
+#endif

 	/*
 	 * We need saved copies of a number of fields - this is because
@@ -1114,6 +1117,18 @@ 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.
+	 */
+#ifdef CONFIG_BLK_INLINE_ENCRYPTION
+	ses->rq_crypt_keyslot = rq->crypt_keyslot;
+	ses->rq_crypt_ctx = rq->crypt_ctx;
+
+	rq->crypt_keyslot = NULL;
+	rq->crypt_ctx = NULL;
+#endif
+
 	/*
 	 * Zero the sense buffer.  The scsi spec mandates that any
 	 * untransferred sense data should be interpreted as being zero.
@@ -1131,6 +1146,10 @@ EXPORT_SYMBOL(scsi_eh_prep_cmnd);
  */
 void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 {
+#ifdef CONFIG_BLK_INLINE_ENCRYPTION
+	struct request *rq = scsi_cmd_to_rq(scmd);
+#endif
+
 	/*
 	 * Restore original data
 	 */
@@ -1143,6 +1162,11 @@ 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;
+
+#ifdef CONFIG_BLK_INLINE_ENCRYPTION
+	rq->crypt_keyslot = ses->rq_crypt_keyslot;
+	rq->crypt_ctx = ses->rq_crypt_ctx;
+#endif
 }
 EXPORT_SYMBOL(scsi_eh_restore_cmnd);

diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 1ae08e81339f..15679be90c5c 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -41,6 +41,12 @@ struct scsi_eh_save {
 	unsigned char cmnd[32];
 	struct scsi_data_buffer sdb;
 	struct scatterlist sense_sgl;
+
+	/* struct request fields */
+#ifdef CONFIG_BLK_INLINE_ENCRYPTION
+	struct bio_crypt_ctx *rq_crypt_ctx;
+	struct blk_crypto_keyslot *rq_crypt_keyslot;
+#endif
 };

 extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
--
2.52.0.223.gf5cc29aaa4-goog

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Christoph Hellwig]

On Wed, Dec 03, 2025 at 07:33:08AM +0000, Po-Wen Kao wrote:
> 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.

So don't do that except for commands that can actually be encrypted,
i.e. those that have non-zero payload size.  I think you really want
to fix this in the driver.

And we really need to stop passing scsi_cmnds to the error handler.

Hannes, any chance you could send another batch of your decades old
series?

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Hannes Reinecke]

On 12/3/25 08:38, Christoph Hellwig wrote:
> On Wed, Dec 03, 2025 at 07:33:08AM +0000, Po-Wen Kao wrote:
>> 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.
> 
> So don't do that except for commands that can actually be encrypted,
> i.e. those that have non-zero payload size.  I think you really want
> to fix this in the driver.
> 
> And we really need to stop passing scsi_cmnds to the error handler.
> 
> Hannes, any chance you could send another batch of your decades old
> series?
> 
There had been an intersection with the reserved command stuff, but
now that Bart has dusted things off there I guess I should give it
another go.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.com                               +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Bart Van Assche]

On 12/2/25 10:42 PM, Hannes Reinecke wrote:
> There had been an intersection with the reserved command stuff, but
> now that Bart has dusted things off there I guess I should give it
> another go.

Does that patch series perhaps involve allocating a reserved command
from inside the SCSI error handler? Won't that break SCSI LLDs that
restrict the queue depth to one? I think that the following SCSI LLDs
only support one command (.can_queue = 1):
* drivers/scsi/fdomain.c
* drivers/scsi/mac53c94.c
* drivers/scsi/ppa.c
* drivers/scsi/imm.c
* drivers/scsi/aha152x.c

Thanks,

Bart.

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Bart Van Assche]

On 12/2/25 9:38 PM, Christoph Hellwig wrote:
> On Wed, Dec 03, 2025 at 07:33:08AM +0000, Po-Wen Kao wrote:
>> 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.
> 
> So don't do that except for commands that can actually be encrypted,
> i.e. those that have non-zero payload size.  I think you really want
> to fix this in the driver.

That would make it impossible to submit SCSI commands from the SCSI
error handler that read data, e.g. to check that the medium is still
readable. I think that the approach of this patch is better than
requiring that every SCSI LLD driver that supports inline encryption
only sets up inline encryption for commands that have a non-zero
payload size.

Thanks,

Bart.

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Hannes Reinecke]

On 12/3/25 16:55, Bart Van Assche wrote:
> On 12/2/25 10:42 PM, Hannes Reinecke wrote:
>> There had been an intersection with the reserved command stuff, but
>> now that Bart has dusted things off there I guess I should give it
>> another go.
> 
> Does that patch series perhaps involve allocating a reserved command
> from inside the SCSI error handler? Won't that break SCSI LLDs that
> restrict the queue depth to one? I think that the following SCSI LLDs
> only support one command (.can_queue = 1):

reserved commands are only implemented for adapters which require an
LLD specific 'tag' to send TMFs (eg fnic, aacraid, or hpsa).
Most HBAs (especially the older ones) are not that elaborate, and
there TMFs are not commands per se but rather operations on the HBA.

EG fdomain host_reset is just settings some bits in some registers,
no command allocation needed at all.

So for those we don't need to allocate _any_ commands for TMFs,
consequently we don't need to implement reserved commands and
hence TMFs are not guarded by .can_queue at all.

> * drivers/scsi/fdomain.c
> * drivers/scsi/mac53c94.c
> * drivers/scsi/ppa.c
> * drivers/scsi/imm.c
> * drivers/scsi/aha152x.c
> 
Cheers,
Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.com                               +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Christoph Hellwig]

On Wed, Dec 03, 2025 at 05:57:49AM -1000, Bart Van Assche wrote:
> That would make it impossible to submit SCSI commands from the SCSI
> error handler that read data, e.g. to check that the medium is still
> readable.

That's already impossible right now, because data mapping isn't handled
for EH commands.

> I think that the approach of this patch is better than
> requiring that every SCSI LLD driver that supports inline encryption
> only sets up inline encryption for commands that have a non-zero
> payload size.

There is exactly one such driver (ufshcd) and the error handler needs
to clean up how it sends commands anyway.

Re: [PATCH v2 1/1] scsi: core: Fix error handler encryption support

[Brian Kao]

Hi all,

Thanks for the review. I will fix this in ufshcd instead. Let's drop this patch.