[PATCH v1] iommu/hyperv: Don't free a paging domain the hypervisor did not delete

[PATCH v1] iommu/hyperv: Don't free a paging domain the hypervisor did not delete

[Yuho Choi]

hv_iommu_paging_domain_free() frees all page-table pages with
pt_iommu_deinit() and only then asks the hypervisor to delete the
device domain that points at them. If HVCALL_DELETE_DEVICE_DOMAIN
fails, hv_delete_device_domain() only logs the error, returns the
domain ID to the IDA, and the domain is freed anyway.

The hypervisor then keeps a device domain whose page tables have been
freed and can be reused for anything. The ID is also handed out again
by the next paging domain allocation, while the hypervisor still has a
domain with that ID.

Delete the domain before freeing its page tables, and keep the ID, the
page tables and the domain if the hypervisor fails to delete it.

Fixes: 2690d4156b51 ("iommu/hyperv: Add para-virtualized IOMMU support for Hyper-V guest")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
Compile-tested only (x86_64 with HYPERV and HYPERV_PVIOMMU,
W=1); I have no Hyper-V guest with the para-virtualized IOMMU to test
it on.

 drivers/iommu/hyperv/hv-iommu-guest.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/hyperv/hv-iommu-guest.c b/drivers/iommu/hyperv/hv-iommu-guest.c
index 876a22f21f1e..205c642c8d40 100644
--- a/drivers/iommu/hyperv/hv-iommu-guest.c
+++ b/drivers/iommu/hyperv/hv-iommu-guest.c
@@ -107,7 +107,7 @@ static int hv_create_device_domain(struct hv_iommu_domain *hv_domain)
 	return hv_result_to_errno(status);
 }
 
-static void hv_delete_device_domain(struct hv_iommu_domain *hv_domain)
+static int hv_delete_device_domain(struct hv_iommu_domain *hv_domain)
 {
 	u64 status;
 	unsigned long flags;
@@ -122,11 +122,15 @@ static void hv_delete_device_domain(struct hv_iommu_domain *hv_domain)
 
 	local_irq_restore(flags);
 
-	if (!hv_result_success(status))
+	if (!hv_result_success(status)) {
+		/* The domain still exists, so its ID cannot be reused */
 		hv_status_err(status, "HVCALL_DELETE_DEVICE_DOMAIN failed\n");
+		return hv_result_to_errno(status);
+	}
 
 	ida_free(&hv_domain->hv_iommu->domain_ids,
 		 hv_domain->device_domain.domain_id.id);
+	return 0;
 }
 
 static int
@@ -466,11 +470,16 @@ static void hv_iommu_paging_domain_free(struct iommu_domain *domain)
 {
 	struct hv_iommu_domain *hv_domain = to_hv_iommu_domain(domain);
 
+	/*
+	 * If the hypervisor still has the domain, it may still use its page
+	 * tables, so leak them.
+	 */
+	if (hv_delete_device_domain(hv_domain))
+		return;
+
 	/* Free all remaining mappings */
 	pt_iommu_deinit(&hv_domain->pt_iommu);
 
-	hv_delete_device_domain(hv_domain);
-
 	kfree(hv_domain);
 }
 

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0

Re: [PATCH v1] iommu/hyperv: Don't free a paging domain the hypervisor did not delete

[Easwar Hariharan]

On 11/5/2026 11:27, Yuho Choi wrote:
> hv_iommu_paging_domain_free() frees all page-table pages with
> pt_iommu_deinit() and only then asks the hypervisor to delete the
> device domain that points at them. If HVCALL_DELETE_DEVICE_DOMAIN
> fails, hv_delete_device_domain() only logs the error, returns the
> domain ID to the IDA, and the domain is freed anyway.
> 
> The hypervisor then keeps a device domain whose page tables have been
> freed and can be reused for anything. The ID is also handed out again
> by the next paging domain allocation, while the hypervisor still has a
> domain with that ID.
> 
> Delete the domain before freeing its page tables, and keep the ID, the
> page tables and the domain if the hypervisor fails to delete it.
> 
> Fixes: 2690d4156b51 ("iommu/hyperv: Add para-virtualized IOMMU support for Hyper-V guest")
> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
> ---
> Compile-tested only (x86_64 with HYPERV and HYPERV_PVIOMMU,
> W=1); I have no Hyper-V guest with the para-virtualized IOMMU to test
> it on.
> 
>  drivers/iommu/hyperv/hv-iommu-guest.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
Good catch!

Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>

Re: [PATCH v1] iommu/hyperv: Don't free a paging domain the hypervisor did not delete

[Jason Gunthorpe]

On Thu, Nov 05, 2026 at 02:27:08PM -0500, Yuho Choi wrote:
> hv_iommu_paging_domain_free() frees all page-table pages with
> pt_iommu_deinit() and only then asks the hypervisor to delete the
> device domain that points at them. If HVCALL_DELETE_DEVICE_DOMAIN
> fails, hv_delete_device_domain() only logs the error, returns the
> domain ID to the IDA, and the domain is freed anyway.

Isn't there a WARN_ON on this path? I recall asking for one ..

So long as that is there this is the right way to continue

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Re: [PATCH v1] iommu/hyperv: Don't free a paging domain the hypervisor did not delete

[Jacob Pan]

Hi Yuho,

On Thu,  5 Nov 2026 14:27:08 -0500
Yuho Choi <oss.patchbox@gmail.com> wrote:

