Re: [PATCH v1 08/13] mshv: accept partial CPU feature banks

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Kernel stack memory leak to the hypervisor on ARM64 when the MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES flag is omitted.
--

--- Patch [8]: [PATCH v1 08/13] mshv: accept partial CPU feature banks ---
commit f76ad6e0838cf1ac9e5affb070dfe61ab6bea40d
Author: Wei Liu <[email protected]>

mshv: accept partial CPU feature banks

Allow userspace to provide fewer than MSHV_NUM_CPU_FEATURES_BANKS banks
for the create-partition v2 ioctl. Preserve the default disabled-feature
mask for omitted banks instead of rejecting the ioctl or overwriting all
banks.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 11d3c86050b45..729916e1eacff 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -2423,20 +2423,12 @@ static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
>  		    args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
>  			return -EINVAL;
>  
> -		if (args.pt_num_cpu_fbanks != MSHV_NUM_CPU_FEATURES_BANKS ||
> +		if (args.pt_num_cpu_fbanks > MSHV_NUM_CPU_FEATURES_BANKS ||
>  		    mshv_field_nonzero(args, pt_rsvd) ||
>  		    mshv_field_nonzero(args, pt_rsvd1))
>  			return -EINVAL;
>  
> -		/*
> -		 * Note this assumes MSHV_NUM_CPU_FEATURES_BANKS will never
> -		 * change and equals HV_PARTITION_PROCESSOR_FEATURES_BANKS
> -		 * (i.e. 2).
> -		 *
> -		 * Further banks (index >= 2) will be modifiable as 'early'
> -		 * properties via the set partition property hypercall.
> -		 */
> -		for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> +		for (i = 0; i < args.pt_num_cpu_fbanks; i++)
>  			disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
>  
>  #if IS_ENABLED(CONFIG_X86_64)

[Severity: Medium]
Does this code leak uninitialized kernel stack memory to the hypervisor on
ARM64?

When a user calls the MSHV_CREATE_PARTITION ioctl without the
MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES flag, the initialization of
disabled_processor_xsave_features appears to be skipped on ARM64.

In mshv_ioctl_create_partition(), creation_properties is allocated on the
kernel stack uninitialized:

    struct hv_partition_creation_properties creation_properties;

If the MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES flag is omitted, the block below
is skipped in mshv_ioctl_process_pt_flags():

    if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {

The default initialization of disabled_processor_xsave_features is
conditionally excluded via #if IS_ENABLED(CONFIG_X86_64), leaving it
uninitialized on ARM64.

Later, hv_call_create_partition() copies the entire structure to the
hypercall input page:

    memcpy(&input->partition_creation_properties, &creation_properties,
           sizeof(creation_properties));

Could this send 8 bytes of uninitialized kernel stack memory to the
hypervisor, potentially causing it to disable random XSAVE features for the
guest VM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8