This patch ensure that the domid 0 is allocated only during
start_xen() function by the create_dom0().
Add a comment in create_domUs() right before domain_create()
to explain the importance of the pre-increment operator
on the variable max_init_domid.
Add an additional check in do_domctl() to make sure domid 0
is never used when calling domain_create().

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
 xen/arch/arm/domain_build.c | 5 +++++
 xen/common/domctl.c         | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d7c9c7f4d1..3fa5c8e54c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2508,6 +2508,11 @@ void __init create_domUs(void)
                                          GUEST_VPL011_SPI - 32 + 1);
         }
 
+        /*
+         * The variable max_init_domid is initialized with zero, so here it's
+         * very important to use the pre-increment operator to call
+         * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
+         */
         d = domain_create(++max_init_domid, &d_cfg, false);
         if ( IS_ERR(d) )
             panic("Error creating domain %s\n", dt_node_name(node));
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index af044e2eda..8258f157ef 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -419,7 +419,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             {
                 if ( dom == DOMID_FIRST_RESERVED )
                     dom = 1;
-                if ( is_free_domid(dom) )
+                if ( (dom != 0) && is_free_domid(dom) )
                     break;
             }
 
-- 
2.17.1

On 08.04.2021 11:48, Luca Fancellu wrote:
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -419,7 +419,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>              {
>                  if ( dom == DOMID_FIRST_RESERVED )
>                      dom = 1;
> -                if ( is_free_domid(dom) )
> +                if ( (dom != 0) && is_free_domid(dom) )
>                      break;
>              }
>  

I don't think this change is needed - I don't see how dom could
ever end up being zero. The code is already intended to be safe
wrt accidentally creating a domain with ID zero. (Granted "rover"
would benefit from being moved into the yet more narrow scope,
which would make this even more obvious.)

Jan

> On 8 Apr 2021, at 11:46, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 08.04.2021 11:48, Luca Fancellu wrote:
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -419,7 +419,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>             {
>>                 if ( dom == DOMID_FIRST_RESERVED )
>>                     dom = 1;
>> -                if ( is_free_domid(dom) )
>> +                if ( (dom != 0) && is_free_domid(dom) )
>>                     break;
>>             }
>> 
> 
> I don't think this change is needed - I don't see how dom could
> ever end up being zero. The code is already intended to be safe
> wrt accidentally creating a domain with ID zero. (Granted "rover"
> would benefit from being moved into the yet more narrow scope,
> which would make this even more obvious.)

Yes I agree, I will remove the check in the next version patch.

Cheers,
Luca

> 
> Jan