diff mbox

bnx2fc: Fix check in SCSI completion handler when reqeust has already timed out.

Message ID 20180124160706.32700-1-chad.dupuis@cavium.com (mailing list archive)
State Accepted
Headers show

Commit Message

Dupuis, Chad Jan. 24, 2018, 4:07 p.m. UTC
When a request times out we set the io_req flag BNX2FC_FLAG_IO_COMPL so
that if a subsequent completion comes in on that task ID we will ignore
it.  The issue is that in the check for this flag there is a missing
return so we will continue to process a request which may have already
been returned to the ownership of the SCSI layer.  This can cause
unpredictable results.

Solution is to add in the missing return.

Signed-off-by: Chad Dupuis <chad.dupuis@cavium.com>
---
 drivers/scsi/bnx2fc/bnx2fc_io.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Laurence Oberman Jan. 24, 2018, 4:12 p.m. UTC | #1
On Wed, 2018-01-24 at 08:07 -0800, Chad Dupuis wrote:
> When a request times out we set the io_req flag BNX2FC_FLAG_IO_COMPL
> so
> that if a subsequent completion comes in on that task ID we will
> ignore
> it.  The issue is that in the check for this flag there is a missing
> return so we will continue to process a request which may have
> already
> been returned to the ownership of the SCSI layer.  This can cause
> unpredictable results.
> 
> Solution is to add in the missing return.
> 
> Signed-off-by: Chad Dupuis <chad.dupuis@cavium.com>
> ---
>  drivers/scsi/bnx2fc/bnx2fc_io.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 8e2f767147cb..5a645b8b9af1 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1889,6 +1889,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
>  		/* we will not receive ABTS response for this IO */
>  		BNX2FC_IO_DBG(io_req, "Timer context finished
> processing "
>  			   "this scsi cmd\n");
> +		return;
>  	}
>  
>  	/* Cancel the timeout_work, as we received IO completion */

Hello

This was added after the race showed up for the first issue we fixed.
Its been running stable in our lab and at the customer.

Reviewed-by: Laurence Oberman <loberman@redhat.com>
Tested-by:   Laurence Oberman <loberman@redhat.com>

Thanks
Laurence
Martin K. Petersen Jan. 31, 2018, 2:26 a.m. UTC | #2
Chad,

> When a request times out we set the io_req flag BNX2FC_FLAG_IO_COMPL so
> that if a subsequent completion comes in on that task ID we will ignore
> it.  The issue is that in the check for this flag there is a missing
> return so we will continue to process a request which may have already
> been returned to the ownership of the SCSI layer.  This can cause
> unpredictable results.

Applied to 4.16/scsi-fixes. Thanks!
diff mbox

Patch

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 8e2f767147cb..5a645b8b9af1 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1889,6 +1889,7 @@  void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
 		/* we will not receive ABTS response for this IO */
 		BNX2FC_IO_DBG(io_req, "Timer context finished processing "
 			   "this scsi cmd\n");
+		return;
 	}
 
 	/* Cancel the timeout_work, as we received IO completion */