Commit 37b72085 authored by André Draszik's avatar André Draszik Committed by Zicheng Qu
Browse files

scsi: ufs: core: Fix use-after free in init error and remove paths

mainline inclusion
from mainline-v6.14-rc2
commit f8fb2403ddebb5eea0033d90d9daae4c88749ada
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IBPCAA
CVE: CVE-2025-21739

Reference: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8fb2403ddebb5eea0033d90d9daae4c88749ada



--------------------------------

devm_blk_crypto_profile_init() registers a cleanup handler to run when
the associated (platform-) device is being released. For UFS, the
crypto private data and pointers are stored as part of the ufs_hba's
data structure 'struct ufs_hba::crypto_profile'. This structure is
allocated as part of the underlying ufshcd and therefore Scsi_host
allocation.

During driver release or during error handling in ufshcd_pltfrm_init(),
this structure is released as part of ufshcd_dealloc_host() before the
(platform-) device associated with the crypto call above is released.
Once this device is released, the crypto cleanup code will run, using
the just-released 'struct ufs_hba::crypto_profile'. This causes a
use-after-free situation:

  Call trace:
   kfree+0x60/0x2d8 (P)
   kvfree+0x44/0x60
   blk_crypto_profile_destroy_callback+0x28/0x70
   devm_action_release+0x1c/0x30
   release_nodes+0x6c/0x108
   devres_release_all+0x98/0x100
   device_unbind_cleanup+0x20/0x70
   really_probe+0x218/0x2d0

In other words, the initialisation code flow is:

  platform-device probe
    ufshcd_pltfrm_init()
      ufshcd_alloc_host()
        scsi_host_alloc()
          allocation of struct ufs_hba
          creation of scsi-host devices
    devm_blk_crypto_profile_init()
      devm registration of cleanup handler using platform-device

and during error handling of ufshcd_pltfrm_init() or during driver
removal:

  ufshcd_dealloc_host()
    scsi_host_put()
      put_device(scsi-host)
        release of struct ufs_hba
  put_device(platform-device)
    crypto cleanup handler

To fix this use-after free, change ufshcd_alloc_host() to register a
devres action to automatically cleanup the underlying SCSI device on
ufshcd destruction, without requiring explicit calls to
ufshcd_dealloc_host(). This way:

    * the crypto profile and all other ufs_hba-owned resources are
      destroyed before SCSI (as they've been registered after)
    * a memleak is plugged in tc-dwc-g210-pci.c remove() as a
      side-effect
    * EXPORT_SYMBOL_GPL(ufshcd_dealloc_host) can be removed fully as
      it's not needed anymore
    * no future drivers using ufshcd_alloc_host() could ever forget
      adding the cleanup

Fixes: cb77cb5a ("blk-crypto: rename blk_keyslot_manager to blk_crypto_profile")
Fixes: d76d9d7d ("scsi: ufs: use devm_blk_ksm_init()")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarAndré Draszik <andre.draszik@linaro.org>
Link: https://lore.kernel.org/r/20250124-ufshcd-fix-v4-1-c5d0144aae59@linaro.org


Reviewed-by: default avatarBean Huo <beanhuo@micron.com>
Reviewed-by: default avatarManivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Acked-by: default avatarEric Biggers <ebiggers@kernel.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Conflicts:
	drivers/ufs/host/ufshcd-pltfrm.c
	include/ufs/ufshcd.h
[For ufshcd-pltfrm.c, the main purpose is to remove the goto
dealloc_host. However, in the current version, some parts that are
intended to be removed do not exist, resulting in conflicts.
For ufshcd.h, it aims to remove ufshcd_dealloc_host only, but in the
mainline, it has more code (ufshcd_enable_irq, ufshcd_disable_irq)
around ufshcd_dealloc_host, but they are not relevant to this patch.]
Signed-off-by: default avatarZicheng Qu <quzicheng@huawei.com>
parent 94d50bef
Loading
Loading
Loading
Loading
+21 −10
Original line number Diff line number Diff line
@@ -10219,16 +10219,6 @@ int ufshcd_system_thaw(struct device *dev)
EXPORT_SYMBOL_GPL(ufshcd_system_thaw);
#endif /* CONFIG_PM_SLEEP  */

/**
 * ufshcd_dealloc_host - deallocate Host Bus Adapter (HBA)
 * @hba: pointer to Host Bus Adapter (HBA)
 */
void ufshcd_dealloc_host(struct ufs_hba *hba)
{
	scsi_host_put(hba->host);
}
EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);

