[PATCH] iommu/amd: Fix ineffective error check in nested domain allocation

[Hemanth Selam]

amd_iommu_pdom_id_alloc() returns an int: a domain ID on success, or the
negative errno from ida_alloc_range() when the ID space is exhausted or
memory is short.  amd_iommu_alloc_domain_nested() stores that return value
in gdom_info->hdom_id, which is a u32, and only then tests it:

	gdom_info->hdom_id = amd_iommu_pdom_id_alloc();
	if (gdom_info->hdom_id <= 0) {

The assignment discards the sign, so -ENOSPC becomes 0xffffffe4 and the
test never fires.  The nested domain is then set up with a host domain ID
that was never allocated, instead of the allocation failing with -ENOSPC.

Keep the value in an int, test it there, and store it only once it is
known to be valid, which is what the other amd_iommu_pdom_id_alloc()
callers already do.

Fixes: 757d2b1fdf5b ("iommu/amd: Introduce gDomID-to-hDomID Mapping and handle parent domain invalidation")
Signed-off-by: Hemanth Selam <hemanth.selam@xxxxxxxxx>
---
Verified by driving amd_iommu_alloc_domain_nested() directly from a debugfs
hook, since the normal entry point needs iommufd to request a nested HWPT on
real hardware.  No error injection was used: the harness allocates domain IDs
until ida_alloc_range() genuinely runs out, then calls the function with a
zeroed guest DTE, which is what validate_gdte_nested() accepts.

Without this patch:

  NESTEDTEST: first alloc, IDs available        -> success, hdom_id=1 (0x00000001)
  NESTEDTEST: second alloc, same gDomID (reuse) -> success, hdom_id=1 (0x00000001)
  NESTEDTEST: exhausted the ID space after 65534 IDs, allocator returns -28
  NESTEDTEST: third alloc, IDs exhausted        -> success, hdom_id=4294967268 (0xffffffe4)

With this patch:

  NESTEDTEST: first alloc, IDs available        -> success, hdom_id=1 (0x00000001)
  NESTEDTEST: second alloc, same gDomID (reuse) -> success, hdom_id=1 (0x00000001)
  NESTEDTEST: exhausted the ID space after 65534 IDs, allocator returns -28
  NESTEDTEST: third alloc, IDs exhausted        -> error -28

The successful and the reuse paths are unchanged; only the exhausted case
differs, and the result was the same in three runs of each kernel.  Built with
CONFIG_AMD_IOMMU_IOMMUFD=y; sparse and checkpatch are clean.

 drivers/iommu/amd/nested.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/nested.c b/drivers/iommu/amd/nested.c
index 63b53b29e029..f1c7987fc585 100644
--- a/drivers/iommu/amd/nested.c
+++ b/drivers/iommu/amd/nested.c
@@ -96,7 +96,7 @@ struct iommu_domain *
 amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
 			      const struct iommu_user_data *user_data)
 {
-	int ret;
+	int ret, hdom_id;
 	unsigned long irqflags;
 	struct nested_domain *ndom;
 	struct guest_domain_mapping_info *gdom_info;
@@ -161,8 +161,8 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
 	}
 
 	/* The gDomID does not exist. We allocate new hdom_id */
-	gdom_info->hdom_id = amd_iommu_pdom_id_alloc();
-	if (gdom_info->hdom_id <= 0) {
+	hdom_id = amd_iommu_pdom_id_alloc();
+	if (hdom_id <= 0) {
 		__xa_cmpxchg(&aviommu->gdomid_array,
 			     ndom->gdom_id, gdom_info, NULL, GFP_ATOMIC);
 		xa_unlock_irqrestore(&aviommu->gdomid_array, irqflags);
@@ -170,6 +170,7 @@ amd_iommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
 		goto out_err_gdom_info;
 	}
 
+	gdom_info->hdom_id = hdom_id;
 	ndom->gdom_info = gdom_info;
 	refcount_set(&gdom_info->users, 1);
 
-- 
2.43.7