Messages in this thread | | | Date | Mon, 15 Nov 2021 09:38:15 -0400 | From | Jason Gunthorpe <> | Subject | Re: [PATCH 02/11] driver core: Set DMA ownership during driver bind/unbind |
| |
On Mon, Nov 15, 2021 at 07:59:10AM +0100, Greg Kroah-Hartman wrote: > > @@ -566,6 +567,12 @@ static int really_probe(struct device *dev, struct device_driver *drv) > > goto done; > > } > > > > + if (!drv->suppress_auto_claim_dma_owner) { > > + ret = iommu_device_set_dma_owner(dev, DMA_OWNER_KERNEL, NULL); > > + if (ret) > > + return ret; > > + } > > + > > This feels wrong to be doing it in the driver core, why doesn't the bus > that cares about this handle it instead?
As Christoph said, it is not related to the bus. To elaborate any bus_type that has iommu_ops != NULL needs this check, and it must be done on an individual struct device as the result is sensitive to the iommu_group member of each struct device.
> You just caused all drivers in the kernel today to set and release this > ownership, as none set this flag. Shouldn't it be the other way around?
No - the whole point is to cause every driver to do this test.
iommu_device_set_dma_owner() can fail for any device, if it does then a kernel driver must not be probed. Probing a kernel driver when iommu_device_set_dma_owner() fails will break kernel integrity due to HW limitations.
The drv->suppress_auto_claim_dma_owner disables this restriction because three drivers will deal with DMA ownership on their own.
> You only have problems with 1 driver out of thousands, this feels wrong > to abuse the driver core this way for just that one.
I think you have it backwards. Few drivers out of thousands can take an action that impacts the security of a thousand other drivers.
The key thing is that device A can have a driver with suppress_auto_claim_dma_owner=1 and call iommu_device_set_dma_owner(DMA_OWNER_USER) which will then cause another device B to be unsable in the kernel.
Device B, with a normal driver, must be prevented from having a kernel driver because of what the special driver on device A did.
This behavior is a IOMMU HW limitation that cannot be avoided. The restrictions have always been in the kernel, they were just enforced with a BUG_ON at probe via a bus_notifier instead of a clean failure.
So, I don't know how to block probing of the thousands of drivers without adding a test during probing, do you have an different idea?
Jason
| |