Remove redundant domid_free() call in the XEN_DOMCTL_createdomain error
handling path to prevent a double-free condition.

When domain_create() fails, it internally calls _domain_destroy() during
its cleanup routine, which already invokes domid_free() to release the
allocated domain ID. The additional domid_free() call in the domctl error
path creates a double-free scenario, triggering an assertion failure in
domid.c:

    Assertion 'rc' failed at common/domid.c:84

The domain creation flow is:
1. domid_alloc() allocates a domain ID
2. domain_create() is called with the allocated ID
3. If domain_create() fails:
   a) domain_create() calls _domain_destroy() internally
   b) _domain_destroy() calls domid_free() to release the ID
   c) domctl incorrectly calls domid_free() again

This double-free violates the domain ID management invariants and causes
system instability. The fix ensures domid_free() is called exactly once
per allocated domain ID, maintaining proper resource cleanup
semantics.

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---

 xen/common/domctl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 71e712c1f3..954d790226 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -421,7 +421,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         d = domain_create(domid, &op->u.createdomain, false);
         if ( IS_ERR(d) )
         {
-            domid_free(domid);
             ret = PTR_ERR(d);
             d = NULL;
             break;
-- 
2.34.1

On Sat, Sep 13, 2025 at 10:44:39AM +0000, Oleksii Moisieiev wrote:
> Remove redundant domid_free() call in the XEN_DOMCTL_createdomain error
> handling path to prevent a double-free condition.
> 
> When domain_create() fails, it internally calls _domain_destroy() during
> its cleanup routine, which already invokes domid_free() to release the
> allocated domain ID. The additional domid_free() call in the domctl error
> path creates a double-free scenario, triggering an assertion failure in
> domid.c:
> 
>     Assertion 'rc' failed at common/domid.c:84
> 
> The domain creation flow is:
> 1. domid_alloc() allocates a domain ID
> 2. domain_create() is called with the allocated ID
> 3. If domain_create() fails:
>    a) domain_create() calls _domain_destroy() internally
>    b) _domain_destroy() calls domid_free() to release the ID
>    c) domctl incorrectly calls domid_free() again
> 
> This double-free violates the domain ID management invariants and causes
> system instability. The fix ensures domid_free() is called exactly once
> per allocated domain ID, maintaining proper resource cleanup
> semantics.
> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>

Thanks a quick fix and sorry for the breakage.

--
Denis

On 13/09/2025 11:44 am, Oleksii Moisieiev wrote:
> Remove redundant domid_free() call in the XEN_DOMCTL_createdomain error
> handling path to prevent a double-free condition.
>
> When domain_create() fails, it internally calls _domain_destroy() during
> its cleanup routine, which already invokes domid_free() to release the
> allocated domain ID. The additional domid_free() call in the domctl error
> path creates a double-free scenario, triggering an assertion failure in
> domid.c:
>
>     Assertion 'rc' failed at common/domid.c:84
>
> The domain creation flow is:
> 1. domid_alloc() allocates a domain ID
> 2. domain_create() is called with the allocated ID
> 3. If domain_create() fails:
>    a) domain_create() calls _domain_destroy() internally
>    b) _domain_destroy() calls domid_free() to release the ID
>    c) domctl incorrectly calls domid_free() again
>
> This double-free violates the domain ID management invariants and causes
> system instability. The fix ensures domid_free() is called exactly once
> per allocated domain ID, maintaining proper resource cleanup
> semantics.

Fixes: 2d5065060710 ("xen/domain: unify domain ID allocation")

> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

the tl;dr is that domain_create() either inserts the domain into the
domlist, or cleans up after itself.

The domid alloc infrastructure is problematic in multiple ways, not
least because it now means there are two sources of truth for which
domain's exist, and they are not interlocked.

I would have blocked this from being committed if I'd had any time to
look at it.  It will need remediating one way or another before 4.21
goes out.

~Andrew

On 9/13/25 07:56, Andrew Cooper wrote:
> On 13/09/2025 11:44 am, Oleksii Moisieiev wrote:
>> Remove redundant domid_free() call in the XEN_DOMCTL_createdomain error
>> handling path to prevent a double-free condition.
>>
>> When domain_create() fails, it internally calls _domain_destroy() during
>> its cleanup routine, which already invokes domid_free() to release the
>> allocated domain ID. The additional domid_free() call in the domctl error
>> path creates a double-free scenario, triggering an assertion failure in
>> domid.c:
>>
>>     Assertion 'rc' failed at common/domid.c:84
>>
>> The domain creation flow is:
>> 1. domid_alloc() allocates a domain ID
>> 2. domain_create() is called with the allocated ID
>> 3. If domain_create() fails:
>>    a) domain_create() calls _domain_destroy() internally
>>    b) _domain_destroy() calls domid_free() to release the ID
>>    c) domctl incorrectly calls domid_free() again
>>
>> This double-free violates the domain ID management invariants and causes
>> system instability. The fix ensures domid_free() is called exactly once
>> per allocated domain ID, maintaining proper resource cleanup
>> semantics.
> 
> Fixes: 2d5065060710 ("xen/domain: unify domain ID allocation")
> 
>> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> the tl;dr is that domain_create() either inserts the domain into the
> domlist, or cleans up after itself.
> 
> The domid alloc infrastructure is problematic in multiple ways, not
> least because it now means there are two sources of truth for which
> domain's exist, and they are not interlocked.
> 
> I would have blocked this from being committed if I'd had any time to
> look at it.  It will need remediating one way or another before 4.21
> goes out.
Revert time?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)