diff mbox series

[1/2] scsi: save/restore command resid for error handling

Message ID 20190926220844.26631-2-damien.lemoal@wdc.com (mailing list archive)
State Superseded
Headers show
Series Fix SCSI & USB Storage CHECK CONDITION handling | expand

Commit Message

Damien Le Moal Sept. 26, 2019, 10:08 p.m. UTC
When a command is terminated with CHECK CONDITION and request sense
executed by hijacking the command descriptor, the original command resid
is lost and replaced with the resid from the execution of request sense.
If based on the obtained sense data the command is aborted and not
retried, the resid that will be seen by drivers such as sd will be the
resid of the request sense execution and not the value from the original
command failure. Make sure this does not happen by adding resid as part
of the command information saved using struct scsi_eh_save.

Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/scsi_error.c | 2 ++
 include/scsi/scsi_eh.h    | 1 +
 2 files changed, 3 insertions(+)

Comments

Christoph Hellwig Sept. 27, 2019, 7:20 p.m. UTC | #1
On Fri, Sep 27, 2019 at 07:08:43AM +0900, Damien Le Moal wrote:
> When a command is terminated with CHECK CONDITION and request sense
> executed by hijacking the command descriptor, the original command resid
> is lost and replaced with the resid from the execution of request sense.
> If based on the obtained sense data the command is aborted and not
> retried, the resid that will be seen by drivers such as sd will be the
> resid of the request sense execution and not the value from the original
> command failure. Make sure this does not happen by adding resid as part
> of the command information saved using struct scsi_eh_save.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/scsi/scsi_error.c | 2 ++
>  include/scsi/scsi_eh.h    | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 1c470e31ae81..d4ac13979189 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -967,6 +967,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
>  	ses->data_direction = scmd->sc_data_direction;
>  	ses->sdb = scmd->sdb;
>  	ses->result = scmd->result;
> +	ses->resid = scsi_get_resid(scmd);

Don't we also need to reset the resid to 0 here as we do in the
queuecommand path?  That would take care of what you are trying to do
for usb-storage in the next patch in generic code.
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 1c470e31ae81..d4ac13979189 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -967,6 +967,7 @@  void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 	ses->data_direction = scmd->sc_data_direction;
 	ses->sdb = scmd->sdb;
 	ses->result = scmd->result;
+	ses->resid = scsi_get_resid(scmd);
 	ses->underflow = scmd->underflow;
 	ses->prot_op = scmd->prot_op;
 	ses->eh_eflags = scmd->eh_eflags;
@@ -1029,6 +1030,7 @@  void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 	scmd->sc_data_direction = ses->data_direction;
 	scmd->sdb = ses->sdb;
 	scmd->result = ses->result;
+	scsi_set_resid(scmd, ses->resid);
 	scmd->underflow = ses->underflow;
 	scmd->prot_op = ses->prot_op;
 	scmd->eh_eflags = ses->eh_eflags;
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 3810b340551c..9caa9b262a32 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -32,6 +32,7 @@  extern int scsi_ioctl_reset(struct scsi_device *, int __user *);
 struct scsi_eh_save {
 	/* saved state */
 	int result;
+	unsigned int resid;
 	int eh_eflags;
 	enum dma_data_direction data_direction;
 	unsigned underflow;