lkml.org 
[lkml]   [2022]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [patch V4 09/65] x86/fpu: Sanitize xstateregs_set()
    From
    On 7/25/2022 2:26 PM, Dave Hansen wrote:
    >
    > Do you happen to have a quick reproducer for this, or at least the
    > contents of the buffer that you are trying to restore?

    While not following this report, I think there is a regression along
    with the changes:

    As looking into the spec, this state load does not depend on XSTATE_BV:

    RFBM := XCR0 AND EDX:EAX;
    COMPMASK := XCOMP_BV field from XSAVE header;

    IF COMPMASK[63] = 0
    THEN
    ...
    IF RFBM[1] = 1 OR RFBM[2] = 1
    THEN load MXCSR from legacy region of XSAVE area;
    FI;
    ...
    ELSE
    ...

    But our upstream code does reference XSTATE_BV instead of RFBM [1,2].

    My test case [3] fails with the upstream but works with 5.13, which is
    before the series. Then, this change looks to make it work at least for it:

    diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
    index c8340156bfd2..db4ab5c7ba8b 100644
    --- a/arch/x86/kernel/fpu/xstate.c
    +++ b/arch/x86/kernel/fpu/xstate.c
    @@ -1094,7 +1094,7 @@ void __copy_xstate_to_uabi_buf(struct membuf to,
    struct fpstate *fpstate,
    &xinit->i387, off_mxcsr);

    /* Copy MXCSR when SSE or YMM are set in the feature mask */
    - copy_feature(header.xfeatures & (XFEATURE_MASK_SSE |
    XFEATURE_MASK_YMM),
    + copy_feature(fpstate->user_xfeatures & (XFEATURE_MASK_SSE |
    XFEATURE_MASK_YMM),
    &to, &xsave->i387.mxcsr, &xinit->i387.mxcsr,
    MXCSR_AND_FLAGS_SIZE);

    @@ -1214,7 +1214,7 @@ static int copy_uabi_to_xstate(struct fpstate
    *fpstate, const void *kbuf,

    /* Validate MXCSR when any of the related features is in use */
    mask = XFEATURE_MASK_FP | XFEATURE_MASK_SSE | XFEATURE_MASK_YMM;
    - if (hdr.xfeatures & mask) {
    + if (fpstate->user_xfeatures & mask) {
    u32 mxcsr[2];

    offset = offsetof(struct fxregs_state, mxcsr);
    @@ -1226,7 +1226,7 @@ static int copy_uabi_to_xstate(struct fpstate
    *fpstate, const void *kbuf,
    return -EINVAL;

    /* SSE and YMM require MXCSR even when FP is not in
    use. */
    - if (!(hdr.xfeatures & XFEATURE_MASK_FP)) {
    + if (fpstate->user_xfeatures & (XFEATURE_MASK_SSE |
    XFEATURE_MASK_YMM)) {
    xsave->i387.mxcsr = mxcsr[0];
    xsave->i387.mxcsr_mask = mxcsr[1];
    }
    Thanks,
    Chang

    [1]
    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/fpu/xstate.c#n1097
    [2]
    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/fpu/xstate.c#n1217
    [3] test case:

    #include <err.h>
    #include <elf.h>
    #include <stdio.h>
    #include <string.h>
    #include <unistd.h>
    #include <x86intrin.h>

    #include <sys/ptrace.h>
    #include <sys/syscall.h>
    #include <sys/wait.h>
    #include <sys/uio.h>

    #define PAGE_SIZE 4096

    typedef uint8_t u8;
    typedef uint16_t u16;
    typedef uint32_t u32;
    typedef uint64_t u64;

    /* The below struct and define are copied from
    arch/x86/include/asm/fpu/types.h */
    struct fxregs_state {
    u16 cwd; /* Control Word */
    u16 swd; /* Status Word */
    u16 twd; /* Tag Word */
    u16 fop; /* Last Instruction Opcode */
    union {
    struct {
    u64 rip; /* Instruction Pointer */
    u64 rdp; /* Data Pointer */
    };
    struct {
    u32 fip; /* FPU IP Offset */
    u32 fcs; /* FPU IP Selector */
    u32 foo; /* FPU Operand Offset */
    u32 fos; /* FPU Operand Selector */
    };
    };
    u32 mxcsr; /* MXCSR Register State */
    u32 mxcsr_mask; /* MXCSR Mask */
    /* 8*16 bytes for each FP-reg = 128 bytes: */
    u32 st_space[32];
    /* 16*16 bytes for each XMM-reg = 256 bytes: */
    u32 xmm_space[64];
    u32 padding[12];

    union {
    u32 padding1[12];
    u32 sw_reserved[12];
    };
    } __attribute__((aligned(16)));
    struct xstate_header {
    u64 xfeatures;
    u64 xcomp_bv;
    u64 reserved[6];
    } __attribute__((packed));
    struct xregs_state {
    struct fxregs_state i387;
    struct xstate_header header;
    u8 extended_state_area[0];
    } __attribute__ ((packed, aligned (64)));
    union fpregs_state {
    struct xregs_state xsave;
    u8 __padding[PAGE_SIZE];
    };
    /*
    * List of XSAVE features Linux knows about:
    */
    enum xfeature {
    XFEATURE_FP,
    XFEATURE_SSE,
    };
    #define XFEATURE_MASK_SSE (1 << XFEATURE_SSE)

    /* Default value for fxregs_state.mxcsr: */
    #define MXCSR_DEFAULT 0x1f80
    void main(void)
    {
    union fpregs_state *xbuf;
    struct iovec iov;
    pid_t child;
    int status;
    u32 mxcsr;
    xbuf = aligned_alloc(64, sizeof(union fpregs_state));
    if (!xbuf)
    err(1, "aligned_alloc()");
    memset(xbuf, 0, sizeof(union fpregs_state));
    iov.iov_base = xbuf;
    iov.iov_len = sizeof(union fpregs_state);

    child = fork();
    if (!child) {
    if (ptrace(PTRACE_TRACEME, 0, NULL, NULL))
    err(1, "PTRACE_TRACEME");
    raise(SIGTRAP);
    _exit(0);
    }
    do {
    wait(&status);
    } while (WSTOPSIG(status) != SIGTRAP);
    printf("[RUN]\tCheck the default MXCSR state.\n");

    if (ptrace(PTRACE_GETREGSET, child, (uint32_t)NT_X86_XSTATE, &iov))
    err(1, "PTRACE_GETREGSET");
    printf("[%svalid init value.\n",
    (xbuf->xsave.i387.mxcsr == MXCSR_DEFAULT) ?
    "OK]\twith the " : "FAIL]\twith an in");

    printf("[RUN]\tTest MXCSR state write.\n");
    xbuf->xsave.i387.mxcsr = 0;
    /* the MXCSR state should be loaded regardless of XSTATE_BV */
    xbuf->xsave.header.xfeatures = 0;
    if (ptrace(PTRACE_SETREGSET, child, (uint32_t)NT_X86_XSTATE, &iov))
    err(1, "PTRACE_SETREGSET");
    /* ditch the MXCSR state */
    xbuf->xsave.i387.mxcsr = 0xff;
    xbuf->xsave.i387.mxcsr_mask = 0xff;
    if (ptrace(PTRACE_GETREGSET, child, (uint32_t)NT_X86_XSTATE, &iov))
    err(1, "PTRACE_GETREGSET");
    printf("[%s]\tthe state was %swritten correctly\n",
    !xbuf->xsave.i387.mxcsr ? "OK" : "FAIL",
    !xbuf->xsave.i387.mxcsr ? "" : "not ");

    ptrace(PTRACE_DETACH, child, NULL, NULL);
    wait(&status);
    if (!WIFEXITED(status) || WEXITSTATUS(status))
    err(1, "PTRACE_DETACH");
    free(xbuf);
    }
    \
     
     \ /
      Last update: 2022-07-29 01:32    [W:5.044 / U:1.616 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site