[PATCH v2 6/6] xen: make config argument const

[Julian Vetter <julian.vetter@xxxxxxxxxx>]

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>
---
Changes in v2:
- New patch
---
 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                 |  6 ++++--
 8 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index d6d80ac55d..f605446cc5 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 0835ddeeec..a973679eaf 100644
--- a/xen/arch/arm/firmware/scmi-smc.c
+++ b/xen/arch/arm/firmware/scmi-smc.c
@@ -82,7 +82,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 2819ff4e7c..e096a53cb5 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -289,7 +289,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 4252339978..35f591ab5d 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 011292e9f7..66ed7454ba 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -756,9 +756,11 @@ 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.
+ * requested by the toolstack. config is not modified: createdomain is
+ * input-only, and the toolstack is expected to have already resolved any
+ * defaults.
  */
-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
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech