Re: [PATCH v7 5/5] xen: make config argument const

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

On 10/12/26 3:44 PM, Julian Vetter wrote:
> arch_sanitise_domain_config() validates the configuration requested by
> the toolstack, and should not fill anything in. The config struct passed
> to createdomain is supposed to be pure input. ARM used to abuse this
> (GIC_NATIVE resolution, now removed) to smuggle output back to the
> toolstack. Making the parameter const stops that type of abuse from
> happening on any architecture.
>
> The x86 implementation turned out to have its own instance of the same
> issue. It set XEN_DOMCTL_CDF_oos_off into config->flags for non-HVM
> guests. Since The sanitisation runs before the function domain_create()
> copies config->flags into d->options, this relied on mutating the
> toolstack's config to take effect. Move the default onto d->options
> directly in arch_domain_create() (which runs after d->options is
> populated), where all the remaining domain options are resolved. This
> has the same effect and no mutation of the input config is required.
>
> ARM, PPC and RISC-V need no equivalent change, Their implementations
> were already read-only.
>
> Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> # x86
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
LGTM on riscv.
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx> # riscv> ---
> Changes in v7:
> - No changes.
> ---
>   xen/arch/arm/domain.c                   |  2 +-
>   xen/arch/arm/firmware/sci.c             |  2 +-
>   xen/arch/arm/firmware/scmi-smc.c        |  2 +-
>   xen/arch/arm/include/asm/firmware/sci.h |  6 +++---
>   xen/arch/ppc/stubs.c                    |  2 +-
>   xen/arch/riscv/domain.c                 |  2 +-
>   xen/arch/x86/domain.c                   | 16 ++++++++--------
>   xen/include/xen/sched.h                 |  5 ++---
>   8 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 4ca3a3749f..34b5a85f9f 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -557,7 +557,7 @@ static bool v8r_el1_msa_domain_sanitise_config(
>       }
>   }
>   
> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> +int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
>   {
>       unsigned int max_vcpus;
>       unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
> diff --git a/xen/arch/arm/firmware/sci.c b/xen/arch/arm/firmware/sci.c
> index aa93cda7f0..f73ed06092 100644
> --- a/xen/arch/arm/firmware/sci.c
> +++ b/xen/arch/arm/firmware/sci.c
> @@ -45,7 +45,7 @@ int sci_domain_init(struct domain *d, struct 
> xen_domctl_createdomain *config)
>       return cur_mediator->domain_init(d, config);
>   }
>   
> -int sci_domain_sanitise_config(struct xen_domctl_createdomain *config)
> +int sci_domain_sanitise_config(const struct xen_domctl_createdomain *config)
>   {
>       if ( !cur_mediator )
>           return 0;
> diff --git a/xen/arch/arm/firmware/scmi-smc.c b/xen/arch/arm/firmware/scmi-smc.c
> index a0cc6c6192..391df8b945 100644
> --- a/xen/arch/arm/firmware/scmi-smc.c
> +++ b/xen/arch/arm/firmware/scmi-smc.c
> @@ -68,7 +68,7 @@ static bool scmi_handle_smc(struct cpu_user_regs *regs)
>   }
>   
>   static int
> -scmi_smc_domain_sanitise_config(struct xen_domctl_createdomain *config)
> +scmi_smc_domain_sanitise_config(const struct xen_domctl_createdomain *config)
>   {
>       if ( config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_NONE &&
>            config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC )
> diff --git a/xen/arch/arm/include/asm/firmware/sci.h 
> b/xen/arch/arm/include/asm/firmware/sci.h
> index 485ce211c9..1d566be8e2 100644
> --- a/xen/arch/arm/include/asm/firmware/sci.h
> +++ b/xen/arch/arm/include/asm/firmware/sci.h
> @@ -32,7 +32,7 @@ struct sci_mediator_ops {
>        * it to sanitize domain SCI configuration parameters.
>        * Optional.
>        */
> -    int (*domain_sanitise_config)(struct xen_domctl_createdomain *config);
> +    int (*domain_sanitise_config)(const struct xen_domctl_createdomain 
> *config);
>   
>       /*
>        * Called during domain destruction, releases all resources, that
> @@ -101,7 +101,7 @@ int sci_domain_init(struct domain *d, struct 
> xen_domctl_createdomain *config);
>    * Sanitise domain configuration parameters.
>    *
>    */
> -int sci_domain_sanitise_config(struct xen_domctl_createdomain *config);
> +int sci_domain_sanitise_config(const struct xen_domctl_createdomain *config);
>   
>   /*
>    * Destroy SCI domain instance.
> @@ -162,7 +162,7 @@ static inline int sci_domain_init(struct domain *d,
>   }
>   
>   static inline int
> -sci_domain_sanitise_config(struct xen_domctl_createdomain *config)
> +sci_domain_sanitise_config(const struct xen_domctl_createdomain *config)
>   {
>       if ( config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_NONE )
>           return -EINVAL;
> diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
> index a333f06119..82a289af85 100644
> --- a/xen/arch/ppc/stubs.c
> +++ b/xen/arch/ppc/stubs.c
> @@ -162,7 +162,7 @@ void arch_vcpu_destroy(struct vcpu *v)
>       BUG_ON("unimplemented");
>   }
>   
> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> +int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
>   {
>       BUG_ON("unimplemented");
>   }
> diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
> index d94652809e..cfce8e91fa 100644
> --- a/xen/arch/riscv/domain.c
> +++ b/xen/arch/riscv/domain.c
> @@ -284,7 +284,7 @@ void sync_vcpu_execstate(struct vcpu *v)
>       /* Nothing to do -- no lazy switching */
>   }
>   
> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> +int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
>   {
>       return 0;
>   }
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 6ebe6c5052..368e4deff3 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -590,7 +590,7 @@ void arch_vcpu_destroy(struct vcpu *v)
>           ASSERT_UNREACHABLE();
>   }
>   
> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> +int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
>   {
>       bool hvm = config->flags & XEN_DOMCTL_CDF_hvm;
>       bool hap = config->flags & XEN_DOMCTL_CDF_hap;
> @@ -633,13 +633,6 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>           return -EINVAL;
>       }
>   
> -    if ( !hvm )
> -        /*
> -         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
> -         * for HVM guests.
> -         */
> -        config->flags |= XEN_DOMCTL_CDF_oos_off;
> -
>       if ( nested_virt && !hvm_nested_virt_supported() )
>       {
>           dprintk(XENLOG_INFO, "Nested virt requested but not available\n");
> @@ -833,6 +826,13 @@ int arch_domain_create(struct domain *d,
>   
>       spin_lock_init(&d->arch.e820_lock);
>   
> +    /*
> +     * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear for HVM
> +     * guests.
> +     */
> +    if ( !is_hvm_domain(d) )
> +        d->options |= XEN_DOMCTL_CDF_oos_off;
> +
>       if ( d->domain_id && cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) 
> )
>       {
>           if ( !opt_allow_unsafe )
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 73c54794ac..ce1fa636db 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -772,10 +772,9 @@ static inline void domain_update_node_affinity(struct 
> domain *d)
>   }
>   
>   /*
> - * To be implemented by each architecture, sanity checking the configuration
> - * and filling in any appropriate defaults.
> + * To be implemented by each architecture, sanity checking the configuration.
>    */
> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config);
> +int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config);
>   
>   /*
>    * Create a domain: the configuration is only necessary for real domain