Re: [PATCH v2 02/17] driver core: Add dma_unconfigure callback in bus_type

[Lu Baolu]

On 11/28/21 4:02 PM, Greg Kroah-Hartman wrote:
> On Sun, Nov 28, 2021 at 10:50:36AM +0800, Lu Baolu wrote:
> > The bus_type structure defines dma_configure() callback for bus drivers
> > to configure DMA on the devices. This adds the paired dma_unconfigure()
> > callback and calls it during driver unbinding so that bus drivers can do
> > some cleanup work.
> >
> > One use case for this paired DMA callbacks is for the bus driver to check
> > for DMA ownership conflicts during driver binding, where multiple devices
> > belonging to a same IOMMU group (the minimum granularity of isolation and
> > protection) may be assigned to kernel drivers or user space respectively.
> >
> > Without this change, for example, the vfio driver has to listen to a bus
> > BOUND_DRIVER event and then BUG_ON() in case of dma ownership conflict.
> > This leads to bad user experience since careless driver binding operation
> > may crash the system if the admin overlooks the group restriction. Aside
> > from bad design, this leads to a security problem as a root user, even with
> > lockdown=integrity, can force the kernel to BUG.
> >
> > With this change, the bus driver could check and set the DMA ownership in
> > driver binding process and fail on ownership conflicts. The DMA ownership
> > should be released during driver unbinding.
> >
> > Suggested-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Link: https://lore.kernel.org/linux-iommu/20210922123931.GI327412@xxxxxxxxxx/
> > Link: https://lore.kernel.org/linux-iommu/20210928115751.GK964074@xxxxxxxxxx/
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   include/linux/device/bus.h | 3 +++
> >   drivers/base/dd.c          | 7 ++++++-
> >   2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> > index a039ab809753..ef54a71e5f8f 100644
> > --- a/include/linux/device/bus.h
> > +++ b/include/linux/device/bus.h
> > @@ -59,6 +59,8 @@ struct fwnode_handle;
> >    *		bus supports.
> >    * @dma_configure:	Called to setup DMA configuration on a device on
> >    *			this bus.
> > + * @dma_unconfigure:	Called to cleanup DMA configuration on a device on
> > + *			this bus.
>
> "dma_cleanup()" is a better name for this, don't you think?

I agree with you. dma_cleanup() is more explicit and better here.

> thanks,
>
> greg k-h

Best regards,
baolu