[PATCH 1/1] scsi: ufs: remove ucd_rsp_dma_addr and ucd_prdt_dma_addr from ufshcd_lrb

[PATCH 1/1] scsi: ufs: remove ucd_rsp_dma_addr and ucd_prdt_dma_addr from ufshcd_lrb

[ed.tsai]

From: Ed Tsai <ed.tsai@mediatek.com>

The offsets stored in utp_transfer_req_desc are in double words on
hosts without UFSHCD_QUIRK_PRDT_BYTE_GRAN, using them directly to
compute ucd_rsp_dma_addr and ucd_prdt_dma_addr results in incorrect
DMA addresses.

Since these fields are only used for error logging, remove them from
struct ufshcd_lrb and compute directly in ufshcd_print_tr() using
offsetof(struct utp_transfer_cmd_desc, ...) instead.

Fixes: d5130c5a0932 ("scsi: ufs: Use pre-calculated offsets in ufshcd_init_lrb()")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260424063603.382328-2-ed.tsai@mediatek.com/
Signed-off-by: Ed Tsai <ed.tsai@mediatek.com>
---
 drivers/ufs/core/ufshcd.c | 10 ++++------
 include/ufs/ufshcd.h      |  4 ----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4805e40ed4d7..02fa61322e77 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -621,7 +621,8 @@ static void ufshcd_print_tr(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	ufshcd_hex_dump("UPIU REQ: ", lrbp->ucd_req_ptr,
 			sizeof(struct utp_upiu_req));
 	dev_err(hba->dev, "UPIU[%d] - Response UPIU phys@0x%llx\n", tag,
-		(u64)lrbp->ucd_rsp_dma_addr);
+		(u64)(lrbp->ucd_req_dma_addr +
+		offsetof(struct utp_transfer_cmd_desc, response_upiu)));
 	ufshcd_hex_dump("UPIU RSP: ", lrbp->ucd_rsp_ptr,
 			sizeof(struct utp_upiu_rsp));
 
@@ -633,7 +634,8 @@ static void ufshcd_print_tr(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	dev_err(hba->dev,
 		"UPIU[%d] - PRDT - %d entries  phys@0x%llx\n",
 		tag, prdt_length,
-		(u64)lrbp->ucd_prdt_dma_addr);
+		(u64)(lrbp->ucd_req_dma_addr +
+		offsetof(struct utp_transfer_cmd_desc, prd_table)));
 
 	if (pr_prdt)
 		ufshcd_hex_dump("UPIU PRDT: ", lrbp->ucd_prdt_ptr,
@@ -2971,8 +2973,6 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
 	dma_addr_t cmd_desc_element_addr =
 		hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
-	u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);
-	u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
 	struct ufshcd_lrb *lrb = scsi_cmd_priv(cmd);
 
 	lrb->utr_descriptor_ptr = utrdlp + i;
@@ -2981,9 +2981,7 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 	lrb->ucd_req_ptr = (struct utp_upiu_req *)cmd_descp->command_upiu;
 	lrb->ucd_req_dma_addr = cmd_desc_element_addr;
 	lrb->ucd_rsp_ptr = (struct utp_upiu_rsp *)cmd_descp->response_upiu;
-	lrb->ucd_rsp_dma_addr = cmd_desc_element_addr + response_offset;
 	lrb->ucd_prdt_ptr = (struct ufshcd_sg_entry *)cmd_descp->prd_table;
