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

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

Pre-existing issues:
- [High] Fallback to SDB mode after `ufshcd_alloc_mcq()` failure uses freed or uninitialized command queues.
--

--- Patch [2]: [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode ---
commit 687c47ee8e9a0aab4800ae50a40defa3e567d065
Author: Hyeoncheol Jeong <[email protected]>

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

This commit introduces support for the UFS 5.0 AGGREGATED READ query opcode.
It allocates a dedicated device management command descriptor (UCD) with a
larger response area to handle packets up to 4 KiB, bypassing the standard
shared pool for the reserved tag.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 79331c2bd38de..328320b8119cc 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> @@ -9236,7 +9286,7 @@ static void ufshcd_release_sdb_queue(struct ufs_hba *hba, int nutrs)
>  {
>  	size_t ucdl_size, utrdl_size;
>  
> -	ucdl_size = ufshcd_get_ucd_size(hba) * nutrs;
> +	ucdl_size = ufshcd_get_ucd_size(hba) * (nutrs - UFSHCD_NUM_RESERVED);
>  	dmam_free_coherent(hba->dev, ucdl_size, hba->ucdl_base_addr,
>  			   hba->ucdl_dma_addr);

[Severity: High]
This is a pre-existing issue, but does this fallback path lead to a
use-after-free or NULL pointer dereference?

When ufshcd_alloc_mcq() is called, if the new MCQ tag depth differs from the
SDB tag depth, it frees the existing SDB queues:

ufshcd_add_scsi_host() -> ufshcd_alloc_mcq()
    ufshcd_release_sdb_queue(hba, old_nutrs);

It then attempts to allocate new queues via ufshcd_memory_alloc(). If this
fails, it restores the tag depth and returns an error without reallocating
the SDB memory:

    ret = ufshcd_memory_alloc(hba);
    if (ret)
        goto err;
    ...
err:
    hba->nutrs = old_nutrs;
    return ret;

The caller, ufshcd_add_scsi_host(), catches the error and deliberately falls
back to SDB mode:

    err = ufshcd_alloc_mcq(hba);
    if (err) {
        /* Continue with SDB mode */
        ufshcd_mcq_disable(hba);
        use_mcq_mode = false;

Since the SDB queues were freed and not successfully reallocated, won't any
subsequent command in SDB mode use freed or uninitialized command queues?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728092208epcms2p881b17276fb41c006a4229c1d073b4ad0@epcms2p8?part=2