On 2024/8/6 21:45, Niklas Schnelle wrote:
This fixes a crash when surprise hot-unplugging a PCI device. This crash
happens because during hot-unplug __iommu_group_set_domain_nofail()
attaching the default domain fails when the platform no longer
recognizes the device as it has already been removed and we end up with
a NULL domain pointer and UAF. This is exactly the case referred to in
the second comment in __iommu_device_set_domain() and just as stated
there if we can instead attach the blocking domain the UAF is prevented
as this can handle the already removed device. Implement the blocking
domain to use this handling. This would still leave us with a warning
for a failed attach. As failing to attach when the device is no longer
present is expected behavior turn this into an explicit -ENODEV error
and don't warn for it. Also change the error return for a NULL zdev to
-EIO as we don't want to ignore this case that would be a serious bug.
Fixes: c76c067e488c ("s390/pci: Use dma-iommu layer")
Signed-off-by: Niklas Schnelle<schnelle@xxxxxxxxxxxxx>
---
Note: I somewhat suspect this to be related to the following discussion
or at least we have seen the same backtraces in reports that we suspect
to be caused by the issue fixed with this patch. In the case I was able
to reproduce with vfio-pci pass-through to a KVM guest I got a different
trace though.
Organizational note: I'll be on vacation starting Thursday. Matt will
then take over and sent new revisions as necessary.
---
drivers/iommu/iommu.c | 7 ++++--
drivers/iommu/s390-iommu.c | 55 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ed6c5cb60c5a..91b3b23bf55c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -119,8 +119,11 @@ static int __iommu_group_set_domain(struct iommu_group *group,
static void __iommu_group_set_domain_nofail(struct iommu_group *group,
struct iommu_domain *new_domain)
{
- WARN_ON(__iommu_group_set_domain_internal(
- group, new_domain, IOMMU_SET_DOMAIN_MUST_SUCCEED));
+ int ret = __iommu_group_set_domain_internal(
+ group, new_domain, IOMMU_SET_DOMAIN_MUST_SUCCEED);
+
+ /* Allow attach to fail when the device is gone */
+ WARN_ON(ret && ret != -ENODEV);
The return values of attach_dev have been documented in include/linux
/iommu.h:
/**
* struct iommu_domain_ops - domain specific operations
* @attach_dev: attach an iommu domain to a device
* Return:
* * 0 - success
* * EINVAL - can indicate that device and domain are incompatible
due to
* some previous configuration of the domain, in which
case the
* driver shouldn't log an error, since it is legitimate
for a
* caller to test reuse of existing domains. Otherwise,
it may
* still represent some other fundamental problem
* * ENOMEM - out of memory
* * ENOSPC - non-ENOMEM type of resource allocation failures
* * EBUSY - device is attached to a domain and cannot be changed
* * ENODEV - device specific errors, not able to be attached
* * <others> - treated as ENODEV by the caller. Use is discouraged
ENODEV is obviously not suitable for the case of 'device is gone'.
Thanks,
baolu
