[PATCH v1 0/4] iommu: Fix device lookup lifetime and probe cleanup

weimin xiong posted 4 patches 1 week, 4 days ago
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  8 +++++++-
drivers/iommu/arm/arm-smmu/arm-smmu.c       | 12 +++++++++++-
drivers/iommu/msm_iommu.c                   | 10 ++++++++--
drivers/iommu/vsi-iommu.c                   | 10 +++++++++-
4 files changed, 35 insertions(+), 5 deletions(-)
[PATCH v1 0/4] iommu: Fix device lookup lifetime and probe cleanup
Posted by weimin xiong 1 week, 4 days ago
Fix a few IOMMU driver lifetime and error-path issues found while
auditing fwnode-based device lookup and probe cleanup paths.

The first three patches avoid deriving driver private data after
dropping the device reference returned by bus_find_device_by_fwnode().
They also make the ARM SMMU v2 and VSI probe paths fail cleanly when the
IOMMU lookup fails.

The last patch unwinds msm_iommu_probe() state if sysfs setup or
iommu_device_register() fails.

weimin xiong (4):
  iommu/arm-smmu: Fix fwnode lookup lifetime handling
  iommu/arm-smmu-v3: Fix fwnode lookup lifetime handling
  iommu/vsi: Fix fwnode lookup lifetime handling
  iommu/msm: Clean up probe state on registration failure

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  8 +++++++-
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 12 +++++++++++-
 drivers/iommu/msm_iommu.c                   | 10 ++++++++--
 drivers/iommu/vsi-iommu.c                   | 10 +++++++++-
 4 files changed, 35 insertions(+), 5 deletions(-)

-- 
2.43.0
Re: [PATCH v1 0/4] iommu: Fix device lookup lifetime and probe cleanup
Posted by Pranjal Shrivastava 1 week, 4 days ago
On Tue, Jul 14, 2026 at 02:09:26PM +0800, weimin xiong wrote:
> Fix a few IOMMU driver lifetime and error-path issues found while
> auditing fwnode-based device lookup and probe cleanup paths.
> 
> The first three patches avoid deriving driver private data after
> dropping the device reference returned by bus_find_device_by_fwnode().
> They also make the ARM SMMU v2 and VSI probe paths fail cleanly when the
> IOMMU lookup fails.

I'm not sure if that's really needed? All these drivers are doing is
dropping the "extra" refcount (incremented by calling find_device)  back
to the state *before* the fwnode function call. If you find that this
put_device caused the count to drop to 0, I believe that's the real
problem/bug. These fwnode functions are usually called in probe and the
refcount shouldn't be 0 inside probe.

Could you share your observation / failing logs where this fails? Maybe
something else is wrong with the system?

Thanks,
Praan
Re: [PATCH v1 0/4] iommu: Fix device lookup lifetime and probe cleanup
Posted by Robin Murphy 1 week, 4 days ago
On 14/07/2026 3:06 pm, Pranjal Shrivastava wrote:
> On Tue, Jul 14, 2026 at 02:09:26PM +0800, weimin xiong wrote:
>> Fix a few IOMMU driver lifetime and error-path issues found while
>> auditing fwnode-based device lookup and probe cleanup paths.
>>
>> The first three patches avoid deriving driver private data after
>> dropping the device reference returned by bus_find_device_by_fwnode().
>> They also make the ARM SMMU v2 and VSI probe paths fail cleanly when the
>> IOMMU lookup fails.
> 
> I'm not sure if that's really needed? All these drivers are doing is
> dropping the "extra" refcount (incremented by calling find_device)  back
> to the state *before* the fwnode function call. If you find that this
> put_device caused the count to drop to 0, I believe that's the real
> problem/bug. These fwnode functions are usually called in probe and the
> refcount shouldn't be 0 inside probe.
> 
> Could you share your observation / failing logs where this fails? Maybe
> something else is wrong with the system?

I don't have any trace of the original patches (thanks, Microsoft...) 
but looking on lore, yes these "lifetime" concerns are spurious; it's 
just a particular situation where due to the API, the drivers are taking 
a slightly roundabout route to look up their own valid device instance.

