Skip to content

Commit

Permalink
scsi: ufs: core: Fix a race condition related to device commands
Browse files Browse the repository at this point in the history
There is a TOCTOU race in ufshcd_compl_one_cqe(): hba->dev_cmd.complete may
be cleared from another thread after it has been checked and before it is
used. Fix this race by moving the device command completion from the stack
of the device command submitter into struct ufs_hba. This patch fixes the
following kernel crash:

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
Call trace:
 _raw_spin_lock_irqsave+0x34/0x80
 complete+0x24/0xb8
 ufshcd_compl_one_cqe+0x13c/0x4f0
 ufshcd_mcq_poll_cqe_lock+0xb4/0x108
 ufshcd_intr+0x2f4/0x444
 __handle_irq_event_percpu+0xbc/0x250
 handle_irq_event+0x48/0xb0

Fixes: 5a0b0cb ("[SCSI] ufs: Add support for sending NOP OUT UPIU")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/20250314225206.1487838-1-bvanassche@acm.org
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
Bart Van Assche authored and Martin K. Petersen committed Mar 21, 2025
1 parent daff37f commit 20b97ac
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 20 deletions.
25 changes: 6 additions & 19 deletions drivers/ufs/core/ufshcd.c
Original file line number Diff line number Diff line change
Expand Up @@ -3176,16 +3176,10 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
int err;

retry:
time_left = wait_for_completion_timeout(hba->dev_cmd.complete,
time_left = wait_for_completion_timeout(&hba->dev_cmd.complete,
time_left);

if (likely(time_left)) {
/*
* The completion handler called complete() and the caller of
* this function still owns the @lrbp tag so the code below does
* not trigger any race conditions.
*/
hba->dev_cmd.complete = NULL;
err = ufshcd_get_tr_ocs(lrbp, NULL);
if (!err)
err = ufshcd_dev_cmd_completion(hba, lrbp);
Expand All @@ -3199,7 +3193,6 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
/* successfully cleared the command, retry if needed */
if (ufshcd_clear_cmd(hba, lrbp->task_tag) == 0)
err = -EAGAIN;
hba->dev_cmd.complete = NULL;
return err;
}

Expand All @@ -3215,11 +3208,9 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
spin_lock_irqsave(&hba->outstanding_lock, flags);
pending = test_bit(lrbp->task_tag,
&hba->outstanding_reqs);
if (pending) {
hba->dev_cmd.complete = NULL;
if (pending)
__clear_bit(lrbp->task_tag,
&hba->outstanding_reqs);
}
spin_unlock_irqrestore(&hba->outstanding_lock, flags);

if (!pending) {
Expand All @@ -3237,8 +3228,6 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
spin_lock_irqsave(&hba->outstanding_lock, flags);
pending = test_bit(lrbp->task_tag,
&hba->outstanding_reqs);
if (pending)
hba->dev_cmd.complete = NULL;
spin_unlock_irqrestore(&hba->outstanding_lock, flags);

if (!pending) {
Expand Down Expand Up @@ -3272,13 +3261,9 @@ static void ufshcd_dev_man_unlock(struct ufs_hba *hba)
static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp,
const u32 tag, int timeout)
{
DECLARE_COMPLETION_ONSTACK(wait);
int err;

hba->dev_cmd.complete = &wait;

ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);

ufshcd_send_command(hba, tag, hba->dev_cmd_queue);
err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);

Expand Down Expand Up @@ -5585,12 +5570,12 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
ufshcd_release_scsi_cmd(hba, lrbp);
/* Do not touch lrbp after scsi done */
scsi_done(cmd);
} else if (hba->dev_cmd.complete) {
} else {
if (cqe) {
ocs = le32_to_cpu(cqe->status) & MASK_OCS;
lrbp->utr_descriptor_ptr->header.ocs = ocs;
}
complete(hba->dev_cmd.complete);
complete(&hba->dev_cmd.complete);
}
}

Expand Down Expand Up @@ -10475,6 +10460,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
*/
spin_lock_init(&hba->clk_gating.lock);

init_completion(&hba->dev_cmd.complete);

err = ufshcd_hba_init(hba);
if (err)
goto out_error;
Expand Down
2 changes: 1 addition & 1 deletion include/ufs/ufshcd.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ struct ufs_query {
struct ufs_dev_cmd {
enum dev_cmd_type type;
struct mutex lock;
struct completion *complete;
struct completion complete;
struct ufs_query query;
};

Expand Down

0 comments on commit 20b97ac

Please sign in to comment.