Skip to content

Commit 1db8d08

Browse files
committed
scsi: ufs: core: Fix use-after free in init error and remove paths
JIRA: https://issues.redhat.com/browse/RHEL-84800 CVE: CVE-2025-21739 commit f8fb240 Author: André Draszik <andre.draszik@linaro.org> Date: Fri Jan 24 15:09:00 2025 +0000 scsi: ufs: core: Fix use-after free in init error and remove paths 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: cb77cb5 ("blk-crypto: rename blk_keyslot_manager to blk_crypto_profile") Fixes: d76d9d7 ("scsi: ufs: use devm_blk_ksm_init()") Cc: stable@vger.kernel.org Signed-off-by: André Draszik <andre.draszik@linaro.org> Link: https://lore.kernel.org/r/20250124-ufshcd-fix-v4-1-c5d0144aae59@linaro.org Reviewed-by: Bean Huo <beanhuo@micron.com> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Acked-by: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> Signed-off-by: Jared Kangas <jkangas@redhat.com>
1 parent 4273cca commit 1db8d08

File tree

4 files changed

+30
-32
lines changed

4 files changed

+30
-32
lines changed

drivers/ufs/core/ufshcd.c

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10293,16 +10293,6 @@ int ufshcd_system_thaw(struct device *dev)
1029310293
EXPORT_SYMBOL_GPL(ufshcd_system_thaw);
1029410294
#endif /* CONFIG_PM_SLEEP */
1029510295

10296-
/**
10297-
* ufshcd_dealloc_host - deallocate Host Bus Adapter (HBA)
10298-
* @hba: pointer to Host Bus Adapter (HBA)
10299-
*/
10300-
void ufshcd_dealloc_host(struct ufs_hba *hba)
10301-
{
10302-
scsi_host_put(hba->host);
10303-
}
10304-
EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
10305-
1030610296
/**
1030710297
* ufshcd_set_dma_mask - Set dma mask based on the controller
1030810298
* addressing capability
@@ -10319,12 +10309,26 @@ static int ufshcd_set_dma_mask(struct ufs_hba *hba)
1031910309
return dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(32));
1032010310
}
1032110311

10312+
/**
10313+
* ufshcd_devres_release - devres cleanup handler, invoked during release of
10314+
* hba->dev
10315+
* @host: pointer to SCSI host
10316+
*/
10317+
static void ufshcd_devres_release(void *host)
10318+
{
10319+
scsi_host_put(host);
10320+
}
10321+
1032210322
/**
1032310323
* ufshcd_alloc_host - allocate Host Bus Adapter (HBA)
1032410324
* @dev: pointer to device handle
1032510325
* @hba_handle: driver private handle
1032610326
*
1032710327
* Return: 0 on success, non-zero value on failure.
10328+
*
10329+
* NOTE: There is no corresponding ufshcd_dealloc_host() because this function
10330+
* keeps track of its allocations using devres and deallocates everything on
10331+
* device removal automatically.
1032810332
*/
1032910333
int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
1033010334
{
@@ -10346,6 +10350,13 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
1034610350
err = -ENOMEM;
1034710351
goto out_error;
1034810352
}
10353+
10354+
err = devm_add_action_or_reset(dev, ufshcd_devres_release,
10355+
host);
10356+
if (err)
10357+
return dev_err_probe(dev, err,
10358+
"failed to add ufshcd dealloc action\n");
10359+
1034910360
host->nr_maps = HCTX_TYPE_POLL + 1;
1035010361
hba = shost_priv(host);
1035110362
hba->host = host;

drivers/ufs/host/ufshcd-pci.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
516516
pm_runtime_forbid(&pdev->dev);
517517
pm_runtime_get_noresume(&pdev->dev);
518518
ufshcd_remove(hba);
519-
ufshcd_dealloc_host(hba);
520519
}
521520

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

drivers/ufs/host/ufshcd-pltfrm.c

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -464,21 +464,17 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
464464
struct device *dev = &pdev->dev;
465465

466466
mmio_base = devm_platform_ioremap_resource(pdev, 0);
467-
if (IS_ERR(mmio_base)) {
468-
err = PTR_ERR(mmio_base);
469-
goto out;
470-
}
467+
if (IS_ERR(mmio_base))
468+
return PTR_ERR(mmio_base);
471469

472470
irq = platform_get_irq(pdev, 0);
473-
if (irq < 0) {
474-
err = irq;
475-
goto out;
476-
}
471+
if (irq < 0)
472+
return irq;
477473

478474
err = ufshcd_alloc_host(dev, &hba);
479475
if (err) {
480476
dev_err(dev, "Allocation failed\n");
481-
goto out;
477+
return err;
482478
}
483479

484480
hba->vops = vops;
@@ -487,39 +483,34 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
487483
if (err) {
488484
dev_err(dev, "%s: clock parse failed %d\n",
489485
__func__, err);
490-
goto dealloc_host;
486+
return err;
491487
}
492488
err = ufshcd_parse_regulator_info(hba);
493489
if (err) {
494490
dev_err(dev, "%s: regulator init failed %d\n",
495491
__func__, err);
496-
goto dealloc_host;
492+
return err;
497493
}
498494

499495
ufshcd_init_lanes_per_dir(hba);
500496

501497
err = ufshcd_parse_operating_points(hba);
502498
if (err) {
503499
dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
504-
goto dealloc_host;
500+
return err;
505501
}
506502

507503
err = ufshcd_init(hba, mmio_base, irq);
508504
if (err) {
509505
dev_err_probe(dev, err, "Initialization failed with error %d\n",
510506
err);
511-
goto dealloc_host;
507+
return err;
512508
}
513509

514510
pm_runtime_set_active(dev);
515511
pm_runtime_enable(dev);
516512

517513
return 0;
518-
519-
dealloc_host:
520-
ufshcd_dealloc_host(hba);
521-
out:
522-
return err;
523514
}
524515
EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);
525516

@@ -533,7 +524,6 @@ void ufshcd_pltfrm_remove(struct platform_device *pdev)
533524

534525
pm_runtime_get_sync(&pdev->dev);
535526
ufshcd_remove(hba);
536-
ufshcd_dealloc_host(hba);
537527
pm_runtime_disable(&pdev->dev);
538528
pm_runtime_put_noidle(&pdev->dev);
539529
}

include/ufs/ufshcd.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1297,7 +1297,6 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
12971297
}
12981298

12991299
int ufshcd_alloc_host(struct device *, struct ufs_hba **);
1300-
void ufshcd_dealloc_host(struct ufs_hba *);
13011300
int ufshcd_hba_enable(struct ufs_hba *hba);
13021301
int ufshcd_init(struct ufs_hba *, void __iomem *, unsigned int);
13031302
int ufshcd_link_recovery(struct ufs_hba *hba);

0 commit comments

Comments
 (0)