From 07d6688b22e09be465652cf2da0da6bf86154df6 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:35 +0200 Subject: x86/fpu: Limit xstate copy size in xstateregs_set() If the count argument is larger than the xstate size, this will happily copy beyond the end of xstate. Fixes: 91c3dba7dbc1 ("x86/fpu/xstate: Fix PTRACE frames for XSAVES") Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Andy Lutomirski Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.120741557@linutronix.de --- arch/x86/kernel/fpu/regset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index c413756ba89f..6bb874441de8 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -117,7 +117,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, /* * A whole standard-format XSAVE buffer is needed: */ - if ((pos != 0) || (count < fpu_user_xstate_size)) + if (pos != 0 || count != fpu_user_xstate_size) return -EFAULT; xsave = &fpu->state.xsave; -- cgit From 43be46e89698a41dbf4fff81a322f4c2ae21b5e2 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:36 +0200 Subject: x86/fpu: Sanitize xstateregs_set() xstateregs_set() operates on a stopped task and tries to copy the provided buffer into the task's fpu.state.xsave buffer. Any error while copying or invalid state detected after copying results in wiping the target task's FPU state completely including supervisor states. That's just wrong. The caller supplied invalid data or has a problem with unmapped memory, so there is absolutely no justification to corrupt the target state. Fix this with the following modifications: 1) If data has to be copied from userspace, allocate a buffer and copy from user first. 2) Use copy_kernel_to_xstate() unconditionally so that header checking works correctly. 3) Return on error without corrupting the target state. This prevents corrupting states and lets the caller deal with the problem it caused in the first place. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.214903673@linutronix.de --- arch/x86/include/asm/fpu/xstate.h | 4 ---- arch/x86/kernel/fpu/regset.c | 42 ++++++++++++++++----------------------- arch/x86/kernel/fpu/xstate.c | 14 +++++++------ 3 files changed, 25 insertions(+), 35 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index d22e973845c6..1bb2d16f485b 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -111,8 +111,4 @@ void copy_supervisor_to_kernel(struct xregs_state *xsave); void copy_dynamic_supervisor_to_kernel(struct xregs_state *xstate, u64 mask); void copy_kernel_to_dynamic_supervisor(struct xregs_state *xstate, u64 mask); - -/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */ -int validate_user_xstate_header(const struct xstate_header *hdr); - #endif diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 6bb874441de8..a50c0a935499 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -2,11 +2,13 @@ /* * FPU register's regset abstraction, for ptrace, core dumps, etc. */ +#include +#include + #include #include #include #include -#include /* * The xstateregs_active() routine is the same as the regset_fpregs_active() routine, @@ -108,10 +110,10 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, const void *kbuf, const void __user *ubuf) { struct fpu *fpu = &target->thread.fpu; - struct xregs_state *xsave; + struct xregs_state *tmpbuf = NULL; int ret; - if (!boot_cpu_has(X86_FEATURE_XSAVE)) + if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) return -ENODEV; /* @@ -120,32 +122,22 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, if (pos != 0 || count != fpu_user_xstate_size) return -EFAULT; - xsave = &fpu->state.xsave; - - fpu__prepare_write(fpu); + if (!kbuf) { + tmpbuf = vmalloc(count); + if (!tmpbuf) + return -ENOMEM; - if (using_compacted_format()) { - if (kbuf) - ret = copy_kernel_to_xstate(xsave, kbuf); - else - ret = copy_user_to_xstate(xsave, ubuf); - } else { - ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1); - if (!ret) - ret = validate_user_xstate_header(&xsave->header); + if (copy_from_user(tmpbuf, ubuf, count)) { + ret = -EFAULT; + goto out; + } } - /* - * mxcsr reserved bits must be masked to zero for security reasons. - */ - xsave->i387.mxcsr &= mxcsr_feature_mask; - - /* - * In case of failure, mark all states as init: - */ - if (ret) - fpstate_init(&fpu->state); + fpu__prepare_write(fpu); + ret = copy_kernel_to_xstate(&fpu->state.xsave, kbuf ?: tmpbuf); +out: + vfree(tmpbuf); return ret; } diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index b82879cd9151..2b7b579f06b6 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -543,7 +543,7 @@ int using_compacted_format(void) } /* Validate an xstate header supplied by userspace (ptrace or sigreturn) */ -int validate_user_xstate_header(const struct xstate_header *hdr) +static int validate_user_xstate_header(const struct xstate_header *hdr) { /* No unknown or supervisor features may be set */ if (hdr->xfeatures & ~xfeatures_mask_user()) @@ -1155,7 +1155,7 @@ void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave) } /* - * Convert from a ptrace standard-format kernel buffer to kernel XSAVES format + * Convert from a ptrace standard-format kernel buffer to kernel XSAVE[S] format * and copy to the target thread. This is called from xstateregs_set(). */ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) @@ -1202,14 +1202,16 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) */ xsave->header.xfeatures |= hdr.xfeatures; + /* mxcsr reserved bits must be masked to zero for historical reasons. */ + xsave->i387.mxcsr &= mxcsr_feature_mask; + return 0; } /* - * Convert from a ptrace or sigreturn standard-format user-space buffer to - * kernel XSAVES format and copy to the target thread. This is called from - * xstateregs_set(), as well as potentially from the sigreturn() and - * rt_sigreturn() system calls. + * Convert from a sigreturn standard-format user-space buffer to kernel + * XSAVE[S] format and copy to the target thread. This is called from the + * sigreturn() and rt_sigreturn() system calls. */ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf) { -- cgit From 3a3351126ee8f1f1c86c4c79c60a650c1da89733 Mon Sep 17 00:00:00 2001 From: Dave Hansen Date: Wed, 23 Jun 2021 14:01:38 +0200 Subject: x86/fpu: Simplify PTRACE_GETREGS code ptrace() has interfaces that let a ptracer inspect a ptracee's register state. This includes XSAVE state. The ptrace() ABI includes a hardware-format XSAVE buffer for both the SETREGS and GETREGS interfaces. In the old days, the kernel buffer and the ptrace() ABI buffer were the same boring non-compacted format. But, since the advent of supervisor states and the compacted format, the kernel buffer has diverged from the format presented in the ABI. This leads to two paths in the kernel: 1. Effectively a verbatim copy_to_user() which just copies the kernel buffer out to userspace. This is used when the kernel buffer is kept in the non-compacted form which means that it shares a format with the ptrace ABI. 2. A one-state-at-a-time path: copy_xstate_to_kernel(). This is theoretically slower since it does a bunch of piecemeal copies. Remove the verbatim copy case. Speed probably does not matter in this path, and the vast majority of new hardware will use the one-state-at-a-time path anyway. This ensures greater testing for the "slow" path. This also makes enabling PKRU in this interface easier since a single path can be patched instead of two. Signed-off-by: Dave Hansen Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Andy Lutomirski Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.408457100@linutronix.de --- arch/x86/kernel/fpu/regset.c | 24 +++--------------------- arch/x86/kernel/fpu/xstate.c | 6 +++--- 2 files changed, 6 insertions(+), 24 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index a50c0a935499..d60e77d39222 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -77,32 +77,14 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { struct fpu *fpu = &target->thread.fpu; - struct xregs_state *xsave; - if (!boot_cpu_has(X86_FEATURE_XSAVE)) + if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) return -ENODEV; - xsave = &fpu->state.xsave; - fpu__prepare_read(fpu); - if (using_compacted_format()) { - copy_xstate_to_kernel(to, xsave); - return 0; - } else { - fpstate_sanitize_xstate(fpu); - /* - * Copy the 48 bytes defined by the software into the xsave - * area in the thread struct, so that we can copy the whole - * area to user using one user_regset_copyout(). - */ - memcpy(&xsave->i387.sw_reserved, xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes)); - - /* - * Copy the xstate memory layout. - */ - return membuf_write(&to, xsave, fpu_user_xstate_size); - } + copy_xstate_to_kernel(to, &fpu->state.xsave); + return 0; } int xstateregs_set(struct task_struct *target, const struct user_regset *regset, diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 9cf84c5d1d37..42032471c957 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1069,11 +1069,11 @@ static void copy_feature(bool from_xstate, struct membuf *to, void *xstate, } /* - * Convert from kernel XSAVES compacted format to standard format and copy - * to a kernel-space ptrace buffer. + * Convert from kernel XSAVE or XSAVES compacted format to UABI + * non-compacted format and copy to a kernel-space ptrace buffer. * * It supports partial copy but pos always starts from zero. This is called - * from xstateregs_get() and there we check the CPU has XSAVES. + * from xstateregs_get() and there we check the CPU has XSAVE. */ void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave) { -- cgit From 6164331d15f7d912fb9369245368e9564ea49813 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Wed, 23 Jun 2021 14:01:39 +0200 Subject: x86/fpu: Rewrite xfpregs_set() xfpregs_set() was incomprehensible. Almost all of the complexity was due to trying to support nonsensically sized writes or -EFAULT errors that would have partially or completely overwritten the destination before failing. Nonsensically sized input would only have been possible using PTRACE_SETREGSET on REGSET_XFP. Fortunately, it appears (based on Debian code search results) that no one uses that API at all, let alone with the wrong sized buffer. Failed user access can be handled more cleanly by first copying to kernel memory. Just rewrite it to require sensible input. Signed-off-by: Andy Lutomirski Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.504234607@linutronix.de --- arch/x86/kernel/fpu/regset.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index d60e77d39222..f24ce873bfc2 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -47,30 +47,39 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset, const void *kbuf, const void __user *ubuf) { struct fpu *fpu = &target->thread.fpu; + struct user32_fxsr_struct newstate; int ret; - if (!boot_cpu_has(X86_FEATURE_FXSR)) + BUILD_BUG_ON(sizeof(newstate) != sizeof(struct fxregs_state)); + + if (!cpu_feature_enabled(X86_FEATURE_FXSR)) return -ENODEV; + /* No funny business with partial or oversized writes is permitted. */ + if (pos != 0 || count != sizeof(newstate)) + return -EINVAL; + + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &newstate, 0, -1); + if (ret) + return ret; + + /* Mask invalid MXCSR bits (for historical reasons). */ + newstate.mxcsr &= mxcsr_feature_mask; + fpu__prepare_write(fpu); - fpstate_sanitize_xstate(fpu); - ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, - &fpu->state.fxsave, 0, -1); + /* Copy the state */ + memcpy(&fpu->state.fxsave, &newstate, sizeof(newstate)); - /* - * mxcsr reserved bits must be masked to zero for security reasons. - */ - fpu->state.fxsave.mxcsr &= mxcsr_feature_mask; + /* Clear xmm8..15 */ + BUILD_BUG_ON(sizeof(fpu->state.fxsave.xmm_space) != 16 * 16); + memset(&fpu->state.fxsave.xmm_space[8], 0, 8 * 16); - /* - * update the header bits in the xsave header, indicating the - * presence of FP and SSE state. - */ - if (boot_cpu_has(X86_FEATURE_XSAVE)) + /* Mark FP and SSE as in use when XSAVE is enabled */ + if (use_xsave()) fpu->state.xsave.header.xfeatures |= XFEATURE_MASK_FPSSE; - return ret; + return 0; } int xstateregs_get(struct task_struct *target, const struct user_regset *regset, -- cgit From 145e9e0d8c6fada4a40f9fc65b34658077874d9c Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Wed, 23 Jun 2021 14:01:40 +0200 Subject: x86/fpu: Fail ptrace() requests that try to set invalid MXCSR values There is no benefit from accepting and silently changing an invalid MXCSR value supplied via ptrace(). Instead, return -EINVAL on invalid input. Signed-off-by: Andy Lutomirski Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.613614842@linutronix.de --- arch/x86/kernel/fpu/regset.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index f24ce873bfc2..5610f77cacad 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -63,8 +63,9 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset, if (ret) return ret; - /* Mask invalid MXCSR bits (for historical reasons). */ - newstate.mxcsr &= mxcsr_feature_mask; + /* Do not allow an invalid MXCSR value. */ + if (newstate.mxcsr & ~mxcsr_feature_mask) + return -EINVAL; fpu__prepare_write(fpu); -- cgit From da53f60bb86e60830932926cf1093953a811912c Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Wed, 23 Jun 2021 14:01:41 +0200 Subject: x86/fpu: Clean up fpregs_set() fpregs_set() has unnecessary complexity to support short or nonzero-offset writes and to handle the case in which a copy from userspace overwrites some of the target buffer and then fails. Support for partial writes is useless -- just require that the write has offset 0 and the correct size, and copy into a temporary kernel buffer to avoid clobbering the state if the user access fails. Signed-off-by: Andy Lutomirski Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.710467587@linutronix.de --- arch/x86/kernel/fpu/regset.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 5610f77cacad..7041b140de60 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -304,31 +304,32 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset, struct user_i387_ia32_struct env; int ret; - fpu__prepare_write(fpu); - fpstate_sanitize_xstate(fpu); + /* No funny business with partial or oversized writes is permitted. */ + if (pos != 0 || count != sizeof(struct user_i387_ia32_struct)) + return -EINVAL; - if (!boot_cpu_has(X86_FEATURE_FPU)) + if (!cpu_feature_enabled(X86_FEATURE_FPU)) return fpregs_soft_set(target, regset, pos, count, kbuf, ubuf); - if (!boot_cpu_has(X86_FEATURE_FXSR)) - return user_regset_copyin(&pos, &count, &kbuf, &ubuf, - &fpu->state.fsave, 0, - -1); + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1); + if (ret) + return ret; - if (pos > 0 || count < sizeof(env)) - convert_from_fxsr(&env, target); + fpu__prepare_write(fpu); - ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1); - if (!ret) - convert_to_fxsr(&target->thread.fpu.state.fxsave, &env); + if (cpu_feature_enabled(X86_FEATURE_FXSR)) + convert_to_fxsr(&fpu->state.fxsave, &env); + else + memcpy(&fpu->state.fsave, &env, sizeof(env)); /* - * update the header bit in the xsave header, indicating the + * Update the header bit in the xsave header, indicating the * presence of FP. */ - if (boot_cpu_has(X86_FEATURE_XSAVE)) + if (cpu_feature_enabled(X86_FEATURE_XSAVE)) fpu->state.xsave.header.xfeatures |= XFEATURE_MASK_FP; - return ret; + + return 0; } #endif /* CONFIG_X86_32 || CONFIG_IA32_EMULATION */ -- cgit From eb6f51723f03c9a1c098ed196a31a03e626b9fb6 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:42 +0200 Subject: x86/fpu: Make copy_xstate_to_kernel() usable for [x]fpregs_get() When xsave with init state optimization is used then a component's state in the task's xsave buffer can be stale when the corresponding feature bit is not set. fpregs_get() and xfpregs_get() invoke fpstate_sanitize_xstate() to update the task's xsave buffer before retrieving the FX or FP state. That's just duplicated code as copy_xstate_to_kernel() already handles this correctly. Add a copy mode argument to the function which allows to restrict the state copy to the FP and SSE features. Also rename the function to copy_xstate_to_uabi_buf() so the name reflects what it is doing. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.805327286@linutronix.de --- arch/x86/include/asm/fpu/xstate.h | 12 +++++++++-- arch/x86/kernel/fpu/regset.c | 2 +- arch/x86/kernel/fpu/xstate.c | 42 +++++++++++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 14 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index 1bb2d16f485b..732ae793c2ab 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -103,12 +103,20 @@ extern void __init update_regset_xstate_info(unsigned int size, void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr); int using_compacted_format(void); int xfeature_size(int xfeature_nr); -struct membuf; -void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave); int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf); int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf); void copy_supervisor_to_kernel(struct xregs_state *xsave); void copy_dynamic_supervisor_to_kernel(struct xregs_state *xstate, u64 mask); void copy_kernel_to_dynamic_supervisor(struct xregs_state *xstate, u64 mask); +enum xstate_copy_mode { + XSTATE_COPY_FP, + XSTATE_COPY_FX, + XSTATE_COPY_XSAVE, +}; + +struct membuf; +void copy_xstate_to_uabi_buf(struct membuf to, struct xregs_state *xsave, + enum xstate_copy_mode mode); + #endif diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 7041b140de60..783f84dfcd46 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -93,7 +93,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset, fpu__prepare_read(fpu); - copy_xstate_to_kernel(to, &fpu->state.xsave); + copy_xstate_to_uabi_buf(to, &fpu->state.xsave, XSTATE_COPY_XSAVE); return 0; } diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 42032471c957..8d023a26ae22 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1068,14 +1068,20 @@ static void copy_feature(bool from_xstate, struct membuf *to, void *xstate, membuf_write(to, from_xstate ? xstate : init_xstate, size); } -/* - * Convert from kernel XSAVE or XSAVES compacted format to UABI - * non-compacted format and copy to a kernel-space ptrace buffer. +/** + * copy_xstate_to_uabi_buf - Copy kernel saved xstate to a UABI buffer + * @to: membuf descriptor + * @xsave: The kernel xstate buffer to copy from + * @copy_mode: The requested copy mode + * + * Converts from kernel XSAVE or XSAVES compacted format to UABI conforming + * format, i.e. from the kernel internal hardware dependent storage format + * to the requested @mode. UABI XSTATE is always uncompacted! * - * It supports partial copy but pos always starts from zero. This is called - * from xstateregs_get() and there we check the CPU has XSAVE. + * It supports partial copy but @to.pos always starts from zero. */ -void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave) +void copy_xstate_to_uabi_buf(struct membuf to, struct xregs_state *xsave, + enum xstate_copy_mode copy_mode) { const unsigned int off_mxcsr = offsetof(struct fxregs_state, mxcsr); struct xregs_state *xinit = &init_fpstate.xsave; @@ -1083,12 +1089,22 @@ void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave) unsigned int zerofrom; int i; - /* - * The destination is a ptrace buffer; we put in only user xstates: - */ - memset(&header, 0, sizeof(header)); header.xfeatures = xsave->header.xfeatures; - header.xfeatures &= xfeatures_mask_user(); + + /* Mask out the feature bits depending on copy mode */ + switch (copy_mode) { + case XSTATE_COPY_FP: + header.xfeatures &= XFEATURE_MASK_FP; + break; + + case XSTATE_COPY_FX: + header.xfeatures &= XFEATURE_MASK_FP | XFEATURE_MASK_SSE; + break; + + case XSTATE_COPY_XSAVE: + header.xfeatures &= xfeatures_mask_user(); + break; + } /* Copy FP state up to MXCSR */ copy_feature(header.xfeatures & XFEATURE_MASK_FP, &to, &xsave->i387, @@ -1109,6 +1125,9 @@ void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave) &to, &xsave->i387.xmm_space, &xinit->i387.xmm_space, sizeof(xsave->i387.xmm_space)); + if (copy_mode != XSTATE_COPY_XSAVE) + goto out; + /* Zero the padding area */ membuf_zero(&to, sizeof(xsave->i387.padding)); @@ -1150,6 +1169,7 @@ void copy_xstate_to_kernel(struct membuf to, struct xregs_state *xsave) zerofrom = xstate_offsets[i] + xstate_sizes[i]; } +out: if (to.left) membuf_zero(&to, to.left); } -- cgit From adc997b3d66d1cfa8c15a7dbafdaef239a51b5db Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:43 +0200 Subject: x86/fpu: Use copy_xstate_to_uabi_buf() in xfpregs_get() Use the new functionality of copy_xstate_to_uabi_buf() to retrieve the FX state when XSAVE* is in use. This avoids overwriting the FPU state buffer with fpstate_sanitize_xstate() which is error prone and duplicated code. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121452.901736860@linutronix.de --- arch/x86/kernel/fpu/regset.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 783f84dfcd46..ccbe25f6627d 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -33,13 +33,18 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset, { struct fpu *fpu = &target->thread.fpu; - if (!boot_cpu_has(X86_FEATURE_FXSR)) + if (!cpu_feature_enabled(X86_FEATURE_FXSR)) return -ENODEV; fpu__prepare_read(fpu); - fpstate_sanitize_xstate(fpu); - return membuf_write(&to, &fpu->state.fxsave, sizeof(struct fxregs_state)); + if (!use_xsave()) { + return membuf_write(&to, &fpu->state.fxsave, + sizeof(fpu->state.fxsave)); + } + + copy_xstate_to_uabi_buf(to, &fpu->state.xsave, XSTATE_COPY_FX); + return 0; } int xfpregs_set(struct task_struct *target, const struct user_regset *regset, -- cgit From 3f7f75634ccefefcc929696f346db7a748e78f79 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:44 +0200 Subject: x86/fpu: Use copy_xstate_to_uabi_buf() in fpregs_get() Use the new functionality of copy_xstate_to_uabi_buf() to retrieve the FX state when XSAVE* is in use. This avoids to overwrite the FPU state buffer with fpstate_sanitize_xstate() which is error prone and duplicated code. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121453.014441775@linutronix.de --- arch/x86/kernel/fpu/regset.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index ccbe25f6627d..4b799e42ebc7 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -210,10 +210,10 @@ static inline u32 twd_fxsr_to_i387(struct fxregs_state *fxsave) * FXSR floating point environment conversions. */ -void -convert_from_fxsr(struct user_i387_ia32_struct *env, struct task_struct *tsk) +static void __convert_from_fxsr(struct user_i387_ia32_struct *env, + struct task_struct *tsk, + struct fxregs_state *fxsave) { - struct fxregs_state *fxsave = &tsk->thread.fpu.state.fxsave; struct _fpreg *to = (struct _fpreg *) &env->st_space[0]; struct _fpxreg *from = (struct _fpxreg *) &fxsave->st_space[0]; int i; @@ -247,6 +247,12 @@ convert_from_fxsr(struct user_i387_ia32_struct *env, struct task_struct *tsk) memcpy(&to[i], &from[i], sizeof(to[0])); } +void +convert_from_fxsr(struct user_i387_ia32_struct *env, struct task_struct *tsk) +{ + __convert_from_fxsr(env, tsk, &tsk->thread.fpu.state.fxsave); +} + void convert_to_fxsr(struct fxregs_state *fxsave, const struct user_i387_ia32_struct *env) @@ -279,25 +285,29 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset, { struct fpu *fpu = &target->thread.fpu; struct user_i387_ia32_struct env; + struct fxregs_state fxsave, *fx; fpu__prepare_read(fpu); - if (!boot_cpu_has(X86_FEATURE_FPU)) + if (!cpu_feature_enabled(X86_FEATURE_FPU)) return fpregs_soft_get(target, regset, to); - if (!boot_cpu_has(X86_FEATURE_FXSR)) { + if (!cpu_feature_enabled(X86_FEATURE_FXSR)) { return membuf_write(&to, &fpu->state.fsave, sizeof(struct fregs_state)); } - fpstate_sanitize_xstate(fpu); + if (use_xsave()) { + struct membuf mb = { .p = &fxsave, .left = sizeof(fxsave) }; - if (to.left == sizeof(env)) { - convert_from_fxsr(to.p, target); - return 0; + /* Handle init state optimized xstate correctly */ + copy_xstate_to_uabi_buf(mb, &fpu->state.xsave, XSTATE_COPY_FP); + fx = &fxsave; + } else { + fx = &fpu->state.fxsave; } - convert_from_fxsr(&env, target); + __convert_from_fxsr(&env, target, fx); return membuf_write(&to, &env, sizeof(env)); } -- cgit From 5a32fac8dbe8adc08c10e2c8770c95aebfc627cd Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:46 +0200 Subject: x86/fpu/regset: Move fpu__read_begin() into regset The function can only be used from the regset get() callbacks safely. So there is no reason to have it globally exposed. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121453.234942936@linutronix.de --- arch/x86/include/asm/fpu/internal.h | 1 - arch/x86/kernel/fpu/core.c | 20 -------------------- arch/x86/kernel/fpu/regset.c | 22 +++++++++++++++++++--- 3 files changed, 19 insertions(+), 24 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 0148791acc49..4c669f3b54b4 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -26,7 +26,6 @@ /* * High level FPU state handling functions: */ -extern void fpu__prepare_read(struct fpu *fpu); extern void fpu__prepare_write(struct fpu *fpu); extern void fpu__save(struct fpu *fpu); extern int fpu__restore_sig(void __user *buf, int ia32_frame); diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 9ff467bb4894..d390644e5fa1 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -281,26 +281,6 @@ static void fpu__initialize(struct fpu *fpu) trace_x86_fpu_init_state(fpu); } -/* - * This function must be called before we read a task's fpstate. - * - * There's two cases where this gets called: - * - * - for the current task (when coredumping), in which case we have - * to save the latest FPU registers into the fpstate, - * - * - or it's called for stopped tasks (ptrace), in which case the - * registers were already saved by the context-switch code when - * the task scheduled out. - * - * If the task has used the FPU before then save it. - */ -void fpu__prepare_read(struct fpu *fpu) -{ - if (fpu == ¤t->thread.fpu) - fpu__save(fpu); -} - /* * This function must be called before we write a task's fpstate. * diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 4b799e42ebc7..937adf702b37 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -28,6 +28,22 @@ int regset_xregset_fpregs_active(struct task_struct *target, const struct user_r return 0; } +/* + * The regset get() functions are invoked from: + * + * - coredump to dump the current task's fpstate. If the current task + * owns the FPU then the memory state has to be synchronized and the + * FPU register state preserved. Otherwise fpstate is already in sync. + * + * - ptrace to dump fpstate of a stopped task, in which case the registers + * have already been saved to fpstate on context switch. + */ +static void sync_fpstate(struct fpu *fpu) +{ + if (fpu == ¤t->thread.fpu) + fpu__save(fpu); +} + int xfpregs_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { @@ -36,7 +52,7 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset, if (!cpu_feature_enabled(X86_FEATURE_FXSR)) return -ENODEV; - fpu__prepare_read(fpu); + sync_fpstate(fpu); if (!use_xsave()) { return membuf_write(&to, &fpu->state.fxsave, @@ -96,7 +112,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset, if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) return -ENODEV; - fpu__prepare_read(fpu); + sync_fpstate(fpu); copy_xstate_to_uabi_buf(to, &fpu->state.xsave, XSTATE_COPY_XSAVE); return 0; @@ -287,7 +303,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset, struct user_i387_ia32_struct env; struct fxregs_state fxsave, *fx; - fpu__prepare_read(fpu); + sync_fpstate(fpu); if (!cpu_feature_enabled(X86_FEATURE_FPU)) return fpregs_soft_get(target, regset, to); -- cgit From dbb60ac764581e62f2116c5a6b8926ba3a872dd4 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:47 +0200 Subject: x86/fpu: Move fpu__write_begin() to regset The only usecase for fpu__write_begin is the set() callback of regset, so the function is pointlessly global. Move it to the regset code and rename it to fpu_force_restore() which is exactly decribing what the function does. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121453.328652975@linutronix.de --- arch/x86/include/asm/fpu/internal.h | 1 - arch/x86/kernel/fpu/core.c | 24 ------------------------ arch/x86/kernel/fpu/regset.c | 25 ++++++++++++++++++++++--- 3 files changed, 22 insertions(+), 28 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 4c669f3b54b4..854bfb03645e 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -26,7 +26,6 @@ /* * High level FPU state handling functions: */ -extern void fpu__prepare_write(struct fpu *fpu); extern void fpu__save(struct fpu *fpu); extern int fpu__restore_sig(void __user *buf, int ia32_frame); extern void fpu__drop(struct fpu *fpu); diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index d390644e5fa1..22437011f4f0 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -281,30 +281,6 @@ static void fpu__initialize(struct fpu *fpu) trace_x86_fpu_init_state(fpu); } -/* - * This function must be called before we write a task's fpstate. - * - * Invalidate any cached FPU registers. - * - * After this function call, after registers in the fpstate are - * modified and the child task has woken up, the child task will - * restore the modified FPU state from the modified context. If we - * didn't clear its cached status here then the cached in-registers - * state pending on its former CPU could be restored, corrupting - * the modifications. - */ -void fpu__prepare_write(struct fpu *fpu) -{ - /* - * Only stopped child tasks can be used to modify the FPU - * state in the fpstate buffer: - */ - WARN_ON_FPU(fpu == ¤t->thread.fpu); - - /* Invalidate any cached state: */ - __fpu_invalidate_fpregs_state(fpu); -} - /* * Drops current FPU state: deactivates the fpregs and * the fpstate. NOTE: it still leaves previous contents diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 937adf702b37..ddc290d9bf8e 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -44,6 +44,25 @@ static void sync_fpstate(struct fpu *fpu) fpu__save(fpu); } +/* + * Invalidate cached FPU registers before modifying the stopped target + * task's fpstate. + * + * This forces the target task on resume to restore the FPU registers from + * modified fpstate. Otherwise the task might skip the restore and operate + * with the cached FPU registers which discards the modifications. + */ +static void fpu_force_restore(struct fpu *fpu) +{ + /* + * Only stopped child tasks can be used to modify the FPU + * state in the fpstate buffer: + */ + WARN_ON_FPU(fpu == ¤t->thread.fpu); + + __fpu_invalidate_fpregs_state(fpu); +} + int xfpregs_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { @@ -88,7 +107,7 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset, if (newstate.mxcsr & ~mxcsr_feature_mask) return -EINVAL; - fpu__prepare_write(fpu); + fpu_force_restore(fpu); /* Copy the state */ memcpy(&fpu->state.fxsave, &newstate, sizeof(newstate)); @@ -146,7 +165,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, } } - fpu__prepare_write(fpu); + fpu_force_restore(fpu); ret = copy_kernel_to_xstate(&fpu->state.xsave, kbuf ?: tmpbuf); out: @@ -346,7 +365,7 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset, if (ret) return ret; - fpu__prepare_write(fpu); + fpu_force_restore(fpu); if (cpu_feature_enabled(X86_FEATURE_FXSR)) convert_to_fxsr(&fpu->state.fxsave, &env); -- cgit From 1cc34413ff3f18c30e5df89fefd95cc0f3b3292e Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:01:57 +0200 Subject: x86/fpu: Rename xstate copy functions which are related to UABI Rename them to reflect that these functions deal with user space format XSAVE buffers. copy_kernel_to_xstate() -> copy_uabi_from_kernel_to_xstate() copy_user_to_xstate() -> copy_sigframe_from_user_to_xstate() Again a clear statement that these functions deal with user space ABI. Suggested-by: Andy Lutomirski Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121454.318485015@linutronix.de --- arch/x86/include/asm/fpu/xstate.h | 4 ++-- arch/x86/kernel/fpu/regset.c | 2 +- arch/x86/kernel/fpu/signal.c | 2 +- arch/x86/kernel/fpu/xstate.c | 5 +++-- 4 files changed, 7 insertions(+), 6 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index 6611e069e834..00e1a2ac5239 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -102,8 +102,8 @@ extern void __init update_regset_xstate_info(unsigned int size, void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr); int xfeature_size(int xfeature_nr); -int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf); -int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf); +int copy_uabi_from_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf); +int copy_sigframe_from_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf); void copy_dynamic_supervisor_to_kernel(struct xregs_state *xstate, u64 mask); void copy_kernel_to_dynamic_supervisor(struct xregs_state *xstate, u64 mask); diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index ddc290d9bf8e..892aec1dd822 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -166,7 +166,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset, } fpu_force_restore(fpu); - ret = copy_kernel_to_xstate(&fpu->state.xsave, kbuf ?: tmpbuf); + ret = copy_uabi_from_kernel_to_xstate(&fpu->state.xsave, kbuf ?: tmpbuf); out: vfree(tmpbuf); diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c index 430c66dc2218..fd4b58d7b72e 100644 --- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -422,7 +422,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size) if (use_xsave() && !fx_only) { u64 init_bv = xfeatures_mask_user() & ~user_xfeatures; - ret = copy_user_to_xstate(&fpu->state.xsave, buf_fx); + ret = copy_sigframe_from_user_to_xstate(&fpu->state.xsave, buf_fx); if (ret) goto out; diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 4fca8a8edff8..57674f8c2342 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1099,7 +1099,7 @@ static inline bool mxcsr_valid(struct xstate_header *hdr, const u32 *mxcsr) * Convert from a ptrace standard-format kernel buffer to kernel XSAVE[S] format * and copy to the target thread. This is called from xstateregs_set(). */ -int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) +int copy_uabi_from_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) { unsigned int offset, size; int i; @@ -1154,7 +1154,8 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) * XSAVE[S] format and copy to the target thread. This is called from the * sigreturn() and rt_sigreturn() system calls. */ -int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf) +int copy_sigframe_from_user_to_xstate(struct xregs_state *xsave, + const void __user *ubuf) { unsigned int offset, size; int i; -- cgit From b2681e791dbcee6acb1dca7a5076a0285109ac4c Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 23 Jun 2021 14:02:06 +0200 Subject: x86/fpu: Rename and sanitize fpu__save/copy() Both function names are a misnomer. fpu__save() is actually about synchronizing the hardware register state into the task's memory state so that either coredump or a math exception handler can inspect the state at the time where the problem happens. The function guarantees to preserve the register state, while "save" is a common terminology for saving the current state so it can be modified and restored later. This is clearly not the case here. Rename it to fpu_sync_fpstate(). fpu__copy() is used to clone the current task's FPU state when duplicating task_struct. While the register state is a copy the rest of the FPU state is not. Name it accordingly and remove the really pointless @src argument along with the warning which comes along with it. Nothing can ever copy the FPU state of a non-current task. It's clearly just a consequence of arch_dup_task_struct(), but it makes no sense to proliferate that further. Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121455.196727450@linutronix.de --- arch/x86/include/asm/fpu/internal.h | 6 ++++-- arch/x86/kernel/fpu/core.c | 17 ++++++++--------- arch/x86/kernel/fpu/regset.c | 2 +- arch/x86/kernel/process.c | 3 +-- arch/x86/kernel/traps.c | 5 +++-- 5 files changed, 17 insertions(+), 16 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 3558cd05c62f..f5da2e9b87da 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -26,14 +26,16 @@ /* * High level FPU state handling functions: */ -extern void fpu__save(struct fpu *fpu); extern int fpu__restore_sig(void __user *buf, int ia32_frame); extern void fpu__drop(struct fpu *fpu); -extern int fpu__copy(struct task_struct *dst, struct task_struct *src); extern void fpu__clear_user_states(struct fpu *fpu); extern void fpu__clear_all(struct fpu *fpu); extern int fpu__exception_code(struct fpu *fpu, int trap_nr); +extern void fpu_sync_fpstate(struct fpu *fpu); + +extern int fpu_clone(struct task_struct *dst); + /* * Boot time FPU initialization functions: */ diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 4a59e0fbcfd8..8762b1a8966a 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -159,11 +159,10 @@ void kernel_fpu_end(void) EXPORT_SYMBOL_GPL(kernel_fpu_end); /* - * Save the FPU state (mark it for reload if necessary): - * - * This only ever gets called for the current task. + * Sync the FPU register state to current's memory register state when the + * current task owns the FPU. The hardware register state is preserved. */ -void fpu__save(struct fpu *fpu) +void fpu_sync_fpstate(struct fpu *fpu) { WARN_ON_FPU(fpu != ¤t->thread.fpu); @@ -221,18 +220,18 @@ void fpstate_init(union fpregs_state *state) } EXPORT_SYMBOL_GPL(fpstate_init); -int fpu__copy(struct task_struct *dst, struct task_struct *src) +/* Clone current's FPU state on fork */ +int fpu_clone(struct task_struct *dst) { + struct fpu *src_fpu = ¤t->thread.fpu; struct fpu *dst_fpu = &dst->thread.fpu; - struct fpu *src_fpu = &src->thread.fpu; + /* The new task's FPU state cannot be valid in the hardware. */ dst_fpu->last_cpu = -1; - if (!static_cpu_has(X86_FEATURE_FPU)) + if (!cpu_feature_enabled(X86_FEATURE_FPU)) return 0; - WARN_ON_FPU(src_fpu != ¤t->thread.fpu); - /* * Don't let 'init optimized' areas of the XSAVE area * leak into the child task: diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 892aec1dd822..4575796d547b 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -41,7 +41,7 @@ int regset_xregset_fpregs_active(struct task_struct *target, const struct user_r static void sync_fpstate(struct fpu *fpu) { if (fpu == ¤t->thread.fpu) - fpu__save(fpu); + fpu_sync_fpstate(fpu); } /* diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 5e1f38179f49..af3db530983b 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -87,8 +87,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) #ifdef CONFIG_VM86 dst->thread.vm86 = NULL; #endif - - return fpu__copy(dst, src); + return fpu_clone(dst); } /* diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 853ea7a80806..4c9c4aa83216 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -1046,9 +1046,10 @@ static void math_error(struct pt_regs *regs, int trapnr) } /* - * Save the info for the exception handler and clear the error. + * Synchronize the FPU register state to the memory register state + * if necessary. This allows the exception handler to inspect it. */ - fpu__save(fpu); + fpu_sync_fpstate(fpu); task->thread.trap_nr = trapnr; task->thread.error_code = 0; -- cgit From e84ba47e313dbc097bf859bb6e4f9219883d5f78 Mon Sep 17 00:00:00 2001 From: Dave Hansen Date: Wed, 23 Jun 2021 14:02:19 +0200 Subject: x86/fpu: Hook up PKRU into ptrace() One nice thing about having PKRU be XSAVE-managed is that it gets naturally exposed into the XSAVE-using ABIs. Now that XSAVE will not be used to manage PKRU, these ABIs need to be manually enabled to deal with PKRU. ptrace() uses copy_uabi_xstate_to_kernel() to collect the tracee's XSTATE. As PKRU is not in the task's XSTATE buffer, use task->thread.pkru for filling in up the ptrace buffer. Signed-off-by: Dave Hansen Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Reviewed-by: Borislav Petkov Link: https://lkml.kernel.org/r/20210623121456.508770763@linutronix.de --- arch/x86/include/asm/fpu/xstate.h | 2 +- arch/x86/kernel/fpu/regset.c | 10 ++++------ arch/x86/kernel/fpu/xstate.c | 25 ++++++++++++++++++------- 3 files changed, 23 insertions(+), 14 deletions(-) (limited to 'arch/x86/kernel/fpu/regset.c') diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h index 6a0aaafb93ec..4ff4a0093a48 100644 --- a/arch/x86/include/asm/fpu/xstate.h +++ b/arch/x86/include/asm/fpu/xstate.h @@ -139,7 +139,7 @@ enum xstate_copy_mode { }; struct membuf; -void copy_xstate_to_uabi_buf(struct membuf to, struct xregs_state *xsave, +void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk, enum xstate_copy_mode mode); #endif diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c index 4575796d547b..66ed317ebc0d 100644 --- a/arch/x86/kernel/fpu/regset.c +++ b/arch/x86/kernel/fpu/regset.c @@ -78,7 +78,7 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset, sizeof(fpu->state.fxsave)); } - copy_xstate_to_uabi_buf(to, &fpu->state.xsave, XSTATE_COPY_FX); + copy_xstate_to_uabi_buf(to, target, XSTATE_COPY_FX); return 0; } @@ -126,14 +126,12 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset, int xstateregs_get(struct task_struct *target, const struct user_regset *regset, struct membuf to) { - struct fpu *fpu = &target->thread.fpu; - if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) return -ENODEV; - sync_fpstate(fpu); + sync_fpstate(&target->thread.fpu); - copy_xstate_to_uabi_buf(to, &fpu->state.xsave, XSTATE_COPY_XSAVE); + copy_xstate_to_uabi_buf(to, target, XSTATE_COPY_XSAVE); return 0; } @@ -336,7 +334,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset, struct membuf mb = { .p = &fxsave, .left = sizeof(fxsave) }; /* Handle init state optimized xstate correctly */ - copy_xstate_to_uabi_buf(mb, &fpu->state.xsave, XSTATE_COPY_FP); + copy_xstate_to_uabi_buf(mb, target, XSTATE_COPY_FP); fx = &fxsave; } else { fx = &fpu->state.fxsave; diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index c513596f8ec9..9fd124a001b0 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -962,7 +962,7 @@ static void copy_feature(bool from_xstate, struct membuf *to, void *xstate, /** * copy_xstate_to_uabi_buf - Copy kernel saved xstate to a UABI buffer * @to: membuf descriptor - * @xsave: The kernel xstate buffer to copy from + * @tsk: The task from which to copy the saved xstate * @copy_mode: The requested copy mode * * Converts from kernel XSAVE or XSAVES compacted format to UABI conforming @@ -971,10 +971,11 @@ static void copy_feature(bool from_xstate, struct membuf *to, void *xstate, * * It supports partial copy but @to.pos always starts from zero. */ -void copy_xstate_to_uabi_buf(struct membuf to, struct xregs_state *xsave, +void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk, enum xstate_copy_mode copy_mode) { const unsigned int off_mxcsr = offsetof(struct fxregs_state, mxcsr); + struct xregs_state *xsave = &tsk->thread.fpu.state.xsave; struct xregs_state *xinit = &init_fpstate.xsave; struct xstate_header header; unsigned int zerofrom; @@ -1048,11 +1049,21 @@ void copy_xstate_to_uabi_buf(struct membuf to, struct xregs_state *xsave, if (zerofrom < xstate_offsets[i]) membuf_zero(&to, xstate_offsets[i] - zerofrom); - copy_feature(header.xfeatures & BIT_ULL(i), &to, - __raw_xsave_addr(xsave, i), - __raw_xsave_addr(xinit, i), - xstate_sizes[i]); - + if (i == XFEATURE_PKRU) { + struct pkru_state pkru = {0}; + /* + * PKRU is not necessarily up to date in the + * thread's XSAVE buffer. Fill this part from the + * per-thread storage. + */ + pkru.pkru = tsk->thread.pkru; + membuf_write(&to, &pkru, sizeof(pkru)); + } else { + copy_feature(header.xfeatures & BIT_ULL(i), &to, + __raw_xsave_addr(xsave, i), + __raw_xsave_addr(xinit, i), + xstate_sizes[i]); + } /* * Keep track of the last copied state in the non-compacted * target buffer for gap zeroing. -- cgit