Re: [PATCH 1/2] iommu: free dev->iommu when iommu_fwspec_init() cannot allocate the fwspec

[Robin Murphy <robin.murphy@arm.com>]

On 31/10/2026 8:33 pm, Tal Zussman wrote:
> iommu_fwspec_init() allocates dev->iommu with dev_iommu_get() and then
> the fwspec. If the fwspec allocation fails it returns -ENOMEM but leaves
> dev->iommu allocated, and it leaks.
> 
> kmemleak reports it when iommufd_fail_nth injects the allocation
> failure:
> 
>    unreferenced object 0xffff88811c138b00 (size 192):
>      comm "iommufd_fail_nt", pid 783
>      backtrace:
>        __kmalloc_cache_noprof
>        dev_iommu_get
>        iommu_fwspec_init
>        iommu_mock_device_add
>        iommufd_test
>        iommufd_fops_ioctl
> 
> Free dev->iommu on failure if this call allocated it. iommu_init_device()
> allocates it before calling into the driver and frees it on its own
> error path, so leave it alone in that case.
> 
> Fixes: 72acd9df18f1 ("iommu: Move iommu_fwspec to struct dev_iommu")

No, the bug is clearly the early return in 2a918911ed3d ("iommufd: 
Register iommufd mock devices with fwspec"), so fix it there please. 
iommu_fwspec_init() has always been designed to be called in conjunction 
with iommu_probe_device() which handles cleanup appropriately. All of 
this mock_device hackery not using the proper probe paths is enough of a 
pain as it is...

Thanks,
Robin.

> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>   drivers/iommu/iommu.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index cd1bca7ede9a..69be8819e05d 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3074,6 +3074,7 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>   {
>   	const struct iommu_device *iommu = iommu_from_fwnode(iommu_fwnode);
>   	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +	bool new_iommu = !dev->iommu;
>   
>   	if (!iommu)
>   		return driver_deferred_probe_check_state(dev);
> @@ -3088,8 +3089,11 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode)
>   
>   	/* Preallocate for the overwhelmingly common case of 1 ID */
>   	fwspec = kzalloc_flex(*fwspec, ids, 1);
> -	if (!fwspec)
> +	if (!fwspec) {
> +		if (new_iommu)
> +			dev_iommu_free(dev);
>   		return -ENOMEM;
> +	}
>   
>   	fwnode_handle_get(iommu_fwnode);
>   	fwspec->iommu_fwnode = iommu_fwnode;
>