-	lrb->ucd_prdt_dma_addr = cmd_desc_element_addr + prdt_offset;
 }
 
 static void __ufshcd_setup_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index cfbc75d8df83..8cb845534e63 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -158,8 +158,6 @@ struct ufs_pm_lvl_states {
  * @ucd_rsp_ptr: Response UPIU address for this command
  * @ucd_prdt_ptr: PRDT address of the command
  * @utrd_dma_addr: UTRD dma address for debug
- * @ucd_prdt_dma_addr: PRDT dma address for debug
- * @ucd_rsp_dma_addr: UPIU response dma address for debug
  * @ucd_req_dma_addr: UPIU request dma address for debug
  * @scsi_status: SCSI status of the command
  * @command_type: SCSI, UFS, Query.
@@ -182,8 +180,6 @@ struct ufshcd_lrb {
 
 	dma_addr_t utrd_dma_addr;
 	dma_addr_t ucd_req_dma_addr;
-	dma_addr_t ucd_rsp_dma_addr;
-	dma_addr_t ucd_prdt_dma_addr;
 
 	int scsi_status;
 
-- 
2.45.2

Re: [PATCH 1/1] scsi: ufs: remove ucd_rsp_dma_addr and ucd_prdt_dma_addr from ufshcd_lrb

[Peter Wang (王信友)]

On Mon, 2026-04-27 at 11:58 +0800, ed.tsai@mediatek.com wrote:
> The offsets stored in utp_transfer_req_desc are in double words on
> hosts without UFSHCD_QUIRK_PRDT_BYTE_GRAN, using them directly to
> compute ucd_rsp_dma_addr and ucd_prdt_dma_addr results in incorrect
> DMA addresses.
> 
> Since these fields are only used for error logging, remove them from
> struct ufshcd_lrb and compute directly in ufshcd_print_tr() using
> offsetof(struct utp_transfer_cmd_desc, ...) instead.
> 
> Fixes: d5130c5a0932 ("scsi: ufs: Use pre-calculated offsets in
> ufshcd_init_lrb()")
> Cc: stable@vger.kernel.org
> Link:
> https://lore.kernel.org/all/20260424063603.382328-2-ed.tsai@mediatek.com/
> Signed-off-by: Ed Tsai <ed.tsai@mediatek.com>
> ---

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 1/1] scsi: ufs: remove ucd_rsp_dma_addr and ucd_prdt_dma_addr from ufshcd_lrb

[Bart Van Assche]

On 4/26/26 8:58 PM, ed.tsai@mediatek.com wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 4805e40ed4d7..02fa61322e77 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -621,7 +621,8 @@ static void ufshcd_print_tr(struct ufs_hba *hba, struct scsi_cmnd *cmd,
>   	ufshcd_hex_dump("UPIU REQ: ", lrbp->ucd_req_ptr,
>   			sizeof(struct utp_upiu_req));
>   	dev_err(hba->dev, "UPIU[%d] - Response UPIU phys@0x%llx\n", tag,
> -		(u64)lrbp->ucd_rsp_dma_addr);
> +		(u64)(lrbp->ucd_req_dma_addr +
> +		offsetof(struct utp_transfer_cmd_desc, response_upiu)));
>   	ufshcd_hex_dump("UPIU RSP: ", lrbp->ucd_rsp_ptr,
>   			sizeof(struct utp_upiu_rsp));
>   
> @@ -633,7 +634,8 @@ static void ufshcd_print_tr(struct ufs_hba *hba, struct scsi_cmnd *cmd,
>   	dev_err(hba->dev,
>   		"UPIU[%d] - PRDT - %d entries  phys@0x%llx\n",
>   		tag, prdt_length,
> -		(u64)lrbp->ucd_prdt_dma_addr);
> +		(u64)(lrbp->ucd_req_dma_addr +
> +		offsetof(struct utp_transfer_cmd_desc, prd_table)));

I don't think that it is useful to log DMA addresses and I prefer that 
this information would not be logged at all. Logging this information
might even involve a security risk. Here is some information about this
topic that comes from an LLM:
------------------------------------------------------------------------
Why is logging pointer addresses from kernel code considered a security 
risk?

Exposing kernel pointer addresses—a practice often referred to as 
pointer leaking—is considered a major security risk because it bypasses 
a fundamental defense mechanism called KASLR (Kernel Address Space 
Layout Randomization).
[ ... ]
2. Facilitating Exploit Chains

A pointer leak is rarely an exploit on its own, but it is almost always 
the first step in a sophisticated attack.

* Return-Oriented Programming (ROP): To hijack execution flow, an 
attacker needs "gadgets" (small snippets of existing code). Without 
knowing the exact addresses of these gadgets, their exploit will simply 
crash the system (a Denial of Service).

* Targeted Corruption: If an attacker wants to overwrite a specific 
security structure (like a process's UID to gain root access), they need 
the pointer to that specific object in kernel memory.
[ ... ]
------------------------------------------------------------------------

Thanks,

Bart.