Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Functional regression and uninitialized stack memory use on ARM64 for legacy partition creation. Pre-existing issues: - [Medium] TOCTOU double-fetch race allows validation and use of flag-gated parameters outside their flag gate. -- --- Patch [7]: [PATCH v1 07/13] mshv: default to safe partition CPU features --- commit 85299e9eb98a0db0daae1adea69f8e739d17d8db Author: Wei Liu <[email protected]> mshv: default to safe partition CPU features For legacy create-partition callers, disable all processor and xsave features first, then enable only the default feature set known to be supported. Passing an all-zero disabled feature mask can make SEV-SNP partition creation fail on Hyper-V. > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c > index cc38507570ac4..11d3c86050b45 100644 > --- a/drivers/hv/mshv_root_main.c > +++ b/drivers/hv/mshv_root_main.c > @@ -2333,6 +2333,86 @@ static long mshv_ioctl_process_pt_flags(void __user > *user_arg, u64 *pt_flags, > disabled_procs = &cr_props->disabled_processor_features; > disabled_xsave = &cr_props->disabled_processor_xsave_features; > > + /* Disable all processor features first. */ > + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++) > + disabled_procs->as_uint64[i] = U64_MAX; > + > +#if IS_ENABLED(CONFIG_X86_64) > + /* Enable default features that are known to be supported. */ [ ... ] > + disabled_xsave->as_uint64 = U64_MAX; > + disabled_xsave->xsave_support = 0; > + disabled_xsave->xsaveopt_support = 0; > + disabled_xsave->avx_support = 0; > +#endif > + > /* Check if user provided newer struct with feature fields */ > if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) { > if (copy_from_user(&args, user_arg, sizeof(args))) > return -EFAULT; [Severity: Medium] This is a pre-existing issue, but does this second fetch from userspace introduce a race condition? If a concurrent userspace thread clears the MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES flag between the first and second fetch, args.pt_flags will be overwritten with the new value. The code does not re-verify the flag before proceeding to validate and use extended v2 fields like args.pt_num_cpu_fbanks. This could allow the kernel to operate on uninitialized userspace garbage. [ ... ] > @@ -2371,16 +2451,6 @@ static long mshv_ioctl_process_pt_flags(void __user > *user_arg, u64 *pt_flags, > if (mshv_field_nonzero(args, pt_rsvd2)) > return -EINVAL; > #endif > - } else { > - /* > - * v1 behavior: try to enable everything. The hypervisor will > - * disable features that are not supported. The banks can be > - * queried via the get partition property hypercall. > - */ > - for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++) > - disabled_procs->as_uint64[i] = 0; > - > - disabled_xsave->as_uint64 = 0; > } > > /* Only support EXO partitions */ [Severity: High] Does removing this fallback initialization break legacy v1 callers on ARM64? With this code removed, disabled_procs->as_uint64[i] is unconditionally set to U64_MAX earlier in mshv_ioctl_process_pt_flags(), which disables all processor features. Because the new default initializations are inside an #if IS_ENABLED(CONFIG_X86_64) block, ARM64 callers never get those defaults nor the old 0 initialization. Additionally, disabled_xsave points to the disabled_processor_xsave_features field of cr_props (which is on the stack in mshv_ioctl_create_partition). Since it is no longer initialized to 0 here for non-x86 v1 callers, will it send uninitialized stack memory to the hypervisor? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
