iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

* Clarify XSAVE consistency warnings

Browse Source 
* Fix up ptrace interface to protection keys register (PKRU)
  * Avoid undefined compiler behavior with TYPE_ALIGN
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCgAdFiEEV76QKkVc4xCGURexaDWVMHDJkrAFAmOXYisACgkQaDWVMHDJ
 krAJkA//QRChRwyKi1syinXt2SGoSa3mTzP23SyV0TunOfKBiBUreFJ2mMFjsX0h
 V7SJcu82sCWLHAY6LZRdyiF8zK3Cfzpbgb1QfzBCefE/gU801FhCypqNbQO5Lpdr
 PEo+naaDOzwDWDt0A6OkAArgb0zfaOGL+OBhuwT7mcUtBz6gCakFqG2BMgOzqD1z
 SAp0RraoSsFnKFl5Gv44+gkThq8/8yL5tyrJtnGv1jAsbhw9zmloaOue6MNMPJhH
 3sFQnML3qeNRozquWWeCPu/hxWuFDitPhwdmNRZrnQ3DyRdDhCZPOjv+tQmxI3EO
 5c+UIkMIsRh2nZLwHcM+iO5cWE7lyiAWpgqqArB+r2CFXWK5q2lplhXngBodE9Kr
 ki/NZ6oEitT3+bLXhCwyc7WKxohl2IlmclJ4AD3Qrp4bzPhfsZebL6nNs/3bxWuF
 CxJWIKzjtIcgNSEJaDOzFA5CAImq74r/kCW4e11ZXwmOnx6PX1YG6p0C1yknrZYJ
 bvy8WxureO7OJEcVZfwxpXLYbb+7Q/k/l2DkUdVAvKSCB81uWR4JzEp4oooDxf2j
 6x9qT5Mi95FhAHOCmlxwkQJTBCB36LkVF/3ESEOqJmun4F5ghPbMX2JzpBa6jPCS
 lzkBrzA8MAdmaLHhDO+nd5m8HVY3QBSXDVtRTycmuloeoSeyBno=
 =An0n
 -----END PGP SIGNATURE-----

Merge tag 'x86_fpu_for_6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip

Pull x86 fpu updates from Dave Hansen:
 "There are two little fixes in here, one to give better XSAVE warnings
  and another to address some undefined behavior in offsetof().

  There is also a collection of patches to fix some issues with ptrace
  and the protection keys register (PKRU). PKRU is a real oddity because
  it is exposed in the XSAVE-related ABIs, but it is generally managed
  without using XSAVE in the kernel. This fix thankfully came with a
  selftest to ward off future regressions.

  Summary:

   - Clarify XSAVE consistency warnings

   - Fix up ptrace interface to protection keys register (PKRU)

   - Avoid undefined compiler behavior with TYPE_ALIGN"

* tag 'x86_fpu_for_6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
  x86/fpu: Use _Alignof to avoid undefined behavior in TYPE_ALIGN
  selftests/vm/pkeys: Add a regression test for setting PKRU through ptrace
  x86/fpu: Emulate XRSTOR's behavior if the xfeatures PKRU bit is not set
  x86/fpu: Allow PKRU to be (once again) written by ptrace.
  x86/fpu: Add a pkru argument to copy_uabi_to_xstate()
  x86/fpu: Add a pkru argument to copy_uabi_from_kernel_to_xstate().
  x86/fpu: Take task_struct* in copy_sigframe_from_user_to_xstate()
  x86/fpu/xstate: Fix XSTATE_WARN_ON() to emit relevant diagnostics
This commit is contained in:
Linus Torvalds 2022-12-12 14:41:57 -08:00
commit 40deb5e41a
8 changed files with 208 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
arch/x86/kernel/fpu/core.c
View File

