linux-kernel - Re: [PATCH v4] scsi: ufs: fix use-after free in init error and remove paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250130012532.GB66821@sol.localdomain>
Date: Wed, 29 Jan 2025 17:25:32 -0800
From: Eric Biggers <ebiggers@...nel.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Alim Akhtar <alim.akhtar@...sung.com>,
	Avri Altman <avri.altman@....com>,
	Bart Van Assche <bvanassche@....org>,
	"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Peter Griffin <peter.griffin@...aro.org>,
	Krzysztof Kozlowski <krzk@...nel.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Mike Snitzer <snitzer@...hat.com>, Jens Axboe <axboe@...nel.dk>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Satya Tangirala <satyat@...gle.com>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	Will McVicker <willmcvicker@...gle.com>, kernel-team@...roid.com,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH v4] scsi: ufs: fix use-after free in init error and
 remove paths

On Fri, Jan 24, 2025 at 03:09:00PM +0000, André Draszik wrote:
> 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: cb77cb5abe1f ("blk-crypto: rename blk_keyslot_manager to blk_crypto_profile")
> Fixes: d76d9d7d1009 ("scsi: ufs: use devm_blk_ksm_init()")
> Cc: stable@...r.kernel.org
> Signed-off-by: André Draszik <andre.draszik@...aro.org>

Acked-by: Eric Biggers <ebiggers@...nel.org>

- Eric