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] [day] [month] [year] [list]

Message-ID: <CAHH62BpRkkAE_c9KC-D=OXh0j7SME=FSHGN0Y8C7C6naW=NvnQ@mail.gmail.com>
Date: Mon, 2 Sep 2024 12:14:06 +0530
From: Sachin Parekh <sachinparekh@...gle.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: rafael@...nel.org, will@...nel.org, robin.murphy@....com, 
	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:22 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Thu, Aug 29, 2024 at 12:05:04PM +0000, 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
>
> Please wrap kernel changelog text at 72 columns like your editor asked
> you to :)

I had wrapped while creating the patch but I suppose it got messed up
somewhere. Sorry about that, I will fix it next time.

> >     $ 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
>
> Does this happen for any in-kernel driver?
>
> Why has this not been noticed before?
>

As per Robin's explanation, IOMMU drivers are not intended to be removed so
that could be the reason why this hasn't been noticed.

> >
> > 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
> >
> > Signed-off-by: Sachin Parekh <sachinparekh@...gle.com>
>
> What commit id does this fix?
>

I think these two commit IDs are relevant in this context:
512881eacfa72c2136b27b9934b7b27504a9efc2
07397df29e57cde5799af16e8f148ae10ed75285


> > ---
> >  drivers/base/platform.c | 3 +++
> >  drivers/iommu/iommu.c   | 3 +--
> >  include/linux/iommu.h   | 1 +
> >  3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 4c3ee6521ba5..c8125325a5e9 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -1467,6 +1467,9 @@ static void platform_dma_cleanup(struct device *dev)
> >
> >       if (!drv->driver_managed_dma)
> >               iommu_device_unuse_default_domain(dev);
> > +
> > +     if (dev_of_node(dev))
> > +             iommu_release_device(dev);
>
> Are you sure that all devices that pass this should have this call made?
> How well was this tested on different systems?
>

I am not sure hence I created this patch to understand if this is the
correct approach.

Thanks,
Sachin