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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.05.2025 01:54, 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 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@xxxxxxx>
> Change-Id: Ifaca3533dcce3f409c2efa292c7e96fba6371d9d
> ---
>  tools/libs/light/libxl_x86.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
> index 0b1c2d3a96..b8f6663829 100644
> --- a/tools/libs/light/libxl_x86.c
> +++ b/tools/libs/light/libxl_x86.c
> @@ -821,10 +821,12 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc 
> *gc,
>       * 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) {
> +        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);
> +    }

I think at least the latter half of the comment wants to move inside the
if() then.

Jan