[PATCH v2] iommufd: Fail replace if device has not been attached

[PATCH v2] iommufd: Fail replace if device has not been attached

[Yi Liu]

The current implementation of iommufd_device_do_replace() implicitly
assumes that the input device has already been attached. However, there
is no explicit check to verify this assumption. If another device within
the same group has been attached, the replace operation might succeed,
but the input device itself may not have been attached yet.

As a result, the input device might not be tracked in the
igroup->device_list, and its reserved IOVA might not be added. Despite
this, the caller might incorrectly assume that the device has been
successfully replaced, which could lead to unexpected behavior or errors.

To address this issue, add a check to ensure that the input device has
been attached before proceeding with the replace operation. This check
will help maintain the integrity of the device tracking system and prevent
potential issues arising from incorrect assumptions about the device's
attachment status.

Fixes: e88d4ec154a8 ("iommufd: Add iommufd_device_replace()")
Cc: stable@vger.kernel.org
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
Change log:
v2:
  - Add r-b tag (Kevin)
  - Minor tweaks. I swarpped the order of is_attach check with the
    if (igroup->hwpt == NULL) check, hence no need to add WARN_ON.

v1: https://lore.kernel.org/linux-iommu/20250304120754.12450-1-yi.l.liu@intel.com/
---
 drivers/iommu/iommufd/device.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index b2f0cb909e6d..bd50146e2ad0 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -471,6 +471,17 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
 
 /* The device attach/detach/replace helpers for attach_handle */
 
+/* Check if idev is attached to igroup->hwpt */
+static bool iommufd_device_is_attached(struct iommufd_device *idev)
+{
+	struct iommufd_device *cur;
+
+	list_for_each_entry(cur, &idev->igroup->device_list, group_item)
+		if (cur == idev)
+			return true;
+	return false;
+}
+
 static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
 				      struct iommufd_device *idev)
 {
@@ -710,6 +721,11 @@ iommufd_device_do_replace(struct iommufd_device *idev,
 		goto err_unlock;
 	}
 
+	if (!iommufd_device_is_attached(idev)) {
+		rc = -EINVAL;
+		goto err_unlock;
+	}
+
 	if (hwpt == igroup->hwpt) {
 		mutex_unlock(&idev->igroup->lock);
 		return NULL;
-- 
2.34.1

Re: [PATCH v2] iommufd: Fail replace if device has not been attached

[Jason Gunthorpe]

On Wed, Mar 05, 2025 at 07:48:42PM -0800, Yi Liu wrote:
> The current implementation of iommufd_device_do_replace() implicitly
> assumes that the input device has already been attached. However, there
> is no explicit check to verify this assumption. If another device within
> the same group has been attached, the replace operation might succeed,
> but the input device itself may not have been attached yet.
> 
> As a result, the input device might not be tracked in the
> igroup->device_list, and its reserved IOVA might not be added. Despite
> this, the caller might incorrectly assume that the device has been
> successfully replaced, which could lead to unexpected behavior or errors.
> 
> To address this issue, add a check to ensure that the input device has
> been attached before proceeding with the replace operation. This check
> will help maintain the integrity of the device tracking system and prevent
> potential issues arising from incorrect assumptions about the device's
> attachment status.
> 
> Fixes: e88d4ec154a8 ("iommufd: Add iommufd_device_replace()")
> Cc: stable@vger.kernel.org
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> Change log:
> v2:
>   - Add r-b tag (Kevin)
>   - Minor tweaks. I swarpped the order of is_attach check with the
>     if (igroup->hwpt == NULL) check, hence no need to add WARN_ON.
> 
> v1: https://lore.kernel.org/linux-iommu/20250304120754.12450-1-yi.l.liu@intel.com/
> ---
>  drivers/iommu/iommufd/device.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Applied, I don't think I will do a -rc pull this cycle just for this
one patch, it does not seem critical but if you think otherwise let
me know

Jason