The IOMMU device must already have at least one held reference from way 
back in its device_initialise(), which will not be released unless and 
until device_unregister() is called (which is probably never for a 
non-hotpluggable platform device once it has been successfully created). 
If someone unregistered a platform device while it still had a driver 
bound, or the IOMMU driver could be unbound without unregistering the 
iommu_device through which its ->of_xlate or ->probe_device could be 
called, so many other things would be blowing up already that this would 
still be irrelevant.

Since 17de3f5fdd35 ("iommu: Retire bus ops") these lookups should also 
never return NULL for the same reasons, so do feel free to clean up 
those redundant checks if it helps make things a bit clearer.

Thanks,
Robin.
Re:Re: [PATCH v1 0/4] iommu: Fix device lookup lifetime and probe cleanup
Posted by xiongwm2026 1 week, 3 days ago

Hi Pranjal, Hi Robin,



Thanks a lot for the careful review and for clarifying the
refcount semantics here.


I see I conflated the extra reference taken by
bus_find_device*() with object lifetime. Putting that
reference after the lookup is meant to restore the pre-find
balance; it does not by itself imply that the device is gone
or that later use of driver private data is a use-after-put.
As you both pointed out, a put that reaches zero inside probe
would point at a deeper ownership bug elsewhere, which I have
not demonstrated.


I do not currently have a KASAN report, oops, or other
failing log that shows a concrete breakage for these paths.
Given that, I will not pursue the "lookup lifetime" framing
further and will drop this series as proposed.


Robin: if useful as a follow-up, I can send a small cleanup
that removes the now-redundant NULL checks after
17de3f5fdd35 ("iommu: Retire bus ops"), without claiming a
lifetime fix. Please let me know if you would like that.


Thanks again for the explanation.


Best regards,
Weimin


At 2026-07-14 23:56:50, "Robin Murphy" <robin.murphy@arm.com> wrote:
>On 14/07/2026 3:06 pm, Pranjal Shrivastava wrote:
>> On Tue, Jul 14, 2026 at 02:09:26PM +0800, weimin xiong wrote:
>>> Fix a few IOMMU driver lifetime and error-path issues found while
>>> auditing fwnode-based device lookup and probe cleanup paths.
>>>
>>> The first three patches avoid deriving driver private data after
>>> dropping the device reference returned by bus_find_device_by_fwnode().
>>> They also make the ARM SMMU v2 and VSI probe paths fail cleanly when the
>>> IOMMU lookup fails.
>> 
>> I'm not sure if that's really needed? All these drivers are doing is
>> dropping the "extra" refcount (incremented by calling find_device)  back
>> to the state *before* the fwnode function call. If you find that this
>> put_device caused the count to drop to 0, I believe that's the real
>> problem/bug. These fwnode functions are usually called in probe and the
>> refcount shouldn't be 0 inside probe.
>> 
>> Could you share your observation / failing logs where this fails? Maybe
>> something else is wrong with the system?
>
>I don't have any trace of the original patches (thanks, Microsoft...) 
>but looking on lore, yes these "lifetime" concerns are spurious; it's 
>just a particular situation where due to the API, the drivers are taking 
>a slightly roundabout route to look up their own valid device instance.
>
>The IOMMU device must already have at least one held reference from way 
>back in its device_initialise(), which will not be released unless and 
>until device_unregister() is called (which is probably never for a 
>non-hotpluggable platform device once it has been successfully created). 
>If someone unregistered a platform device while it still had a driver 
>bound, or the IOMMU driver could be unbound without unregistering the 
>iommu_device through which its ->of_xlate or ->probe_device could be 
>called, so many other things would be blowing up already that this would 
>still be irrelevant.
>
>Since 17de3f5fdd35 ("iommu: Retire bus ops") these lookups should also 
>never return NULL for the same reasons, so do feel free to clean up 
>those redundant checks if it helps make things a bit clearer.
>
>Thanks,
>Robin.