Re: [PATCHv2 2/2] scsi: set timed out out mq requests to complete
From: Bart Van Assche <hidden>
Date: 2018-07-24 22:46:17
Also in:
linux-nvme
On 07/23/18 07:37, Keith Busch wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 8932ae81a15a..2715cdaa669c 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c@@ -296,6 +296,20 @@ enum blk_eh_timer_return scsi_times_out(struct request *req) rtn = host->hostt->eh_timed_out(scmd); if (rtn == BLK_EH_DONE) { + /* + * For blk-mq, we must set the request state to complete now + * before sending the request to the scsi error handler. This + * will prevent a use-after-free in the event the LLD manages + * to complete the request before the error handler finishes + * processing this timed out request. + * + * If the request was already completed, then the LLD beat the + * time out handler from transferring the request to the scsi + * error handler. In that case we can return immediately as no + * further action is required. + */ + if (req->q->mq_ops && !blk_mq_mark_complete(req)) + return rtn; if (scsi_abort_command(scmd) != SUCCESS) { set_host_byte(scmd, DID_TIME_OUT); scsi_eh_scmd_add(scmd);
This change looks incomplete to me. I think the following scenario is not handled properly by the above patch: - host->hostt->eh_timed_out() gets called. - The request "req" completes from another context while host->hostt->eh_timed_out() is in progress. - host->hostt->eh_timed_out() calls scsi_finish_command(). I think that scenario will lead to a double completion. Bart.