Re: [PATCH 2/5] scsi: ufs: Fix NULL dereferences after tag lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If a malicious or malfunctioning UFS device sends a completion queue entry for a task tag that is not currently in-flight, `ufshcd_compl_one_cqe` will process it and release resources, leading to a double DMA unmap and power management underflow.
--

--- Patch [2]: [PATCH 2/5] scsi: ufs: Fix NULL dereferences after tag lookup ---
commit 070044fcdb23bd62a58cf9f22d642b3928421c36
Author: Li Qiang <liqiang01@kylinos.cn>

scsi: ufs: Fix NULL dereferences after tag lookup

This commit fixes a potential NULL pointer dereference in the MCQ cleanup
and completion paths by moving the private-command and request lookups
after the cmd NULL check. It also avoids dereferencing a NULL cqe when
reporting an invalid tag.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 3541da5b6f4de..f2a5fa624e749 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5857,13 +5857,14 @@ void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
>  			  struct cq_entry *cqe)
>  {
>  	struct scsi_cmnd *cmd = ufshcd_tag_to_cmd(hba, task_tag);
> -	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
> +	struct ufshcd_lrb *lrbp;
>  	enum utp_ocs ocs;
>  
> -	if (WARN_ONCE(!cmd, "cqe->command_desc_base_addr = %#llx\n",
> -		      le64_to_cpu(cqe->command_desc_base_addr)))
> +	if (WARN_ONCE(!cmd, "invalid completion tag %d, cqe->command_desc_base_addr = %#llx\n",
> +		      task_tag, cqe ? le64_to_cpu(cqe->command_desc_base_addr) : 0ULL))
>  		return;
>  
> +	lrbp = scsi_cmd_priv(cmd);

[Severity: High]
This is a pre-existing issue, but can spurious completions from the device
cause double DMA unmaps and power management refcount underflows here?

Because ufshcd_tag_to_cmd() returns the memory allocated for the tag in the
block layer tagset (which is non-NULL even for inactive requests), the !cmd
check passes even if the tag isn't currently in-flight.

Later in ufshcd_compl_one_cqe(), ufshcd_release_scsi_cmd(hba, cmd) is called
unconditionally, bypassing the block layer's active request protections. If
a malicious or malfunctioning device sends a CQE for an inactive tag, it looks
like it could trigger a double DMA unmap and permanently break clock gating
by underflowing the PM refcounts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784182493.git.liqiang01@kylinos.cn?part=2