/**
 * ufshcd_set_dma_mask - Set dma mask based on the controller
 *			 addressing capability
@@ -10247,12 +10237,26 @@ static int ufshcd_set_dma_mask(struct ufs_hba *hba)
	return dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(32));
}

/**
 * ufshcd_devres_release - devres cleanup handler, invoked during release of
 *			   hba->dev
 * @host: pointer to SCSI host
 */
static void ufshcd_devres_release(void *host)
{
	scsi_host_put(host);
}

/**
 * ufshcd_alloc_host - allocate Host Bus Adapter (HBA)
 * @dev: pointer to device handle
 * @hba_handle: driver private handle
 *
 * Return: 0 on success, non-zero value on failure.
 *
 * NOTE: There is no corresponding ufshcd_dealloc_host() because this function
 * keeps track of its allocations using devres and deallocates everything on
 * device removal automatically.
 */
int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
{
@@ -10274,6 +10278,13 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
		err = -ENOMEM;
		goto out_error;
	}

	err = devm_add_action_or_reset(dev, ufshcd_devres_release,
				       host);
	if (err)
		return dev_err_probe(dev, err,
				     "failed to add ufshcd dealloc action\n");

	host->nr_maps = HCTX_TYPE_POLL + 1;
	hba = shost_priv(host);
	hba->host = host;
+0 −2
Original line number Diff line number Diff line
@@ -516,7 +516,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
	pm_runtime_forbid(&pdev->dev);
	pm_runtime_get_noresume(&pdev->dev);
	ufshcd_remove(hba);
	ufshcd_dealloc_host(hba);
}

/**
@@ -561,7 +560,6 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
	err = ufshcd_init(hba, mmio_base, pdev->irq);
	if (err) {
		dev_err(&pdev->dev, "Initialization failed\n");
		ufshcd_dealloc_host(hba);
		return err;
	}

+8 −18
Original line number Diff line number Diff line
@@ -339,21 +339,17 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
	struct device *dev = &pdev->dev;

	mmio_base = devm_platform_ioremap_resource(pdev, 0);
	if (IS_ERR(mmio_base)) {
		err = PTR_ERR(mmio_base);
		goto out;
	}
	if (IS_ERR(mmio_base))
		return PTR_ERR(mmio_base);

	irq = platform_get_irq(pdev, 0);
	if (irq < 0) {
		err = irq;
		goto out;
	}
	if (irq < 0)
		return irq;

	err = ufshcd_alloc_host(dev, &hba);
	if (err) {
		dev_err(dev, "Allocation failed\n");
		goto out;
		return err;
	}

	hba->vops = vops;
@@ -362,13 +358,13 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
	if (err) {
		dev_err(dev, "%s: clock parse failed %d\n",
				__func__, err);
		goto dealloc_host;
		return err;
	}
	err = ufshcd_parse_regulator_info(hba);
	if (err) {
		dev_err(dev, "%s: regulator init failed %d\n",
				__func__, err);
		goto dealloc_host;
		return err;
	}

	ufshcd_init_lanes_per_dir(hba);
@@ -377,18 +373,13 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
	if (err) {
		dev_err_probe(dev, err, "Initialization failed with error %d\n",
			      err);
		goto dealloc_host;
		return err;
	}

	pm_runtime_set_active(dev);
	pm_runtime_enable(dev);

	return 0;

dealloc_host:
	ufshcd_dealloc_host(hba);
out:
	return err;
}
EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);

@@ -402,7 +393,6 @@ void ufshcd_pltfrm_remove(struct platform_device *pdev)

	pm_runtime_get_sync(&pdev->dev);
	ufshcd_remove(hba);
	ufshcd_dealloc_host(hba);
	pm_runtime_disable(&pdev->dev);
	pm_runtime_put_noidle(&pdev->dev);
}
+0 −1
Original line number Diff line number Diff line
@@ -1235,7 +1235,6 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
}

int ufshcd_alloc_host(struct device *, struct ufs_hba **);
void ufshcd_dealloc_host(struct ufs_hba *);
int ufshcd_hba_enable(struct ufs_hba *hba);
int ufshcd_init(struct ufs_hba *, void __iomem *, unsigned int);
int ufshcd_link_recovery(struct ufs_hba *hba);