[PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the RESET path

[PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the RESET path

[K. Y. Srinivasan]

Properly account for I/O in transit before returning from the RESET call.
In the absense of this patch, we could have a situation where the host may
respond to a command that was issued prior to the issuance of the RESET
command at some arbitrary time after responding to the RESET command.
Currently, the host does not do anything with the RESET command. 

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: stable@vger.kernel.org
---
 drivers/scsi/storvsc_drv.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 528d52b..0144078 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1221,7 +1221,12 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
 	/*
 	 * At this point, all outstanding requests in the adapter
 	 * should have been flushed out and return to us
+	 * There is a potential race here where the host may be in
+	 * the process of responding when we return from here.
+	 * Just wait for all in-transit packets to be accounted for
+	 * before we return from here.
 	 */
+	storvsc_wait_to_drain(stor_device);
 
 	return SUCCESS;
 }
-- 
1.7.4.1

RE: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the RESET path

[KY Srinivasan]

> -----Original Message-----
> From: K. Y. Srinivasan [mailto:kys@microsoft.com]
> Sent: Tuesday, October 02, 2012 2:04 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org
> Cc: KY Srinivasan; stable@vger.kernel.org
> Subject: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the
> RESET path
> 
> Properly account for I/O in transit before returning from the RESET call.
> In the absense of this patch, we could have a situation where the host may
> respond to a command that was issued prior to the issuance of the RESET
> command at some arbitrary time after responding to the RESET command.
> Currently, the host does not do anything with the RESET command.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/scsi/storvsc_drv.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 528d52b..0144078 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1221,7 +1221,12 @@ static int storvsc_host_reset_handler(struct scsi_cmnd
> *scmnd)
>  	/*
>  	 * At this point, all outstanding requests in the adapter
>  	 * should have been flushed out and return to us
> +	 * There is a potential race here where the host may be in
> +	 * the process of responding when we return from here.
> +	 * Just wait for all in-transit packets to be accounted for
> +	 * before we return from here.
>  	 */
> +	storvsc_wait_to_drain(stor_device);
> 
>  	return SUCCESS;
>  }
> --
> 1.7.4.1

James,

This patch is critical for running Linux based workloads on our Cloud infrastructure - Azure.
Please let me know if there are any issues.

Regards,

K. Y

Re: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the RESET path

[James Bottomley]

On Mon, 2012-10-08 at 15:51 +0000, KY Srinivasan wrote:
> 
> > -----Original Message-----
> > From: K. Y. Srinivasan [mailto:kys@microsoft.com]
> > Sent: Tuesday, October 02, 2012 2:04 PM
> > To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> > hch@infradead.org; linux-scsi@vger.kernel.org
> > Cc: KY Srinivasan; stable@vger.kernel.org
> > Subject: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the
> > RESET path
> > 
> > Properly account for I/O in transit before returning from the RESET call.
> > In the absense of this patch, we could have a situation where the host may
> > respond to a command that was issued prior to the issuance of the RESET
> > command at some arbitrary time after responding to the RESET command.
> > Currently, the host does not do anything with the RESET command.
> > 
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/scsi/storvsc_drv.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 528d52b..0144078 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -1221,7 +1221,12 @@ static int storvsc_host_reset_handler(struct scsi_cmnd
> > *scmnd)
> >  	/*
> >  	 * At this point, all outstanding requests in the adapter
> >  	 * should have been flushed out and return to us
> > +	 * There is a potential race here where the host may be in
> > +	 * the process of responding when we return from here.
> > +	 * Just wait for all in-transit packets to be accounted for
> > +	 * before we return from here.
> >  	 */
> > +	storvsc_wait_to_drain(stor_device);
> > 
> >  	return SUCCESS;
> >  }
> > --
> > 1.7.4.1
> 
> James,
> 
> This patch is critical for running Linux based workloads on our Cloud infrastructure - Azure.
> Please let me know if there are any issues.

So just for next time: it's a bit hard to work out this is a critical
issue from the change log.  If I had to guess, I'd say the response to a
command killed by reset causes some type of use after free and a
potential oops (all of which would have been very nice in the change
log)?

James
        

RE: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the RESET path

[KY Srinivasan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3151 bytes --]



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Tuesday, October 09, 2012 7:38 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; hch@infradead.org; linux-
> scsi@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in
> the RESET path
> 
> On Mon, 2012-10-08 at 15:51 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: K. Y. Srinivasan [mailto:kys@microsoft.com]
> > > Sent: Tuesday, October 02, 2012 2:04 PM
> > > To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com;
> > > hch@infradead.org; linux-scsi@vger.kernel.org
> > > Cc: KY Srinivasan; stable@vger.kernel.org
> > > Subject: [PATCH 1/1] Drivers: scsi: storvsc: Account for in-transit packets in
> the
> > > RESET path
> > >
> > > Properly account for I/O in transit before returning from the RESET call.
> > > In the absense of this patch, we could have a situation where the host may
> > > respond to a command that was issued prior to the issuance of the RESET
> > > command at some arbitrary time after responding to the RESET command.
> > > Currently, the host does not do anything with the RESET command.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  drivers/scsi/storvsc_drv.c |    5 +++++
> > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > > index 528d52b..0144078 100644
> > > --- a/drivers/scsi/storvsc_drv.c
> > > +++ b/drivers/scsi/storvsc_drv.c
> > > @@ -1221,7 +1221,12 @@ static int storvsc_host_reset_handler(struct
> scsi_cmnd
> > > *scmnd)
> > >  	/*
> > >  	 * At this point, all outstanding requests in the adapter
> > >  	 * should have been flushed out and return to us
> > > +	 * There is a potential race here where the host may be in
> > > +	 * the process of responding when we return from here.
> > > +	 * Just wait for all in-transit packets to be accounted for
> > > +	 * before we return from here.
> > >  	 */
> > > +	storvsc_wait_to_drain(stor_device);
> > >
> > >  	return SUCCESS;
> > >  }
> > > --
> > > 1.7.4.1
> >
> > James,
> >
> > This patch is critical for running Linux based workloads on our Cloud
> infrastructure - Azure.
> > Please let me know if there are any issues.
> 
> So just for next time: it's a bit hard to work out this is a critical
> issue from the change log.  If I had to guess, I'd say the response to a
> command killed by reset causes some type of use after free and a
> potential oops (all of which would have been very nice in the change
> log)?

You guessed right! My apologies, I will add such details in the change log
in the future.

Regards,

K. Y
> 
> James
> 
> 
> 
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