All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chang S. Bae" <chang.seok.bae@intel.com>
To: Andrei Vagin <avagin@google.com>,
	Thomas Gleixner <tglx@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>
Cc: <linux-kernel@vger.kernel.org>, <criu@lists.linux.dev>,
	Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
	Alexander Mikhalitsyn <alexander@mihalicyn.com>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 4/8] x86/fpu: Document reasoning of FX-only fallback
Date: Wed, 2 Sep 2026 14:15:35 -0700	[thread overview]
Message-ID: <f1e3cb3f-a76d-432f-8d32-9a48c0a8d111@intel.com> (raw)
In-Reply-To: <20260817042048.1579415-5-avagin@google.com>

On 8/16/2026 9:20 PM, Andrei Vagin wrote:
> Add a comment to check_xstate_in_sigframe() to explain reasoning behind
> falling back to the FX-only state when signal frame metadata is
> inconsistent.
> 
> The fallback is intended to preserve backward compatibility with legacy
> user-space processes that are not aware of XSAVE states and might only
> fill or copy just the legacy FP state.
> 
> This fallback is dangerous as it can trigger silent corruptions of

I don't think the fallback itself makes it more _dangerous_. To the 
kernel, this is one of handling options _once_ an error happens. This is 
for the better interaction with user space.

> user-space state by resetting extended registers if the process was
> using them but the frame metadata was malformed.
> 
> Reviewed-by: Alexander Mikhalitsyn <alexander@mihalicyn.com>
> Signed-off-by: Andrei Vagin <avagin@google.com>
> ---
>   arch/x86/kernel/fpu/signal.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index 6a14b528ac7f..85021c5ea649 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -54,6 +54,14 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *buf_fx,
>   	if (likely(magic2 == FP_XSTATE_MAGIC2))
>   		return true;
>   err_setfx:
> +	/*
> +	 * The fallback to FX-only state is used to preserve backward
> +	 * compatibility with user-space processes that are not aware of xsave
> +	 * states.
> +	 *
> +	 * In all other cases, returning false (to trigger SIGSEGV) is
> +	 * preferred to avoid silent user-space state corruption.
> +	 */
>   	trace_x86_fpu_xstate_check_failed(x86_task_fpu(current));
>   
>   	/* Set the parameters for fx only state */

With all discussions in the past, yes. I'd vote for putting such comment 
on this path. With some massage in the changelog,

   Reviewed-by: Chang S. Bae <chang.seok.bae@intel.com>

Thanks,
Chang

  reply	other threads:[~2026-09-02 21:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  4:20 [PATCH v4 0/8] x86/fpu: Restore and reinforce signal frame portability Andrei Vagin
2026-08-17  4:20 ` [PATCH 1/8] x86/fpu: Document " Andrei Vagin
2026-09-02 21:15   ` Chang S. Bae
2026-09-03  4:51   ` Borislav Petkov
2026-08-17  4:20 ` [PATCH 2/8] x86/fpu: Clean up and rename variables in signal frame handling Andrei Vagin
2026-08-17  4:20 ` [PATCH 3/8] x86/fpu: Split __fpu_restore_sig to extract compat path Andrei Vagin
2026-09-02 21:15   ` Chang S. Bae
2026-08-17  4:20 ` [PATCH 4/8] x86/fpu: Document reasoning of FX-only fallback Andrei Vagin
2026-09-02 21:15   ` Chang S. Bae [this message]
2026-08-17  4:20 ` [PATCH 5/8] selftests/x86: Add a test for signal frame FPU portability Andrei Vagin
2026-08-17  4:20 ` [PATCH 6/8] x86/fpu: Fix potential underflow in xstate_calculate_size() Andrei Vagin
2026-09-02 21:15   ` Chang S. Bae
2026-08-17  4:20 ` [PATCH 7/8] x86/fpu: Pre-fault only required size of xstate buffer Andrei Vagin
2026-09-02 21:15   ` Chang S. Bae
2026-08-17  4:20 ` [PATCH 8/8] selftests/x86: Add a sigframe insufficient xstate_size test Andrei Vagin
2026-09-02 21:16   ` Chang S. Bae

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=f1e3cb3f-a76d-432f-8d32-9a48c0a8d111@intel.com \
    --to=chang.seok.bae@intel.com \
    --cc=alexander@mihalicyn.com \
    --cc=avagin@google.com \
    --cc=bp@alien8.de \
    --cc=criu@lists.linux.dev \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /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.