Commit 733ab7e1 authored by David Jeffery's avatar David Jeffery Committed by Martin K. Petersen
Browse files

scsi: fnic: Finish scsi_cmnd before dropping the spinlock 

When aborting a SCSI command through fnic, there is a race with the fnic
interrupt handler which can result in the SCSI command and its request
being completed twice. If the interrupt handler claims the command by
setting CMD_SP to NULL first, the abort handler assumes the interrupt
handler has completed the command and returns SUCCESS, causing the request
for the scsi_cmnd to be re-queued.

But the interrupt handler may not have finished the command yet. After it
drops the spinlock protecting CMD_SP, it does memory cleanup before finally
calling scsi_done() to complete the scsi_cmnd. If the call to scsi_done
occurs after the abort handler finishes and re-queues the request, the
completion of the scsi_cmnd will advance and try to double complete a
request already queued for retry.

This patch fixes the issue by moving scsi_done() and any other use of
scsi_cmnd to before the spinlock is released by the interrupt handler.

Link: https://lore.kernel.org/r/20220311184359.2345319-1-djeffery@redhat.com


Reviewed-by: default avatarLaurence Oberman <loberman@redhat.com>
Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
Signed-off-by: default avatarDavid Jeffery <djeffery@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 69ad4ef8

drivers/scsi/fnic/fnic_scsi.c

+6 −7
Original line number Diff line number Diff line
@@ -986,8 +986,6 @@ static void fnic_fcpio_icmnd_cmpl_handler(struct fnic *fnic, 
	CMD_SP(sc) = NULL;

	CMD_FLAGS(sc) |= FNIC_IO_DONE;



	spin_unlock_irqrestore(io_lock, flags);



	if (hdr_status != FCPIO_SUCCESS) {

		atomic64_inc(&fnic_stats->io_stats.io_failures);

		shost_printk(KERN_ERR, fnic->lport->host, "hdr status = %s\n",
@@ -996,8 +994,6 @@ static void fnic_fcpio_icmnd_cmpl_handler(struct fnic *fnic, 


	fnic_release_ioreq_buf(fnic, io_req, sc);



	mempool_free(io_req, fnic->io_req_pool);



	cmd_trace = ((u64)hdr_status << 56) |

		  (u64)icmnd_cmpl->scsi_status << 48 |

		  (u64)icmnd_cmpl->flags << 40 | (u64)sc->cmnd[0] << 32 |
@@ -1021,6 +1017,12 @@ static void fnic_fcpio_icmnd_cmpl_handler(struct fnic *fnic, 
	} else

		fnic->lport->host_stats.fcp_control_requests++;



	/* Call SCSI completion function to complete the IO */

	scsi_done(sc);

	spin_unlock_irqrestore(io_lock, flags);



	mempool_free(io_req, fnic->io_req_pool);



	atomic64_dec(&fnic_stats->io_stats.active_ios);

	if (atomic64_read(&fnic->io_cmpl_skip))

		atomic64_dec(&fnic->io_cmpl_skip);
@@ -1049,9 +1051,6 @@ static void fnic_fcpio_icmnd_cmpl_handler(struct fnic *fnic, 
		if(io_duration_time > atomic64_read(&fnic_stats->io_stats.current_max_io_time))

			atomic64_set(&fnic_stats->io_stats.current_max_io_time, io_duration_time);

	}



	/* Call SCSI completion function to complete the IO */

	scsi_done(sc);

}



/* fnic_fcpio_itmf_cmpl_handler