Re: [PATCH v3 2/2] scsi: ufs: Add support for the aggregated read query opcode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a dedicated DMA allocation for the device management command descriptor (`devman_ucd_base_addr`) and restricts its Physical Region Descriptor Table (PRDT) size to only 2 entries (`UFSHCD_DEVMAN_SG_ENTRIES`). However, Advanced RPMB requests originating from userspace via the block layer's BSG interface are mapped into scatter-gather lists without enforcing this 2-entry limit. Because userspace can submit highly fragmented I/O vectors (or even a contiguous 4KB buffer that spans across three memory pages), the block layer can generate an `sg_cnt` greater than 2. The driver blindly passes this `sg_cnt` to `ufshcd_sgl_to_prdt()`, which iterates over the scatterlist and writes beyond the bounds of the 2-entry `prd_table`. This results in a silent out-of-bounds write that corrupts adjacent DMA-coherent memory.
--

--- Patch [2]: [PATCH v3 2/2] scsi: ufs: Add support for the aggregated read query opcode ---
commit e3d69e949ea76c3908ee9edb0203a7ddef6ca03f
Author: Hyeoncheol Jeong <hyenc.jeong@samsung.com>

scsi: ufs: Add support for the aggregated read query opcode

UFS 5.0 / JEDEC 220H introduces the AGGREGATED READ query opcode (0x9),
which retrieves an aggregated data packet in a single query request. This
patch adds a dedicated utp_devman_cmd_desc with a 4 KiB response area,
allocated once for the reserved (device management) tag that aggregated
read uses.

> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 1c732601afcea..d59e701851683 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h

[ ... ]

> @@ -1359,6 +1367,18 @@ static inline size_t ufshcd_get_ucd_size(const struct ufs_hba *hba)
>  	return sizeof(struct utp_transfer_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
>  }
>  
> +/*
> + * Two entries should be enough for the largest devman PRDT transfer (4 KiB),
> + * like advanced RPMB.
> + */
> +#define UFSHCD_DEVMAN_SG_ENTRIES	2
> +
> +static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
> +{
> +	return sizeof(struct utp_devman_cmd_desc) +
> +		UFSHCD_DEVMAN_SG_ENTRIES * ufshcd_sg_entry_size(hba);
> +}

[Severity: High]
Does this 2-entry limit expose us to an out-of-bounds write when handling
Advanced RPMB BSG requests?

Advanced RPMB requests coming from userspace (via the BSG interface) can
supply highly fragmented scatter-gather lists. For example, a contiguous 4KB
user buffer could cross page boundaries and span 3 memory pages, producing an
sg_cnt greater than 2.

When ufs_bsg_request() dispatches to ufshcd_advanced_rpmb_req_handler(),
the unvalidated sg_cnt is passed directly to ufshcd_sgl_to_prdt():

ufshcd_advanced_rpmb_req_handler() {
        ...
        if (dir != DMA_NONE && sg_list)
                ufshcd_sgl_to_prdt(hba, lrbp, sg_cnt, sg_list);
        ...
}

ufshcd_sgl_to_prdt() will then iterate over the scatterlist up to sg_cnt:

ufshcd_sgl_to_prdt() {
        ...
        for_each_sg(sg, sg, sg_segments, i) {
                prd_table[i].addr = cpu_to_le64(sg_dma_address(sg));
                prd_table[i].size = cpu_to_le32(sg_dma_len(sg) - 1);
                ...
}

Could this write past the end of the newly introduced 2-entry prd_table array
and corrupt adjacent DMA-coherent memory if a privileged user submits a
request with sg_cnt greater than 2?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724030812epcms2p4eb2c77cb4dcd5fd51ca9c3eaa5ea4bfa@epcms2p4?part=2