x86/boot: Fix XSM module handling during PVH boot

As detailed in commit 0fe607b2a144 ("x86/boot: Fix PVH boot during boot_info
transition period"), the use of __va(mbi->mods_addr) constitutes a
use-after-free on the PVH boot path.

This pattern has been in use since before PVH support was added.  This has
most likely gone unnoticed because no-one's tried using a detached Flask
policy in a PVH VM before.

Plumb the boot_info pointer down, replacing module_map and mbi.  Importantly,
bi->mods[].mod is a safe way to access the module list during PVH boot.

As this is the final non-bi use of mbi in __start_xen(), make the pointer
unusable once bi has been established, to prevent new uses creeping back in.
This is a stopgap until mbi can be fully removed.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 6cf0aaeb8df951fb34679f0408461a5c67cb02c6
master date: 2024-10-23 18:14:24 +0100

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 9e5e871b318387830cb1ed7726e5ad79118fd8a8..89482140cfbaa344968273f7e4b1c559ea4d7d83 100644 (file)
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1792,7 +1792,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
                                   RANGESETF_prettyprint_hex);
 
-    xsm_multiboot_init(module_map, mbi);
+    xsm_multiboot_init(module_map, mbi, mod);
 
     /*
      * IOMMU-related ACPI table parsing may require some of the system domains

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 627c0d2731af2c7ddfcf6ab449ae3b6d467f6e66..5867ccceaff79b33971b3fbe12cd3296e368a263 100644 (file)
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -779,9 +779,11 @@ static inline int xsm_argo_send(const struct domain *d, const struct domain *t)
 
 #ifdef CONFIG_MULTIBOOT
 int xsm_multiboot_init(
-    unsigned long *module_map, const multiboot_info_t *mbi);
+    unsigned long *module_map, const multiboot_info_t *mbi,
+    const module_t mods[]);
 int xsm_multiboot_policy_init(
     unsigned long *module_map, const multiboot_info_t *mbi,
+    const module_t mods[],
     void **policy_buffer, size_t *policy_size);
 #endif
 
@@ -829,7 +831,8 @@ static const inline struct xsm_ops *silo_init(void)
 
 #ifdef CONFIG_MULTIBOOT
 static inline int xsm_multiboot_init (
-    unsigned long *module_map, const multiboot_info_t *mbi)
+    unsigned long *module_map, const multiboot_info_t *mbi,
+    const module_t mods[])
 {
     return 0;
 }

diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index eaa028109bde0bbe5d196e49dcdc5df8ba70dd79..82b0d76d407bad3660eeab5207546b0b1ad04184 100644 (file)
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -140,7 +140,8 @@ static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_init(
-    unsigned long *module_map, const multiboot_info_t *mbi)
+    unsigned long *module_map, const multiboot_info_t *mbi,
+    const module_t mods[])
 {
     int ret = 0;
     void *policy_buffer = NULL;
@@ -150,8 +151,8 @@ int __init xsm_multiboot_init(
 
     if ( XSM_MAGIC )
     {
-        ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer,
-                                        &policy_size);
+        ret = xsm_multiboot_policy_init(module_map, mbi, mods,
+                                        &policy_buffer, &policy_size);
         if ( ret )
         {
             bootstrap_map(NULL);

diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 8dafbc93810f8e1c1e6415ec1b5c9d7ee46a7d72..9244a3612d17bbb8b4da432c7c2e264d545730c7 100644 (file)
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -32,10 +32,10 @@
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_policy_init(
     unsigned long *module_map, const multiboot_info_t *mbi,
+    const module_t mod[],
     void **policy_buffer, size_t *policy_size)
 {
     int i;
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
     int rc = 0;
     u32 *_policy_start;
     unsigned long _policy_len;