> hv_iommu_paging_domain_free() frees all page-table pages with
> pt_iommu_deinit() and only then asks the hypervisor to delete the
> device domain that points at them. If HVCALL_DELETE_DEVICE_DOMAIN
> fails, hv_delete_device_domain() only logs the error, returns the
> domain ID to the IDA, and the domain is freed anyway.
> 
> The hypervisor then keeps a device domain whose page tables have been
> freed and can be reused for anything. The ID is also handed out again
> by the next paging domain allocation, while the hypervisor still has a
> domain with that ID.
> 
> Delete the domain before freeing its page tables, and keep the ID, the
> page tables and the domain if the hypervisor fails to delete it.
> 
> Fixes: 2690d4156b51 ("iommu/hyperv: Add para-virtualized IOMMU
> support for Hyper-V guest") Signed-off-by: Yuho Choi
> <oss.patchbox@gmail.com> ---
> Compile-tested only (x86_64 with HYPERV and HYPERV_PVIOMMU,
> W=1); I have no Hyper-V guest with the para-virtualized IOMMU to test
> it on.
> 
>  drivers/iommu/hyperv/hv-iommu-guest.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/hyperv/hv-iommu-guest.c
> b/drivers/iommu/hyperv/hv-iommu-guest.c index
> 876a22f21f1e..205c642c8d40 100644 ---
> a/drivers/iommu/hyperv/hv-iommu-guest.c +++
> b/drivers/iommu/hyperv/hv-iommu-guest.c @@ -107,7 +107,7 @@ static
> int hv_create_device_domain(struct hv_iommu_domain *hv_domain) return
> hv_result_to_errno(status); }
>  
> -static void hv_delete_device_domain(struct hv_iommu_domain
> *hv_domain) +static int hv_delete_device_domain(struct
> hv_iommu_domain *hv_domain) {
>  	u64 status;
>  	unsigned long flags;
> @@ -122,11 +122,15 @@ static void hv_delete_device_domain(struct
> hv_iommu_domain *hv_domain) 
>  	local_irq_restore(flags);
>  
> -	if (!hv_result_success(status))
> +	if (!hv_result_success(status)) {
> +		/* The domain still exists, so its ID cannot be
> reused */ hv_status_err(status, "HVCALL_DELETE_DEVICE_DOMAIN
> failed\n");
> +		return hv_result_to_errno(status);
> +	}
>  
>  	ida_free(&hv_domain->hv_iommu->domain_ids,
>  		 hv_domain->device_domain.domain_id.id);
> +	return 0;
>  }
>  
>  static int
> @@ -466,11 +470,16 @@ static void hv_iommu_paging_domain_free(struct
> iommu_domain *domain) {
>  	struct hv_iommu_domain *hv_domain =
> to_hv_iommu_domain(domain); 
> +	/*
> +	 * If the hypervisor still has the domain, it may still use
> its page
> +	 * tables, so leak them.
> +	 */
> +	if (hv_delete_device_domain(hv_domain))
> +		return;
> +
>  	/* Free all remaining mappings */
>  	pt_iommu_deinit(&hv_domain->pt_iommu);
>  
> -	hv_delete_device_domain(hv_domain);
> -
>  	kfree(hv_domain);
>  }
Looks good but I think it is incomplete.

if both hv_configure_device_domain and hv_delete_device_domain both
fails in hv_create_configure_device_domain, then unwind code will do
pt_iommu_deinit thus have the same UAF problem.

        ret = hv_create_configure_device_domain(hv_domain, &settings);
        if (ret)
                goto err_pt_deinit;

        return &hv_domain->domain;

err_pt_deinit:
        pt_iommu_deinit(&hv_domain->pt_iommu); 

hv_create_configure_device_domain(struct hv_iommu_domain
*hv_domain, const struct hv_device_domain_settings *settings)
{
        int ret;

        ret = hv_create_device_domain(hv_domain);
        if (ret)
                return ret;

        ret = hv_configure_device_domain(hv_domain, settings);
        if (ret)
                hv_delete_device_domain(hv_domain);//fails here

        return ret;
}

Re: [PATCH v1] iommu/hyperv: Don't free a paging domain the hypervisor did not delete

[Yuho Choi]

On Thu, Nov 05, 2026 at 02:27:08PM -0500, Yuho Choi wrote:
> > hv_iommu_paging_domain_free() frees all page-table pages with
> > pt_iommu_deinit() and only then asks the hypervisor to delete the
> > device domain that points at them. If HVCALL_DELETE_DEVICE_DOMAIN
> > fails, hv_delete_device_domain() only logs the error, returns the
> > domain ID to the IDA, and the domain is freed anyway.
>
> Isn't there a WARN_ON on this path? I recall asking for one ..

There isn't one yet. hv_delete_device_domain() only prints the error
with hv_status_err(). The driver does WARN when a flush fails or when
attaching to the blocking domain fails, but not when a delete fails.

I'll add a WARN for the delete failure in v2.

> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks for the review!

> Looks good but I think it is incomplete.
>
> if both hv_configure_device_domain and hv_delete_device_domain both
> fails in hv_create_configure_device_domain, then unwind code will do
> pt_iommu_deinit thus have the same UAF problem.

You're right, thanks for catching this. If the delete also fails
there, hv_iommu_domain_alloc_paging() still frees the page tables.

In v2, I'll make hv_iommu_domain_alloc_paging() call
hv_create_device_domain() and hv_configure_device_domain() itself, so
it can leak the page tables and the domain when the delete fails, the
same way hv_iommu_paging_domain_free() does.