[PATCH v2] tools/libxl: Only access legacy altp2m on HVM

[PATCH v2] tools/libxl: Only access legacy altp2m on HVM

[Jason Andryuk]

Only access the HVM union b_info->u.hvm on HVM guests.  The union
access is not guarded, so this reads and sets the default even on
non-HVM guests.  Usually this doesn't matter as PV and PVH unions are
smaller and zero-initialized, but the zero default will be re-written as
a -1 boolean.  Generally, it could incorrectly set b_info->altp2m
through aliased data.

Fixes: 0291089f6ea8 ("xen: enable altp2m at create domain domctl")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v2:
Move comment.
---
 tools/libs/light/libxl_x86.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index 0b1c2d3a96..867addfcab 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -814,17 +814,19 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
     libxl_defbool_setdefault(&b_info->acpi, true);
     libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false);
 
-    /*
-     * The config parameter "altp2m" replaces the parameter "altp2mhvm".
-     * For legacy reasons, both parameters are accepted on x86 HVM guests.
-     *
-     * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
-     * Otherwise set altp2m based on the field info->altp2m.
-     */
-    libxl_defbool_setdefault(&b_info->u.hvm.altp2m, false);
-    if (b_info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
-        libxl_defbool_val(b_info->u.hvm.altp2m))
-        b_info->altp2m = libxl_defbool_val(b_info->u.hvm.altp2m);
+    if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        /*
+         * The config parameter "altp2m" replaces the parameter "altp2mhvm".
+         * For legacy reasons, both parameters are accepted on x86 HVM guests.
+         *
+         * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
+         * Otherwise set altp2m based on the field info->altp2m.
+         */
+        libxl_defbool_setdefault(&b_info->u.hvm.altp2m, false);
+        if (b_info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
+            libxl_defbool_val(b_info->u.hvm.altp2m))
+            b_info->altp2m = libxl_defbool_val(b_info->u.hvm.altp2m);
+    }
 
     return 0;
 }
-- 
2.49.0

Re: [PATCH v2] tools/libxl: Only access legacy altp2m on HVM

[Anthony PERARD]

On Tue, May 13, 2025 at 11:44:19AM -0400, Jason Andryuk wrote:
> Only access the HVM union b_info->u.hvm on HVM guests.  The union
> access is not guarded, so this reads and sets the default even on
> non-HVM guests.  Usually this doesn't matter as PV and PVH unions are
> smaller and zero-initialized, but the zero default will be re-written as
> a -1 boolean.  Generally, it could incorrectly set b_info->altp2m
> through aliased data.
> 
> Fixes: 0291089f6ea8 ("xen: enable altp2m at create domain domctl")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD