Re: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Baolu Lu <baolu.lu@linux.intel.com>]

On 6/3/26 22:59, Jason Gunthorpe wrote:
> On Wed, Jun 03, 2026 at 01:28:29PM +0000, Pranjal Shrivastava wrote:
>> On Wed, Jun 03, 2026 at 07:31:38AM +0000, Tian, Kevin wrote:
>>>> From: Pranjal Shrivastava<praan@google.com>
>>>> Sent: Monday, June 1, 2026 10:37 PM
>>>>
>>>> Clear the iommu->priv to NULL while returning an error from probe_device.
>>>>
>>>> Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error
>>>> handling")
>>>> Signed-off-by: Pranjal Shrivastava<praan@google.com>
>>> probably add a note that UAF is theoretical at this point.
>>>
>>> iommu_init_device() calls dev_iommu_free() right after @probe_device()
>>> fails...
>> Ack. This is just to prevent a UAF against future refactors. I saw the
>> intel & amd iommu drivers doing it and felt this is missing from smmuv3
> That is just pointless dead code, the core code immediately frees the
> memory this is NULLing
> 
> static int iommu_init_device(struct device *dev)
> {
> [..]
> err_free:
> 	dev->iommu->iommu_dev = NULL;
> 	dev_iommu_free(dev);
> 	return ret;
> 
> I would remove it from the other drivers not addd it here..

You are right. I ever mistakenly thought that release_device would be
called in the error path, as I noticed the following:

err_release:
         if (ops->release_device)
                 ops->release_device(dev);

That actually is not executed when probe_device() fails, so there is no
UAF issue here. Anyway, it would be better to add a comment line in the
driver to prevent any future misunderstandings.

Thanks,
baolu