Re: [PATCH] Revert "x86/fpu: Refine and simplify the magic number check during signal return"

[Andrei Vagin]

On Wed, Apr 29, 2026 at 2:44 PM Chang S. Bae <chang.seok.bae@xxxxxxxxx> wrote:
>
> On 4/29/2026 1:44 PM, Andrei Vagin wrote:
> >
> > Enforcing validation against 'fpstate->user_size' instead of the frame's
> > own 'fx_sw->xstate_size' changes the kernel ABI, it isn't strengthen the
> > sanity check logic. When user-space supplies a valid, self-consistent
> > frame with an explicit size that older kernels accepted, and the updated
> > logic rejects it, which triggers a userspace regression.
> Sorry, I don't get your version of ABI.
>
> Eventually, XRSTOR will execute to restore the state. The kernel tracks
> each task's requested feature bitmap (RFBM), which determines the size.
> As describe SDM Vol.1, Section 13.13:
>
>     An execution of an instruction in the XSAVE feature set may access
>     any byte of any state component on which that execution operates even
>     when saving a state component is omitted ...
>
> Given this, the kernel must ensure the backing memory is valid and
> sufficient. So this consistency does matter.


We need to add one more paragraph to have the full context:

    Each instruction in the XSAVE feature set operates on a set of
    XSAVE-managed state components. The specific set of components on
    which an instruction operates is determined by the values of XCR0,
    the IA32_XSS MSR, EDX:EAX, and (for XRSTOR and XRSTORS) the XSAVE
    header.

    Section 13.4 provides the details necessary to determine the
    location of each state component for any execution of an
    instruction in the XSAVE feature set.  An execution of an
    instruction in the XSAVE feature set may access any byte of any
    state component on which that execution operates even when saving
    a state component is omitted because it is in its initial
    configuration; when restoring a state component to its initial
    configuration; or when XFD is enabled for the state components
    (see Section 13.14).

I interpret this to mean that XRSTOR will not access memory for a component
if its corresponding bit is clear in the XSAVE header.

However, my point was not about the CPU specification, but about the
kernel ABI. The reverted change broke existing user-space applications
without justifying an ABI regression. Even if xrstor were to trigger a
fault, the kernel handles it properly, so there is no real issue there.

It feels like we are trying to justify the change after the fact. The
rule is: "we don't break user-space". As usual, there are no rules
without exceptions, but any exception should be explicitly analyzed
considering all side effects.  According to the commit message of the
reverted commit, that wasn't such case.

Thanks,
Andrei