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

[Baolu Lu]

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@xxxxxxxxxx>
> > > > 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@xxxxxxxxxx>
> > > 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