| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <37ba2de3-26b3-12eb-6a9d-c0f0572b832c@intel.com>
Date: Thu, 28 Jul 2022 16:32:13 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Dave Hansen <dave.hansen@...el.com>,
Andrei Vagin <avagin@...il.com>
CC: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>,
Andy Lutomirski <luto@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Tony Luck <tony.luck@...el.com>,
"Yu-cheng Yu" <yu-cheng.yu@...el.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Borislav Petkov <bp@...e.de>,
Peter Zijlstra <peterz@...radead.org>,
Kan Liang <kan.liang@...ux.intel.com>,
Megha Dey <megha.dey@...ux.intel.com>,
Oliver Sang <oliver.sang@...el.com>
Subject: Re: [patch V4 09/65] x86/fpu: Sanitize xstateregs_set()
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);
}
Powered by blists - more mailing lists