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
