Re: [GIT PULL] SCSI fixes for 7.3-rc6+

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: ufs: Use-After-Free of hba in ufshcd_async_scan()
- [Medium] ufs: core: Memory and PM runtime usage counter leaks on ufshcd_init() error paths
--

commit bd2cdc88010119e8efabde65d87de3311f493dc1
Author: James Bottomley <James.Bottomley@HansenPartnership.com>

    SCSI fixes for 7.3-rc6+
    
    Driver (and tape media changer) only fixes. Three of the fixes are about
    the same size, (ch, ufs decouble CQ and ufs avoid unsafe MMIO) but the
    unsafe MMIO is the least obvious logic change.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 54f4e7d7de02e..ca8329052cbc5 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
[ ... ]
> @@ -9556,6 +9549,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
>  	ret = ufshcd_add_lus(hba);
>  
>  out:
> +	ufshcd_release(hba);
>  	pm_runtime_put_sync(hba->dev);
>  
>  	if (ret)

[Severity: High]
Can this code cause a use-after-free of the hba structure?

If a user unbinds the UFS driver or the module is unloaded immediately after
initialization (before the ufshcd_async_scan background work completes),
ufshcd_remove() executes and tears down the SCSI host without waiting for the
async domain. Devres subsequently drops the final reference to the Scsi_Host,
freeing the hba structure.

When the pending ufshcd_async_scan() then executes, it dereferences the freed
hba pointer to call the newly added ufshcd_release(hba).

[ ... ]
> @@ -11392,6 +11389,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  	ufshcd_pm_qos_init(hba);
>  	return 0;
>  
> +out_release:
> +	ufshcd_release(hba);
>  out_disable:
>  	hba->is_irq_enabled = false;
>  	ufshcd_hba_exit(hba);

[Severity: Medium]
Does this error handling path leak the SCSI host memory, block layer queues,
and the PM runtime usage counter?

Earlier in ufshcd_init(), ufshcd_add_scsi_host() allocates a SCSI host and
its associated non-devm tmf_queue and tmf_tag_set. Shortly after,
pm_runtime_get_sync(dev) increments the PM usage counter to prevent
auto-suspend.

When the initialization error paths (like ufshcd_link_startup or
ufshcd_verify_dev_init failing) jump to the newly added out_release label,
the cleanup bypasses calling scsi_remove_host(), blk_mq_destroy_queue(),
blk_mq_free_tag_set(), and pm_runtime_put_sync().

(While these omissions also existed in the old out_disable path, this patch
actively redirects execution through this defective cleanup path.)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/eab58e05da0087c70f5473874a549572a9ab0230.camel@HansenPartnership.com?part=1