FROMLIST: scsi: ufs: Fix the SCSI abort handler
Make the following changes in ufshcd_abort(): - Return FAILED instead of SUCCESS if the abort handler notices that a SCSI command has already been completed. Returning SUCCESS in this case triggers a use-after-free and may trigger a kernel crash. - Fix the code for aborting SCSI commands submitted to a WLUN. The current approach for aborting SCSI commands that have been submitted to a WLUN and that timed out is as follows: - Report to the SCSI core that the command has completed successfully. Let the block layer free any data buffers associated with the command. - Mark the command as outstanding in 'outstanding_reqs'. - If the block layer tries to reuse the tag associated with the aborted command, busy-wait until the tag is freed. This approach can result in: - Memory corruption if the controller accesses the data buffer after the block layer has freed the associated data buffers. - A race condition if ufshcd_queuecommand() or ufshcd_exec_dev_cmd() checks the bit that corresponds to an aborted command in 'outstanding_reqs' after it has been cleared and before it is reset. - High energy consumption if ufshcd_queuecommand() repeatedly returns SCSI_MLQUEUE_HOST_BUSY. Fix this by reporting to the SCSI error handler that aborting a SCSI command failed if the SCSI command was submitted to a WLUN. Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Stanley Chu <stanley.chu@mediatek.com> Cc: Can Guo <cang@codeaurora.org> Cc: Asutosh Das <asutoshd@codeaurora.org> Cc: Avri Altman <avri.altman@wdc.com> Fixes: 7a7e66c65d41 ("scsi: ufs: Fix a race condition between ufshcd_abort() and eh_work()") Signed-off-by: Bart Van Assche <bvanassche@acm.org> Link: https://lore.kernel.org/linux-scsi/20210701211224.17070-17-bvanassche@acm.org/ Change-Id: Ice5138ece51bf0e00dc0aa5fcd3ac74659b2afc0 BUG: 192807596 Signed-off-by: Bart Van Assche <bvanassche@google.com>
This commit is contained in:
@@ -2732,15 +2732,6 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
|
|||||||
WARN_ON(ufshcd_is_clkgating_allowed(hba) &&
|
WARN_ON(ufshcd_is_clkgating_allowed(hba) &&
|
||||||
(hba->clk_gating.state != CLKS_ON));
|
(hba->clk_gating.state != CLKS_ON));
|
||||||
|
|
||||||
if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
|
|
||||||
if (hba->pm_op_in_progress)
|
|
||||||
set_host_byte(cmd, DID_BAD_TARGET);
|
|
||||||
else
|
|
||||||
err = SCSI_MLQUEUE_HOST_BUSY;
|
|
||||||
ufshcd_release(hba);
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
lrbp = &hba->lrb[tag];
|
lrbp = &hba->lrb[tag];
|
||||||
WARN_ON(lrbp->cmd);
|
WARN_ON(lrbp->cmd);
|
||||||
lrbp->cmd = cmd;
|
lrbp->cmd = cmd;
|
||||||
@@ -2953,11 +2944,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
|
|||||||
req->timeout = msecs_to_jiffies(2 * timeout);
|
req->timeout = msecs_to_jiffies(2 * timeout);
|
||||||
blk_mq_start_request(req);
|
blk_mq_start_request(req);
|
||||||
|
|
||||||
if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
|
|
||||||
err = -EBUSY;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
init_completion(&wait);
|
init_completion(&wait);
|
||||||
lrbp = &hba->lrb[tag];
|
lrbp = &hba->lrb[tag];
|
||||||
WARN_ON(lrbp->cmd);
|
WARN_ON(lrbp->cmd);
|
||||||
@@ -6670,11 +6656,6 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
|
|||||||
tag = req->tag;
|
tag = req->tag;
|
||||||
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
|
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
|
||||||
|
|
||||||
if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
|
|
||||||
err = -EBUSY;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
init_completion(&wait);
|
init_completion(&wait);
|
||||||
lrbp = &hba->lrb[tag];
|
lrbp = &hba->lrb[tag];
|
||||||
WARN_ON(lrbp->cmd);
|
WARN_ON(lrbp->cmd);
|
||||||
@@ -6742,7 +6723,6 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
out:
|
|
||||||
blk_put_request(req);
|
blk_put_request(req);
|
||||||
out_unlock:
|
out_unlock:
|
||||||
up_read(&hba->clk_scaling_lock);
|
up_read(&hba->clk_scaling_lock);
|
||||||
@@ -6972,7 +6952,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
|
|||||||
struct ufs_hba *hba;
|
struct ufs_hba *hba;
|
||||||
unsigned long flags;
|
unsigned long flags;
|
||||||
unsigned int tag;
|
unsigned int tag;
|
||||||
int err = 0;
|
int err = FAILED, res;
|
||||||
struct ufshcd_lrb *lrbp;
|
struct ufshcd_lrb *lrbp;
|
||||||
u32 reg;
|
u32 reg;
|
||||||
|
|
||||||
@@ -6989,12 +6969,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
|
|||||||
|
|
||||||
ufshcd_hold(hba, false);
|
ufshcd_hold(hba, false);
|
||||||
reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
|
reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
|
||||||
/* If command is already aborted/completed, return SUCCESS */
|
/* If command is already aborted/completed, return FAILED. */
|
||||||
if (!(test_bit(tag, &hba->outstanding_reqs))) {
|
if (!(test_bit(tag, &hba->outstanding_reqs))) {
|
||||||
dev_err(hba->dev,
|
dev_err(hba->dev,
|
||||||
"%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n",
|
"%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n",
|
||||||
__func__, tag, hba->outstanding_reqs, reg);
|
__func__, tag, hba->outstanding_reqs, reg);
|
||||||
goto out;
|
goto release;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Print Transfer Request of aborted task */
|
/* Print Transfer Request of aborted task */
|
||||||
@@ -7023,7 +7003,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
|
|||||||
dev_err(hba->dev,
|
dev_err(hba->dev,
|
||||||
"%s: cmd was completed, but without a notifying intr, tag = %d",
|
"%s: cmd was completed, but without a notifying intr, tag = %d",
|
||||||
__func__, tag);
|
__func__, tag);
|
||||||
goto cleanup;
|
__ufshcd_transfer_req_compl(hba, 1UL << tag);
|
||||||
|
goto release;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -7036,36 +7017,32 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
|
|||||||
*/
|
*/
|
||||||
if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
|
if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
|
||||||
ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, lrbp->lun);
|
ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, lrbp->lun);
|
||||||
__ufshcd_transfer_req_compl(hba, (1UL << tag));
|
|
||||||
set_bit(tag, &hba->outstanding_reqs);
|
|
||||||
spin_lock_irqsave(host->host_lock, flags);
|
spin_lock_irqsave(host->host_lock, flags);
|
||||||
hba->force_reset = true;
|
hba->force_reset = true;
|
||||||
ufshcd_schedule_eh_work(hba);
|
ufshcd_schedule_eh_work(hba);
|
||||||
spin_unlock_irqrestore(host->host_lock, flags);
|
spin_unlock_irqrestore(host->host_lock, flags);
|
||||||
goto out;
|
goto release;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Skip task abort in case previous aborts failed and report failure */
|
/* Skip task abort in case previous aborts failed and report failure */
|
||||||
if (lrbp->req_abort_skip)
|
if (lrbp->req_abort_skip) {
|
||||||
err = -EIO;
|
dev_err(hba->dev, "%s: skipping abort\n", __func__);
|
||||||
else
|
|
||||||
err = ufshcd_try_to_abort_task(hba, tag);
|
|
||||||
|
|
||||||
if (!err) {
|
|
||||||
cleanup:
|
|
||||||
__ufshcd_transfer_req_compl(hba, (1UL << tag));
|
|
||||||
out:
|
|
||||||
err = SUCCESS;
|
|
||||||
} else {
|
|
||||||
dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
|
|
||||||
ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
|
ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
|
||||||
err = FAILED;
|
goto release;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
res = ufshcd_try_to_abort_task(hba, tag);
|
||||||
* This ufshcd_release() corresponds to the original scsi cmd that got
|
if (res) {
|
||||||
* aborted here (as we won't get any IRQ for it).
|
dev_err(hba->dev, "%s: failed with err %d\n", __func__, res);
|
||||||
*/
|
ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
|
||||||
|
goto release;
|
||||||
|
}
|
||||||
|
|
||||||
|
err = SUCCESS;
|
||||||
|
|
||||||
|
release:
|
||||||
|
/* Matches the ufshcd_hold() call at the start of this function. */
|
||||||
ufshcd_release(hba);
|
ufshcd_release(hba);
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user