linux-kernel - Re: [RFC PATCH] driver core: platform: Call iommu_release_device in dma

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAHH62Bqm57DECYakBWCyWbKHQTs9jnvxMBjJsrnEhnumj+VsEQ@mail.gmail.com>
Date: Mon, 2 Sep 2024 11:47:00 +0530
From: Sachin Parekh <sachinparekh@...gle.com>
To: Robin Murphy <robin.murphy@....com>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, will@...nel.org, 
	lokeshvutla@...gle.com, linux-kernel@...r.kernel.org, iommu@...ts.linux.dev
Subject: Re: [RFC PATCH] driver core: platform: Call iommu_release_device in dma_cleanup

On Thu, Aug 29, 2024 at 7:28 PM Robin Murphy <robin.murphy@....com> wrote:
>
> On 2024-08-29 1:05 pm, Sachin Parekh wrote:
> > Installing a kernel module that has an iommu node in its
> > device tree increments corresponding iommu kernel module
> > reference count during driver_register.
> > Removing the same kernel module doesn't decrement the
> > iommu reference count resulting in error while
> > removing the iommu kernel module
> >
> >      $ modprobe arm-smmu-v3
> >      $ modprobe test_module
> >      $ modprobe -r test_module
> >      $ modprobe -r arm-smmu-v3
> >      modprobe: can't unload module arm_smmu_v3: Resource temporarily unavailable
> >
> > Cause:
> >      platform_driver_register
> >          ...
> >          -> platform_dma_configure
> >              ...
> >              -> iommu_probe_device (Increments reference count)
> >
> >      platform_driver_unregister
> >          ...
> >          -> platform_dma_cleanup
> >              ...
> >              -> No corresponding iommu_release_device call
> >
> > Fix:
> >      Call iommu_release_device in platform_dma_cleanup to remove the
> >      iommu from the corresponding kernel module
>
> There is nothing to fix here, the current behaviour is correct and
> expected. It appears the comment in of_iommu_configure() got lost, but
> the equivalent in acpi_iommu_configure_id() is still there - the dodgy
> iommu_probe_device() calls at those points still logically represent the
> same operation which should have happened via iommu_bus_notifier() or
> iommu_device_register(), it's just being replayed later for hacky reasons.
>
> IOMMU groups are supposed to be tied to the lifetime of the struct
> device, and unbinding a driver does not make the device itself go away -
> if the devicetree says it exists, then the IOMMU layer knows that.
> Modular IOMMU drivers are still not really intended to be removable,
> unless all their client devices can actually be removed form the system
> (and/or the IOMMU driver is being tested without any clients present).
>

Thanks Robin for explaining the intended behaviour and the context.

Does this mean that the iommu reference count should not be decremented
when we call platform_driver_unregister ?
In my scenario, platform_driver_register is incrementing the reference
count so I expected platform_driver_unregister to decrement the reference count

Even if the IOMMU drivers are not intended to be removed, the reference count
should be in accordance with the drivers that register the iommu group ?

Thanks,
Sachin