@ -391,8 +391,6 @@ int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
{
struct fpstate *kstate = gfpu->fpstate;
const union fpregs_state *ustate = buf;
struct pkru_state *xpkru;
int ret;
if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) {
if (ustate->xsave.header.xfeatures & ~XFEATURE_MASK_FPSSE)
@ -406,16 +404,15 @@ int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
if (ustate->xsave.header.xfeatures & ~xcr0)
return -EINVAL;
ret = copy_uabi_from_kernel_to_xstate(kstate, ustate);
if (ret)
return ret;
/*
* Nullify @vpkru to preserve its current value if PKRU's bit isn't set
* in the header. KVM's odd ABI is to leave PKRU untouched in this
* case (all other components are eventually re-initialized).
*/
if (!(ustate->xsave.header.xfeatures & XFEATURE_MASK_PKRU))
vpkru = NULL;
/* Retrieve PKRU if not in init state */
if (kstate->regs.xsave.header.xfeatures & XFEATURE_MASK_PKRU) {
xpkru = get_xsave_addr(&kstate->regs.xsave, XFEATURE_PKRU);
*vpkru = xpkru->pkru;
}
return 0;
return copy_uabi_from_kernel_to_xstate(kstate, ustate, vpkru);
}
EXPORT_SYMBOL_GPL(fpu_copy_uabi_to_guest_fpstate);
#endif /* CONFIG_KVM */

7
arch/x86/kernel/fpu/init.c
View File

