All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Eric Biggers" <ebiggers@kernel.org>
Cc: linux-raid@vger.kernel.org, yukuai@fygo.io
Subject: Re: [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
Date: Mon, 15 Jun 2026 19:25:38 +0000	[thread overview]
Message-ID: <20260615192538.EE0C31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260615190338.26581-1-ebiggers@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AVX-512 XOR implementation is forced based solely on `boot_cpu_has(X86_FEATURE_AVX512F)` without checking if the OS actually manages ZMM registers via `cpu_has_xfeatures()`. This introduces an Undefined Opcode (#UD) kernel panic vulnerability on systems where AVX-512 is supported by the CPU but restricted in XCR0.
--

commit b63aaca8e8b386ef1820d1caac3bce2cb328e8c3
Author: Eric Biggers <ebiggers@kernel.org>

lib/raid/xor: x86: Add AVX-512 optimized xor_gen()

This commit adds an AVX-512 implementation of xor_gen() using 512-bit ZMM
registers and the vpternlogq instruction. It enables this implementation on
x86_64 CPUs that support AVX512F but do not prefer YMM registers, avoiding
known downclocking issues on older architectures. The implementation
explicitly omits the cpu_has_xfeatures() check.

> Note: for now I omitted the cpu_has_xfeatures() check that the AVX-512
> optimized crypto and CRC code does, since it's not implemented on
> User-Mode Linux and it's never been present in the RAID6 code either.

[Severity: High]
Does omitting the cpu_has_xfeatures() check introduce a potential Invalid
Opcode exception in virtualized environments?

If a hypervisor advertises AVX512F in CPUID but omits the AVX-512 FPU
extended state from XSTATE, boot_cpu_has(X86_FEATURE_AVX512F) will evaluate
to true, but the OS will not enable AVX-512 in XCR0. Executing 512-bit
EVEX instructions without XCR0[7:5] enabled causes an Invalid Opcode
exception and kernel panic.

Would it be safer to preserve this check, perhaps providing a stub or
using ifdef for User-Mode Linux compatibility instead?

> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c66..b5d49376fc974 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
[ ... ]
> @@ -6,22 +6,26 @@ extern struct xor_block_template xor_block_p5_mmx;
>  extern struct xor_block_template xor_block_sse;
>  extern struct xor_block_template xor_block_sse_pf64;
>  extern struct xor_block_template xor_block_avx;
> +extern struct xor_block_template xor_block_avx512;
>  
>  static __always_inline void __init arch_xor_init(void)
>  {
> -	if (boot_cpu_has(X86_FEATURE_AVX) &&
> -	    boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> +	if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> +	    boot_cpu_has(X86_FEATURE_OSXSAVE) &&
> +	    !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
> +		/* AVX-512 will be the best; no need to try others. */
> +		/* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
> +		xor_force(&xor_block_avx512);

[Severity: High]
Can this force the AVX-512 implementation on systems that do not actually
support ZMM registers in the OS?

During boot, fpu__init_system_xstate() limits fpu_kernel_cfg.max_features
based on the XSTATE leaf, but does not clear X86_FEATURE_AVX512F from
boot_cpu_data.x86_capability. As a result, this check passes and forces
xor_block_avx512, which then attempts to execute AVX-512 instructions on
an unsupported FPU context.

Is it necessary to include cpu_has_xfeatures(XFEATURE_MASK_AVX512, NULL)
here to verify the FPU context supports these instructions before forcing
the block?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615190338.26581-1-ebiggers@kernel.org?part=1

  reply	other threads:[~2026-06-15 19:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-15 19:03 [PATCH v3] lib/raid/xor: x86: Add AVX-512 optimized xor_gen() Eric Biggers
2026-06-15 19:25 ` sashiko-bot [this message]
2026-06-15 20:10 ` Eric Biggers
2026-06-15 21:16   ` Borislav Petkov
2026-06-15 21:29     ` Eric Biggers
2026-06-15 23:53       ` Borislav Petkov
2026-06-16  0:29         ` Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260615192538.EE0C31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yukuai@fygo.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.