FAILED: patch "[PATCH] scsi: qla2xxx: Fix double scsi_done for abort path" failed to apply to 5.3-stable tree

PATCH [email protected] 2 participants 3 messages Dec 14, 2019 – Dec 15, 2019

Subsystems (3)

Stable branch SCSI QLogic QLA2XXX FC-SCSI driver

lore

Share & tools

lore

[email protected]

author maintainer

2019-12-14 15:05 UTC #0 lore raw

The patch below does not apply to the 5.3-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From f45bca8c5052e8c59bab64ee90c44441678b9a52 Mon Sep 17 00:00:00 2001
From: Quinn Tran <[email protected]>
Date: Tue, 5 Nov 2019 07:06:54 -0800
Subject: [PATCH] scsi: qla2xxx: Fix double scsi_done for abort path

Current code assumes abort will remove the original command from the active list where scsi_done will not be called. Instead, the eh_abort thread will do the scsi_done. That is not the case. Instead, we have a double scsi_done calls triggering use after free.

Abort will tell FW to release the command from FW possesion. The original command will return to ULP with error in its normal fashion via scsi_done. eh_abort path would wait for the original command completion before returning. eh_abort path will not perform the scsi_done call.

Fixes 219d27d7147e0 ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands")

Cc [email protected] # 5.2

Link https://lore.kernel.org/r/[email protected] open here

Reviewed-by Ewan D. Milne <[email protected]>

Signed-off-by Quinn Tran <[email protected]>

Signed-off-by Arun Easi <[email protected]>

Signed-off-by Himanshu Madhani <[email protected]>

Signed-off-by Martin K. Petersen <[email protected]>

drivers/scsi/qla2xxx/qla_def.h +4 -1

index ef9bb3c7ad6f..2a9e6a9a8c9d 100644
@@ -591,13 +591,16 @@ typedef struct srb {
 	 */
 	uint8_t cmd_type;
 	uint8_t pad[3];
-	atomic_t ref_count;
 	struct kref cmd_kref;	/* need to migrate ref_count over to this */
 	void *priv;
 	wait_queue_head_t nvme_ls_waitq;
 	struct fc_port *fcport;
 	struct scsi_qla_host *vha;
 	unsigned int start_timer:1;
+	unsigned int abort:1;
+	unsigned int aborted:1;
+	unsigned int completed:1;
+
 	uint32_t handle;
 	uint16_t flags;
 	uint16_t type;

drivers/scsi/qla2xxx/qla_isr.c +5 -0

index acef3d73983c..1b8f297449cf 100644
@@ -2487,6 +2487,11 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 		return;
 	}
 
+	if (sp->abort)
+		sp->aborted = 1;
+	else
+		sp->completed = 1;
+
 	if (sp->cmd_type != TYPE_SRB) {
 		req->outstanding_cmds[handle] = NULL;
 		ql_dbg(ql_dbg_io, vha, 0x3015,

drivers/scsi/qla2xxx/qla_nvme.c +2 -2

index 6cc19e060afc..941aa53363f5 100644
@@ -224,8 +224,8 @@ static void qla_nvme_abort_work(struct work_struct *work)
 
 	if (ha->flags.host_shutting_down) {
 		ql_log(ql_log_info, sp->fcport->vha, 0xffff,
-		    "%s Calling done on sp: %p, type: 0x%x, sp->ref_count: 0x%x\n",
-		    __func__, sp, sp->type, atomic_read(&sp->ref_count));
+		    "%s Calling done on sp: %p, type: 0x%x\n",
+		    __func__, sp, sp->type);
 		sp->done(sp, 0);
 		goto out;
 	}

drivers/scsi/qla2xxx/qla_os.c +61 ~~-56~~

228 lines not on this page · raw message

Sasha Levin

maintainer

2019-12-15 17:32 UTC #1 lore raw

Quoted 27 lines · On Sat, Dec 14, 2019 at 04:04:57PM +0100, [email protected] wrote:

The patch below does not apply to the 5.3-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From f45bca8c5052e8c59bab64ee90c44441678b9a52 Mon Sep 17 00:00:00 2001
From: Quinn Tran <[email protected]>
Date: Tue, 5 Nov 2019 07:06:54 -0800
Subject: [PATCH] scsi: qla2xxx: Fix double scsi_done for abort path
Current code assumes abort will remove the original command from the active list where scsi_done will not be called. Instead, the eh_abort thread will do the scsi_done. That is not the case. Instead, we have a double scsi_done calls triggering use after free.
Abort will tell FW to release the command from FW possesion. The original command will return to ULP with error in its normal fashion via scsi_done. eh_abort path would wait for the original command completion before returning. eh_abort path will not perform the scsi_done call.

Quoted 8 lines

Fixes 219d27d7147e0 ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands")
Cc [email protected] # 5.2
Link https://lore.kernel.org/r/[email protected] open here
Reviewed-by Ewan D. Milne <[email protected]>
Signed-off-by Quinn Tran <[email protected]>
Signed-off-by Arun Easi <[email protected]>
Signed-off-by Himanshu Madhani <[email protected]>
Signed-off-by Martin K. Petersen <[email protected]>

I took these two additional patches to resolve the conflict:

85cffefa09e4 ("scsi: qla2xxx: Fix a race condition between aborting and completing a SCSI command") bdb61b9b944d ("scsi: qla2xxx: Introduce the function qla2xxx_init_sp()")

Signature · 2 lines

Thanks,
Sasha

Greg KH

author maintainer

2019-12-15 17:40 UTC #2 lore raw

Quoted 37 lines · Sasha Levin

Quoted 27 lines · On Sat, Dec 14, 2019 at 04:04:57PM +0100, [email protected] wrote:
The patch below does not apply to the 5.3-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
Quoted 1 line
From f45bca8c5052e8c59bab64ee90c44441678b9a52 Mon Sep 17 00:00:00 2001
From: Quinn Tran <[email protected]>
Date: Tue, 5 Nov 2019 07:06:54 -0800
Subject: [PATCH] scsi: qla2xxx: Fix double scsi_done for abort path
Current code assumes abort will remove the original command from the active list where scsi_done will not be called. Instead, the eh_abort thread will do the scsi_done. That is not the case. Instead, we have a double scsi_done calls triggering use after free.
Abort will tell FW to release the command from FW possesion. The original command will return to ULP with error in its normal fashion via scsi_done. eh_abort path would wait for the original command completion before returning. eh_abort path will not perform the scsi_done call.
Quoted 8 lines
Fixes 219d27d7147e0 ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands")
Cc [email protected] # 5.2
Link https://lore.kernel.org/r/[email protected] open here
Reviewed-by Ewan D. Milne <[email protected]>
Signed-off-by Quinn Tran <[email protected]>
Signed-off-by Arun Easi <[email protected]>
Signed-off-by Himanshu Madhani <[email protected]>
Signed-off-by Martin K. Petersen <[email protected]>

Quoted 4 lines

I took these two additional patches to resolve the conflict:
85cffefa09e4 ("scsi: qla2xxx: Fix a race condition between aborting and completing a SCSI command") bdb61b9b944d ("scsi: qla2xxx: Introduce the function qla2xxx_init_sp()")

Thanks for fixing up all of these scsi patches, much appreciated.

greg k-h