@ -133,9 +133,6 @@ static void __init fpu__init_system_generic(void)
fpu__init_system_mxcsr();
}
/* Get alignment of the TYPE. */
#define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test)
/*
* Enforce that 'MEMBER' is the last field of 'TYPE'.
*
@ -143,8 +140,8 @@ static void __init fpu__init_system_generic(void)
* because that's how C aligns structs.
*/
#define CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) \
BUILD_BUG_ON(sizeof(TYPE) != ALIGN(offsetofend(TYPE, MEMBER), \
TYPE_ALIGN(TYPE)))
BUILD_BUG_ON(sizeof(TYPE) != \
ALIGN(offsetofend(TYPE, MEMBER), _Alignof(TYPE)))
/*
* We append the 'struct fpu' to the task_struct:

2
arch/x86/kernel/fpu/regset.c
View File

@ -167,7 +167,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
}
fpu_force_restore(fpu);
ret = copy_uabi_from_kernel_to_xstate(fpu->fpstate, kbuf ?: tmpbuf);
ret = copy_uabi_from_kernel_to_xstate(fpu->fpstate, kbuf ?: tmpbuf, &target->thread.pkru);
out:
vfree(tmpbuf);

2
arch/x86/kernel/fpu/signal.c
View File

@ -396,7 +396,7 @@ static bool __fpu_restore_sig(void __user *buf, void __user *buf_fx,
fpregs = &fpu->fpstate->regs;
if (use_xsave() && !fx_only) {
if (copy_sigframe_from_user_to_xstate(fpu->fpstate, buf_fx))
if (copy_sigframe_from_user_to_xstate(tsk, buf_fx))
return false;
} else {
if (__copy_from_user(&fpregs->fxsave, buf_fx,

64
arch/x86/kernel/fpu/xstate.c
View File

@ -440,8 +440,8 @@ static void __init __xstate_dump_leaves(void)
}
}
#define XSTATE_WARN_ON(x) do { \
if (WARN_ONCE(x, "XSAVE consistency problem, dumping leaves")) { \
#define XSTATE_WARN_ON(x, fmt, ...) do { \
if (WARN_ONCE(x, "XSAVE consistency problem: " fmt, ##__VA_ARGS__)) { \
__xstate_dump_leaves(); \
} \
} while (0)
@ -554,8 +554,7 @@ static bool __init check_xstate_against_struct(int nr)
(nr >= XFEATURE_MAX) ||
(nr == XFEATURE_PT_UNIMPLEMENTED_SO_FAR) ||
((nr >= XFEATURE_RSRVD_COMP_11) && (nr <= XFEATURE_RSRVD_COMP_16))) {
WARN_ONCE(1, "no structure for xstate: %d\n", nr);
XSTATE_WARN_ON(1);
XSTATE_WARN_ON(1, "No structure for xstate: %d\n", nr);
return false;
}
return true;
@ -598,12 +597,13 @@ static bool __init paranoid_xstate_size_valid(unsigned int kernel_size)
* XSAVES.
*/
if (!xsaves && xfeature_is_supervisor(i)) {
XSTATE_WARN_ON(1);
XSTATE_WARN_ON(1, "Got supervisor feature %d, but XSAVES not advertised\n", i);
return false;
}
}
size = xstate_calculate_size(fpu_kernel_cfg.max_features, compacted);
XSTATE_WARN_ON(size != kernel_size);
XSTATE_WARN_ON(size != kernel_size,
"size %u != kernel_size %u\n", size, kernel_size);
return size == kernel_size;
}
@ -1200,8 +1200,36 @@ static int copy_from_buffer(void *dst, unsigned int offset, unsigned int size,
}
/**
* copy_uabi_to_xstate - Copy a UABI format buffer to the kernel xstate
* @fpstate: The fpstate buffer to copy to
* @kbuf: The UABI format buffer, if it comes from the kernel
* @ubuf: The UABI format buffer, if it comes from userspace
* @pkru: The location to write the PKRU value to
*
* Converts from the UABI format into the kernel internal hardware
* dependent format.
*
* This function ultimately has three different callers with distinct PKRU
* behavior.
* 1. When called from sigreturn the PKRU register will be restored from
* @fpstate via an XRSTOR. Correctly copying the UABI format buffer to
* @fpstate is sufficient to cover this case, but the caller will also
* pass a pointer to the thread_struct's pkru field in @pkru and updating
* it is harmless.
* 2. When called from ptrace the PKRU register will be restored from the
* thread_struct's pkru field. A pointer to that is passed in @pkru.
* The kernel will restore it manually, so the XRSTOR behavior that resets
* the PKRU register to the hardware init value (0) if the corresponding
* xfeatures bit is not set is emulated here.
* 3. When called from KVM the PKRU register will be restored from the vcpu's
* pkru field. A pointer to that is passed in @pkru. KVM hasn't used
* XRSTOR and hasn't had the PKRU resetting behavior described above. To
* preserve that KVM behavior, it passes NULL for @pkru if the xfeatures
* bit is not set.
*/
static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
const void __user *ubuf)
const void __user *ubuf, u32 *pkru)
{
struct xregs_state *xsave = &fpstate->regs.xsave;
unsigned int offset, size;
@ -1250,6 +1278,20 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
}
}
if (hdr.xfeatures & XFEATURE_MASK_PKRU) {
struct pkru_state *xpkru;
xpkru = __raw_xsave_addr(xsave, XFEATURE_PKRU);
*pkru = xpkru->pkru;
} else {
/*
* KVM may pass NULL here to indicate that it does not need
* PKRU updated.
*/
if (pkru)
*pkru = 0;
}
/*
* The state that came in from userspace was user-state only.
* Mask all the user states out of 'xfeatures':
@ -1268,9 +1310,9 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
* Convert from a ptrace standard-format kernel buffer to kernel XSAVE[S]
* format and copy to the target thread. Used by ptrace and KVM.
*/
int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf)
int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf, u32 *pkru)
{
return copy_uabi_to_xstate(fpstate, kbuf, NULL);
return copy_uabi_to_xstate(fpstate, kbuf, NULL, pkru);
}
/*
@ -1278,10 +1320,10 @@ int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, 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_sigframe_from_user_to_xstate(struct fpstate *fpstate,
int copy_sigframe_from_user_to_xstate(struct task_struct *tsk,
const void __user *ubuf)
{
return copy_uabi_to_xstate(fpstate, NULL, ubuf);
return copy_uabi_to_xstate(tsk->thread.fpu.fpstate, NULL, ubuf, &tsk->thread.pkru);
}
static bool validate_independent_components(u64 mask)

4
arch/x86/kernel/fpu/xstate.h
View File

@ -46,8 +46,8 @@ extern void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate,
u32 pkru_val, enum xstate_copy_mode copy_mode);
extern void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk,
enum xstate_copy_mode mode);
extern int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf);
extern int copy_sigframe_from_user_to_xstate(struct fpstate *fpstate, const void __user *ubuf);
extern int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf, u32 *pkru);
extern int copy_sigframe_from_user_to_xstate(struct task_struct *tsk, const void __user *ubuf);
extern void fpu__init_cpu_xstate(void);

12
tools/testing/selftests/vm/pkey-x86.h
View File

@ -104,6 +104,18 @@ static inline int cpu_has_pkeys(void)
return 1;
}
static inline int cpu_max_xsave_size(void)
{
unsigned long XSTATE_CPUID = 0xd;
unsigned int eax;
unsigned int ebx;
unsigned int ecx;
unsigned int edx;
__cpuid_count(XSTATE_CPUID, 0, eax, ebx, ecx, edx);
return ecx;
}
static inline u32 pkey_bit_position(int pkey)
{
return pkey * PKEY_BITS_PER_PKEY;

131
tools/testing/selftests/vm/protection_keys.c
View File

@ -18,12 +18,13 @@
* do a plain mprotect() to a mprotect_pkey() area and make sure the pkey sticks
*
* Compile like this:
* gcc -o protection_keys -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
* gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
* gcc -mxsave -o protection_keys -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
* gcc -mxsave -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
*/
#define _GNU_SOURCE
#define __SANE_USERSPACE_TYPES__
#include <errno.h>
#include <linux/elf.h>
#include <linux/futex.h>
#include <time.h>
#include <sys/time.h>
@ -1550,6 +1551,129 @@ void test_implicit_mprotect_exec_only_memory(int *ptr, u16 pkey)
do_not_expect_pkey_fault("plain read on recently PROT_EXEC area");
}
#if defined(__i386__) || defined(__x86_64__)
void test_ptrace_modifies_pkru(int *ptr, u16 pkey)
{
u32 new_pkru;
pid_t child;
int status, ret;
int pkey_offset = pkey_reg_xstate_offset();
size_t xsave_size = cpu_max_xsave_size();
void *xsave;
u32 *pkey_register;
u64 *xstate_bv;
struct iovec iov;
new_pkru = ~read_pkey_reg();
/* Don't make PROT_EXEC mappings inaccessible */
new_pkru &= ~3;
child = fork();
pkey_assert(child >= 0);
dprintf3("[%d] fork() ret: %d\n", getpid(), child);
if (!child) {
ptrace(PTRACE_TRACEME, 0, 0, 0);
/* Stop and allow the tracer to modify PKRU directly */
raise(SIGSTOP);
/*
* need __read_pkey_reg() version so we do not do shadow_pkey_reg
* checking
*/
if (__read_pkey_reg() != new_pkru)
exit(1);
/* Stop and allow the tracer to clear XSTATE_BV for PKRU */
raise(SIGSTOP);
if (__read_pkey_reg() != 0)
exit(1);
/* Stop and allow the tracer to examine PKRU */
raise(SIGSTOP);
exit(0);
}
pkey_assert(child == waitpid(child, &status, 0));
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
xsave = (void *)malloc(xsave_size);
pkey_assert(xsave > 0);
/* Modify the PKRU register directly */
iov.iov_base = xsave;
iov.iov_len = xsave_size;
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
pkey_register = (u32 *)(xsave + pkey_offset);
pkey_assert(*pkey_register == read_pkey_reg());
*pkey_register = new_pkru;
ret = ptrace(PTRACE_SETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
/* Test that the modification is visible in ptrace before any execution */
memset(xsave, 0xCC, xsave_size);
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
pkey_assert(*pkey_register == new_pkru);
/* Execute the tracee */
ret = ptrace(PTRACE_CONT, child, 0, 0);
pkey_assert(ret == 0);
/* Test that the tracee saw the PKRU value change */
pkey_assert(child == waitpid(child, &status, 0));
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
/* Test that the modification is visible in ptrace after execution */
memset(xsave, 0xCC, xsave_size);
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
pkey_assert(*pkey_register == new_pkru);
/* Clear the PKRU bit from XSTATE_BV */
xstate_bv = (u64 *)(xsave + 512);
*xstate_bv &= ~(1 << 9);
ret = ptrace(PTRACE_SETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
/* Test that the modification is visible in ptrace before any execution */
memset(xsave, 0xCC, xsave_size);
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
pkey_assert(*pkey_register == 0);
ret = ptrace(PTRACE_CONT, child, 0, 0);
pkey_assert(ret == 0);
/* Test that the tracee saw the PKRU value go to 0 */
pkey_assert(child == waitpid(child, &status, 0));
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
/* Test that the modification is visible in ptrace after execution */
memset(xsave, 0xCC, xsave_size);
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
pkey_assert(ret == 0);
pkey_assert(*pkey_register == 0);
ret = ptrace(PTRACE_CONT, child, 0, 0);
pkey_assert(ret == 0);
pkey_assert(child == waitpid(child, &status, 0));
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
pkey_assert(WIFEXITED(status));
pkey_assert(WEXITSTATUS(status) == 0);
free(xsave);
}
#endif
void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
{
int size = PAGE_SIZE;
@ -1585,6 +1709,9 @@ void (*pkey_tests[])(int *ptr, u16 pkey) = {
test_pkey_syscalls_bad_args,
test_pkey_alloc_exhaust,
test_pkey_alloc_free_attach_pkey0,
#if defined(__i386__) || defined(__x86_64__)
test_ptrace_modifies_pkru,
#endif
};
void run_tests_once(void)