| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20210916155813.935249339@linuxfoundation.org>
Date: Thu, 16 Sep 2021 17:57:22 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: linux-kernel@...r.kernel.org
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
stable@...r.kernel.org, Adrian Hunter <adrian.hunter@...el.com>,
Stanley Chu <stanley.chu@...iatek.com>,
Can Guo <cang@...eaurora.org>,
Asutosh Das <asutoshd@...eaurora.org>,
Avri Altman <avri.altman@....com>,
Bean Huo <beanhuo@...ron.com>,
Bart Van Assche <bvanassche@....org>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Sasha Levin <sashal@...nel.org>
Subject: [PATCH 5.14 093/432] scsi: ufs: Fix the SCSI abort handler
From: Bart Van Assche <bvanassche@....org>
[ Upstream commit 64180742605f772d511903c9e50262afb13726c6 ]
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.
Link: https://lore.kernel.org/r/20210722033439.26550-15-bvanassche@acm.org
Fixes: 7a7e66c65d41 ("scsi: ufs: Fix a race condition between ufshcd_abort() and eh_work()")
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Stanley Chu <stanley.chu@...iatek.com>
Cc: Can Guo <cang@...eaurora.org>
Cc: Asutosh Das <asutoshd@...eaurora.org>
Cc: Avri Altman <avri.altman@....com>
Reviewed-by: Bean Huo <beanhuo@...ron.com>
Signed-off-by: Bart Van Assche <bvanassche@....org>
Signed-off-by: Martin K. Petersen <martin.petersen@...cle.com>
Signed-off-by: Sasha Levin <sashal@...nel.org>
---
drivers/scsi/ufs/ufshcd.c | 54 ++++++++++++++-------------------------
1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0fe559ddc789..1708d7ced527 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2766,15 +2766,6 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
WARN_ON(ufshcd_is_clkgating_allowed(hba) &&
(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];
WARN_ON(lrbp->cmd);
lrbp->cmd = cmd;
@@ -2973,11 +2964,6 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
req->timeout = msecs_to_jiffies(2 * timeout);
blk_mq_start_request(req);
- if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
- err = -EBUSY;
- goto out;
- }
-
lrbp = &hba->lrb[tag];
WARN_ON(lrbp->cmd);
err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag);
@@ -6984,7 +6970,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
struct ufs_hba *hba;
unsigned long flags;
unsigned int tag;
- int err = 0;
+ int err = FAILED;
struct ufshcd_lrb *lrbp;
u32 reg;
@@ -7001,12 +6987,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
ufshcd_hold(hba, false);
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))) {
dev_err(hba->dev,
"%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n",
__func__, tag, hba->outstanding_reqs, reg);
- goto out;
+ goto release;
}
/* Print Transfer Request of aborted task */
@@ -7035,7 +7021,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
dev_err(hba->dev,
"%s: cmd was completed, but without a notifying intr, tag = %d",
__func__, tag);
- goto cleanup;
+ __ufshcd_transfer_req_compl(hba, 1UL << tag);
+ goto release;
}
/*
@@ -7048,36 +7035,33 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
*/
if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) {
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);
hba->force_reset = true;
ufshcd_schedule_eh_work(hba);
spin_unlock_irqrestore(host->host_lock, flags);
- goto out;
+ goto release;
}
/* Skip task abort in case previous aborts failed and report failure */
- if (lrbp->req_abort_skip)
- err = -EIO;
- else
- err = ufshcd_try_to_abort_task(hba, tag);
+ if (lrbp->req_abort_skip) {
+ dev_err(hba->dev, "%s: skipping abort\n", __func__);
+ ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
+ goto release;
+ }
- if (!err) {
-cleanup:
- __ufshcd_transfer_req_compl(hba, (1UL << tag));
-out:
- err = SUCCESS;
- } else {
+ err = ufshcd_try_to_abort_task(hba, tag);
+ if (err) {
dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs);
err = FAILED;
+ goto release;
}
- /*
- * This ufshcd_release() corresponds to the original scsi cmd that got
- * aborted here (as we won't get any IRQ for it).
- */
+ err = SUCCESS;
+
+release:
+ /* Matches the ufshcd_hold() call at the start of this function. */
ufshcd_release(hba);
return err;
}
--
2.30.2
Powered by blists - more mailing lists