From 2b18f341cb5c66bbc3260a8e0dd9f42b2f58d78c Mon Sep 17 00:00:00 2001 From: "Daniel P. Smith" Date: Tue, 29 Oct 2024 16:42:29 +0100 Subject: [PATCH] x86/boot: Fix XSM module handling during PVH boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Andrew Cooper Reviewed-by: Daniel P. Smith Acked-by: Roger Pau Monné master commit: 6cf0aaeb8df951fb34679f0408461a5c67cb02c6 master date: 2024-10-23 18:14:24 +0100 --- xen/arch/x86/setup.c | 2 +- xen/include/xsm/xsm.h | 7 +++++-- xen/xsm/xsm_core.c | 7 ++++--- xen/xsm/xsm_policy.c | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 1d5d3f8a66..689f828d6a 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1771,7 +1771,7 @@ void __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 627c0d2731..5867ccceaf 100644 --- 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 eaa028109b..82b0d76d40 100644 --- 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 8dafbc9381..9244a3612d 100644 --- 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;