Re: [PATCH v2 2/8] um: Check for missing AVX and AVX-512 xstate bits

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Incomplete XCR0 validation whitelist allows AVX-dependent and AMX features to bypass state checks, causing guest application crashes.

Pre-existing issues:
- [High] Substring matching flaw in `parse_host_cpu_flags()` incorrectly enables CPU features, leading to application crashes.
--

--- Patch [2]: [PATCH v2 2/8] um: Check for missing AVX and AVX-512 xstate bits ---
commit 14fa8af6eb070800589e8e98d9a0ba3804acb926
Author: Eric Biggers <[email protected]>

um: Check for missing AVX and AVX-512 xstate bits

This patch checks if the CPU declares AVX or AVX-512 support and verifies
that the corresponding bits are set in the XCR0 register. If missing, it
warns and does not set the X86_FEATURE_* flags.

> diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
> index 2141f5f1f5a20..aafbaef2ae82f 100644
> --- a/arch/um/kernel/um_arch.c
> +++ b/arch/um/kernel/um_arch.c
[ ... ]
> @@ -264,12 +264,88 @@ unsigned long brk_start;
[ ... ]
> +static void __init validate_and_set_cpu_cap(int cap, u64 xcr0)
> +{
> +	/*
> +	 * Check for missing xstate features right away, so that there's no
> +	 * perceived need for all optimized code in the kernel to do so.
> +	 */
> +	switch (cap) {
> +	case X86_FEATURE_AVX:
> +	case X86_FEATURE_AVX2:

[Severity: High]
Will this switch statement inadvertently bypass state checks for features like
X86_FEATURE_AMX_TILE, X86_FEATURE_F16C, X86_FEATURE_XOP, and
X86_FEATURE_FMA4?

If these features are missing from the switch cases, they will fall through
and be unconditionally enabled at the end of the function. If the host CPU
supports AMX, UML will advertise amx_tile. If a guest application attempts
to use it without the UML process calling prctl(ARCH_REQ_XCOMP_PERM), the
host will send SIGILL.

A similar #UD crash could occur for f16c, fma4, or xop if host AVX XCR0
state is disabled but the features are not cleared by the host.

> +	case X86_FEATURE_AVX_VNNI:
> +	case X86_FEATURE_FMA:
[ ... ]
> +		break;
> +	}
> +	set_cpu_cap(&boot_cpu_data, cap);
> +}
> +
>  static void __init parse_host_cpu_flags(char *line)
>  {
> +	u64 xcr0 = read_xcr0();
>  	int i;
> +
>  	for (i = 0; i < 32*NCAPINTS; i++) {
>  		if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
> -			set_cpu_cap(&boot_cpu_data, i);
> +			validate_and_set_cpu_cap(i, xcr0);

[Severity: High]
This is a pre-existing issue, but does this substring match cause false
positive feature detection?

Using strstr without word boundary checks means that if a host CPU supports
fma4 but not fma (like AMD Bulldozer), the string "fma" will match inside
"fma4".

This causes UML to falsely advertise fma to the guest, and guest
applications probing for fma will reliably crash with a #UD exception when
attempting to execute FMA3 instructions.

>  	}
>  }

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