xen/domctl: add XEN_DOMCTL_CDF_ALL mask

[XEN][PATCH] xen/domctl: add XEN_DOMCTL_CDF_ALL mask

Posted by Grygorii Strashko 2 months, 4 weeks ago

From: Grygorii Strashko <grygorii_strashko@epam.com>

Every XEN_DOMCTL_CDF_x flag:
- is defined in public/domctl.h
- explicitly listed in sanitise_domain_config() (common/domain.c) to form
mask containing all supported DOMCTL_CDF flags for "Unknown CDF flags"
check.

So, it is required to modify two files every time XEN_DOMCTL_CDF_x flags
set is modified.

Simplify the things by introducing XEN_DOMCTL_CDF_ALL mask, so
sanitise_domain_config() no need to be modified any more.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
 xen/common/domain.c         | 7 +------
 xen/include/public/domctl.h | 7 +++++++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 775c33928585..4f91316ad93e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -730,12 +730,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
     bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
     bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
 
-    if ( config->flags &
-         ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
-           XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
-           XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
-           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
-           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
+    if ( config->flags & ~XEN_DOMCTL_CDF_ALL )
     {
         dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
         return -EINVAL;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 8f6708c0a7cd..94a8e3042cbf 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -72,6 +72,13 @@ struct xen_domctl_createdomain {
 /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
 #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
 
+#define XEN_DOMCTL_CDF_ALL                                                     \
+        (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |                             \
+         XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |                \
+         XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |                     \
+         XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |                    \
+         XEN_DOMCTL_CDF_trap_unmapped_accesses)
+
     uint32_t flags;
 
 #define _XEN_DOMCTL_IOMMU_no_sharept  0
-- 
2.34.1

Re: [XEN][PATCH] xen/domctl: add XEN_DOMCTL_CDF_ALL mask

Posted by Andrew Cooper 1 month, 2 weeks ago

On 11/11/2025 8:16 pm, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Every XEN_DOMCTL_CDF_x flag:
> - is defined in public/domctl.h
> - explicitly listed in sanitise_domain_config() (common/domain.c) to form
> mask containing all supported DOMCTL_CDF flags for "Unknown CDF flags"
> check.
>
> So, it is required to modify two files every time XEN_DOMCTL_CDF_x flags
> set is modified.
>
> Simplify the things by introducing XEN_DOMCTL_CDF_ALL mask, so
> sanitise_domain_config() no need to be modified any more.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
>  xen/common/domain.c         | 7 +------
>  xen/include/public/domctl.h | 7 +++++++
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 775c33928585..4f91316ad93e 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -730,12 +730,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>      bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
>      bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
>  
> -    if ( config->flags &
> -         ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> -           XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
> -           XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> -           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
> +    if ( config->flags & ~XEN_DOMCTL_CDF_ALL )
>      {
>          dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>          return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 8f6708c0a7cd..94a8e3042cbf 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -72,6 +72,13 @@ struct xen_domctl_createdomain {
>  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
>  #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
>  
> +#define XEN_DOMCTL_CDF_ALL                                                     \
> +        (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |                             \
> +         XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |                \
> +         XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |                     \
> +         XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |                    \
> +         XEN_DOMCTL_CDF_trap_unmapped_accesses)
> +
>      uint32_t flags;

The problem with this is that now userspace has a constant called
XEN_DOMCTL_CDF_ALL in scope which is unsafe to use.

If the new constant was within #ifdef __XEN__ then at least it couldn't
be misused by userspace.

~Andrew

Re: [XEN][PATCH] xen/domctl: add XEN_DOMCTL_CDF_ALL mask

Posted by Grygorii Strashko 1 month, 3 weeks ago


On 11.11.25 22:16, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
> 
> Every XEN_DOMCTL_CDF_x flag:
> - is defined in public/domctl.h
> - explicitly listed in sanitise_domain_config() (common/domain.c) to form
> mask containing all supported DOMCTL_CDF flags for "Unknown CDF flags"
> check.
> 
> So, it is required to modify two files every time XEN_DOMCTL_CDF_x flags
> set is modified.
> 
> Simplify the things by introducing XEN_DOMCTL_CDF_ALL mask, so
> sanitise_domain_config() no need to be modified any more.
> 
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---

Seems nobody interested, sad, dropping from backlog.


>   xen/common/domain.c         | 7 +------
>   xen/include/public/domctl.h | 7 +++++++
>   2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 775c33928585..4f91316ad93e 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -730,12 +730,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>       bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
>       bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
>   
> -    if ( config->flags &
> -         ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> -           XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
> -           XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> -           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
> +    if ( config->flags & ~XEN_DOMCTL_CDF_ALL )
>       {
>           dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>           return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 8f6708c0a7cd..94a8e3042cbf 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -72,6 +72,13 @@ struct xen_domctl_createdomain {
>   /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
>   #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
>   
> +#define XEN_DOMCTL_CDF_ALL                                                     \
> +        (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |                             \
> +         XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |                \
> +         XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |                     \
> +         XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |                    \
> +         XEN_DOMCTL_CDF_trap_unmapped_accesses)
> +
>       uint32_t flags;
>   
>   #define _XEN_DOMCTL_IOMMU_no_sharept  0

-- 
Best regards,
-grygorii

Re: [XEN][PATCH] xen/domctl: add XEN_DOMCTL_CDF_ALL mask

Posted by Teddy Astie 1 month, 3 weeks ago

Le 18/12/2025 à 16:21, Grygorii Strashko a écrit :
> 
> 
> On 11.11.25 22:16, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Every XEN_DOMCTL_CDF_x flag:
>> - is defined in public/domctl.h
>> - explicitly listed in sanitise_domain_config() (common/domain.c) to form
>> mask containing all supported DOMCTL_CDF flags for "Unknown CDF flags"
>> check.
>>
>> So, it is required to modify two files every time XEN_DOMCTL_CDF_x flags
>> set is modified.
>>
>> Simplify the things by introducing XEN_DOMCTL_CDF_ALL mask, so
>> sanitise_domain_config() no need to be modified any more.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
> 
> Seems nobody interested, sad, dropping from backlog.
> 

Sorry, I missed this one (I guess others have too).

> 
>>   xen/common/domain.c         | 7 +------
>>   xen/include/public/domctl.h | 7 +++++++
>>   2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 775c33928585..4f91316ad93e 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -730,12 +730,7 @@ static int sanitise_domain_config(struct 
>> xen_domctl_createdomain *config)
>>       bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
>>       bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
>> -    if ( config->flags &
>> -         ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>> -           XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>> -           XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
>> -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
>> -           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
>> +    if ( config->flags & ~XEN_DOMCTL_CDF_ALL )
>>       {
>>           dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>>           return -EINVAL;
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>> index 8f6708c0a7cd..94a8e3042cbf 100644
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -72,6 +72,13 @@ struct xen_domctl_createdomain {
>>   /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
>>   #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
>> +#define 
>> XEN_DOMCTL_CDF_ALL                                                     \
>> +        (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap | 
>>                              \
>> +         XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off | 
>>                 \
>> +         XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu | 
>>                      \
>> +         XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu | 
>>                     \
>> +         XEN_DOMCTL_CDF_trap_unmapped_accesses)
>> +
>>       uint32_t flags;
>>   #define _XEN_DOMCTL_IOMMU_no_sharept  0
> 

Given that we already have XEN_DOMCTL_CDF_MAX, we could base the check 
using it instead.
Something like checking fls(config->flags) > fls(XEN_DOMCTL_CDF_MAX) ?

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [XEN][PATCH] xen/domctl: add XEN_DOMCTL_CDF_ALL mask

Posted by Grygorii Strashko 1 month, 2 weeks ago


On 19.12.25 18:10, Teddy Astie wrote:
> Le 18/12/2025 à 16:21, Grygorii Strashko a écrit :
>>
>>
>> On 11.11.25 22:16, Grygorii Strashko wrote:
>>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>>
>>> Every XEN_DOMCTL_CDF_x flag:
>>> - is defined in public/domctl.h
>>> - explicitly listed in sanitise_domain_config() (common/domain.c) to form
>>> mask containing all supported DOMCTL_CDF flags for "Unknown CDF flags"
>>> check.
>>>
>>> So, it is required to modify two files every time XEN_DOMCTL_CDF_x flags
>>> set is modified.
>>>
>>> Simplify the things by introducing XEN_DOMCTL_CDF_ALL mask, so
>>> sanitise_domain_config() no need to be modified any more.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>> ---
>>
>> Seems nobody interested, sad, dropping from backlog.
>>
> 
> Sorry, I missed this one (I guess others have too).
> 
>>
>>>    xen/common/domain.c         | 7 +------
>>>    xen/include/public/domctl.h | 7 +++++++
>>>    2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>>> index 775c33928585..4f91316ad93e 100644
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -730,12 +730,7 @@ static int sanitise_domain_config(struct
>>> xen_domctl_createdomain *config)
>>>        bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
>>>        bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
>>> -    if ( config->flags &
>>> -         ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>>> -           XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>>> -           XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
>>> -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
>>> -           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
>>> +    if ( config->flags & ~XEN_DOMCTL_CDF_ALL )
>>>        {
>>>            dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>>>            return -EINVAL;
>>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>>> index 8f6708c0a7cd..94a8e3042cbf 100644
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -72,6 +72,13 @@ struct xen_domctl_createdomain {
>>>    /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
>>>    #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
>>> +#define
>>> XEN_DOMCTL_CDF_ALL                                                     \
>>> +        (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>>>                               \
>>> +         XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>>>                  \
>>> +         XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
>>>                       \
>>> +         XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
>>>                      \
>>> +         XEN_DOMCTL_CDF_trap_unmapped_accesses)
>>> +
>>>        uint32_t flags;
>>>    #define _XEN_DOMCTL_IOMMU_no_sharept  0
>>
> 
> Given that we already have XEN_DOMCTL_CDF_MAX, we could base the check
> using it instead.
> Something like checking fls(config->flags) > fls(XEN_DOMCTL_CDF_MAX) ?

That's what I've tried first.
The problem is that there is possibility of CDF_X to be removed/deprecated, small, but still it is.
In such case fls() can't be used.
So using define which list allowed flags is safer from long term point of view.

-- 
Best regards,
-grygorii