Skip to content

Commit 40deb5e

Browse files
committed
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
2 parents 1cab145 + 55228db commit 40deb5e

File tree

8 files changed

+208
-33
lines changed

8 files changed

+208
-33
lines changed

arch/x86/kernel/fpu/core.c

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -391,8 +391,6 @@ int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
391391
{
392392
struct fpstate *kstate = gfpu->fpstate;
393393
const union fpregs_state *ustate = buf;
394-
struct pkru_state *xpkru;
395-
int ret;
396394

397395
if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) {
398396
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,
406404
if (ustate->xsave.header.xfeatures & ~xcr0)
407405
return -EINVAL;
408406

409-
ret = copy_uabi_from_kernel_to_xstate(kstate, ustate);
410-
if (ret)
411-
return ret;
407+
/*
408+
* Nullify @vpkru to preserve its current value if PKRU's bit isn't set
409+
* in the header. KVM's odd ABI is to leave PKRU untouched in this
410+
* case (all other components are eventually re-initialized).
411+
*/
412+
if (!(ustate->xsave.header.xfeatures & XFEATURE_MASK_PKRU))
413+
vpkru = NULL;
412414

413-
/* Retrieve PKRU if not in init state */
414-
if (kstate->regs.xsave.header.xfeatures & XFEATURE_MASK_PKRU) {
415-
xpkru = get_xsave_addr(&kstate->regs.xsave, XFEATURE_PKRU);
416-
*vpkru = xpkru->pkru;
417-
}
418-
return 0;
415+
return copy_uabi_from_kernel_to_xstate(kstate, ustate, vpkru);
419416
}
420417
EXPORT_SYMBOL_GPL(fpu_copy_uabi_to_guest_fpstate);
421418
#endif /* CONFIG_KVM */

arch/x86/kernel/fpu/init.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,15 @@ static void __init fpu__init_system_generic(void)
133133
fpu__init_system_mxcsr();
134134
}
135135

136-
/* Get alignment of the TYPE. */
137-
#define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test)
138-
139136
/*
140137
* Enforce that 'MEMBER' is the last field of 'TYPE'.
141138
*
142139
* Align the computed size with alignment of the TYPE,
143140
* because that's how C aligns structs.
144141
*/
145142
#define CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) \
146-
BUILD_BUG_ON(sizeof(TYPE) != ALIGN(offsetofend(TYPE, MEMBER), \
147-
TYPE_ALIGN(TYPE)))
143+
BUILD_BUG_ON(sizeof(TYPE) != \
144+
ALIGN(offsetofend(TYPE, MEMBER), _Alignof(TYPE)))
148145

149146
/*
150147
* We append the 'struct fpu' to the task_struct:

arch/x86/kernel/fpu/regset.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
167167
}
168168

169169
fpu_force_restore(fpu);
170-
ret = copy_uabi_from_kernel_to_xstate(fpu->fpstate, kbuf ?: tmpbuf);
170+
ret = copy_uabi_from_kernel_to_xstate(fpu->fpstate, kbuf ?: tmpbuf, &target->thread.pkru);
171171

172172
out:
173173
vfree(tmpbuf);

arch/x86/kernel/fpu/signal.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ static bool __fpu_restore_sig(void __user *buf, void __user *buf_fx,
396396

397397
fpregs = &fpu->fpstate->regs;
398398
if (use_xsave() && !fx_only) {
399-
if (copy_sigframe_from_user_to_xstate(fpu->fpstate, buf_fx))
399+
if (copy_sigframe_from_user_to_xstate(tsk, buf_fx))
400400
return false;
401401
} else {
402402
if (__copy_from_user(&fpregs->fxsave, buf_fx,

arch/x86/kernel/fpu/xstate.c

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,8 @@ static void __init __xstate_dump_leaves(void)
440440
}
441441
}
442442

443-
#define XSTATE_WARN_ON(x) do { \
444-
if (WARN_ONCE(x, "XSAVE consistency problem, dumping leaves")) { \
443+
#define XSTATE_WARN_ON(x, fmt, ...) do { \
444+
if (WARN_ONCE(x, "XSAVE consistency problem: " fmt, ##__VA_ARGS__)) { \
445445
__xstate_dump_leaves(); \
446446
} \
447447
} while (0)
@@ -554,8 +554,7 @@ static bool __init check_xstate_against_struct(int nr)
554554
(nr >= XFEATURE_MAX) ||
555555
(nr == XFEATURE_PT_UNIMPLEMENTED_SO_FAR) ||
556556
((nr >= XFEATURE_RSRVD_COMP_11) && (nr <= XFEATURE_RSRVD_COMP_16))) {
557-
WARN_ONCE(1, "no structure for xstate: %d\n", nr);
558-
XSTATE_WARN_ON(1);
557+
XSTATE_WARN_ON(1, "No structure for xstate: %d\n", nr);
559558
return false;
560559
}
561560
return true;
@@ -598,12 +597,13 @@ static bool __init paranoid_xstate_size_valid(unsigned int kernel_size)
598597
* XSAVES.
599598
*/
600599
if (!xsaves && xfeature_is_supervisor(i)) {
601-
XSTATE_WARN_ON(1);
600+
XSTATE_WARN_ON(1, "Got supervisor feature %d, but XSAVES not advertised\n", i);
602601
return false;
603602
}
604603
}
605604
size = xstate_calculate_size(fpu_kernel_cfg.max_features, compacted);
606-
XSTATE_WARN_ON(size != kernel_size);
605+
XSTATE_WARN_ON(size != kernel_size,
606+
"size %u != kernel_size %u\n", size, kernel_size);
607607
return size == kernel_size;
608608
}
609609

@@ -1200,8 +1200,36 @@ static int copy_from_buffer(void *dst, unsigned int offset, unsigned int size,
12001200
}
12011201

12021202

1203+
/**
1204+
* copy_uabi_to_xstate - Copy a UABI format buffer to the kernel xstate
1205+
* @fpstate: The fpstate buffer to copy to
1206+
* @kbuf: The UABI format buffer, if it comes from the kernel
1207+
* @ubuf: The UABI format buffer, if it comes from userspace
1208+
* @pkru: The location to write the PKRU value to
1209+
*
1210+
* Converts from the UABI format into the kernel internal hardware
1211+
* dependent format.
1212+
*
1213+
* This function ultimately has three different callers with distinct PKRU
1214+
* behavior.
1215+
* 1. When called from sigreturn the PKRU register will be restored from
1216+
* @fpstate via an XRSTOR. Correctly copying the UABI format buffer to
1217+
* @fpstate is sufficient to cover this case, but the caller will also
1218+
* pass a pointer to the thread_struct's pkru field in @pkru and updating
1219+
* it is harmless.
1220+
* 2. When called from ptrace the PKRU register will be restored from the
1221+
* thread_struct's pkru field. A pointer to that is passed in @pkru.
1222+
* The kernel will restore it manually, so the XRSTOR behavior that resets
1223+
* the PKRU register to the hardware init value (0) if the corresponding
1224+
* xfeatures bit is not set is emulated here.
1225+
* 3. When called from KVM the PKRU register will be restored from the vcpu's
1226+
* pkru field. A pointer to that is passed in @pkru. KVM hasn't used
1227+
* XRSTOR and hasn't had the PKRU resetting behavior described above. To
1228+
* preserve that KVM behavior, it passes NULL for @pkru if the xfeatures
1229+
* bit is not set.
1230+
*/
12031231
static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
1204-
const void __user *ubuf)
1232+
const void __user *ubuf, u32 *pkru)
12051233
{
12061234
struct xregs_state *xsave = &fpstate->regs.xsave;
12071235
unsigned int offset, size;
@@ -1250,6 +1278,20 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
12501278
}
12511279
}
12521280

1281+
if (hdr.xfeatures & XFEATURE_MASK_PKRU) {
1282+
struct pkru_state *xpkru;
1283+
1284+
xpkru = __raw_xsave_addr(xsave, XFEATURE_PKRU);
1285+
*pkru = xpkru->pkru;
1286+
} else {
1287+
/*
1288+
* KVM may pass NULL here to indicate that it does not need
1289+
* PKRU updated.
1290+
*/
1291+
if (pkru)
1292+
*pkru = 0;
1293+
}
1294+
12531295
/*
12541296
* The state that came in from userspace was user-state only.
12551297
* Mask all the user states out of 'xfeatures':
@@ -1268,20 +1310,20 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
12681310
* Convert from a ptrace standard-format kernel buffer to kernel XSAVE[S]
12691311
* format and copy to the target thread. Used by ptrace and KVM.
12701312
*/
1271-
int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf)
1313+
int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf, u32 *pkru)
12721314
{
1273-
return copy_uabi_to_xstate(fpstate, kbuf, NULL);
1315+
return copy_uabi_to_xstate(fpstate, kbuf, NULL, pkru);
12741316
}
12751317

12761318
/*
12771319
* Convert from a sigreturn standard-format user-space buffer to kernel
12781320
* XSAVE[S] format and copy to the target thread. This is called from the
12791321
* sigreturn() and rt_sigreturn() system calls.
12801322
*/
1281-
int copy_sigframe_from_user_to_xstate(struct fpstate *fpstate,
1323+
int copy_sigframe_from_user_to_xstate(struct task_struct *tsk,
12821324
const void __user *ubuf)
12831325
{
1284-
return copy_uabi_to_xstate(fpstate, NULL, ubuf);
1326+
return copy_uabi_to_xstate(tsk->thread.fpu.fpstate, NULL, ubuf, &tsk->thread.pkru);
12851327
}
12861328

12871329
static bool validate_independent_components(u64 mask)

arch/x86/kernel/fpu/xstate.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ extern void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate,
4646
u32 pkru_val, enum xstate_copy_mode copy_mode);
4747
extern void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk,
4848
enum xstate_copy_mode mode);
49-
extern int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf);
50-
extern int copy_sigframe_from_user_to_xstate(struct fpstate *fpstate, const void __user *ubuf);
49+
extern int copy_uabi_from_kernel_to_xstate(struct fpstate *fpstate, const void *kbuf, u32 *pkru);
50+
extern int copy_sigframe_from_user_to_xstate(struct task_struct *tsk, const void __user *ubuf);
5151

5252

5353
extern void fpu__init_cpu_xstate(void);

tools/testing/selftests/vm/pkey-x86.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,18 @@ static inline int cpu_has_pkeys(void)
104104
return 1;
105105
}
106106

107+
static inline int cpu_max_xsave_size(void)
108+
{
109+
unsigned long XSTATE_CPUID = 0xd;
110+
unsigned int eax;
111+
unsigned int ebx;
112+
unsigned int ecx;
113+
unsigned int edx;
114+
115+
__cpuid_count(XSTATE_CPUID, 0, eax, ebx, ecx, edx);
116+
return ecx;
117+
}
118+
107119
static inline u32 pkey_bit_position(int pkey)
108120
{
109121
return pkey * PKEY_BITS_PER_PKEY;

tools/testing/selftests/vm/protection_keys.c

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818
* do a plain mprotect() to a mprotect_pkey() area and make sure the pkey sticks
1919
*
2020
* Compile like this:
21-
* gcc -o protection_keys -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
22-
* gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
21+
* gcc -mxsave -o protection_keys -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
22+
* gcc -mxsave -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm
2323
*/
2424
#define _GNU_SOURCE
2525
#define __SANE_USERSPACE_TYPES__
2626
#include <errno.h>
27+
#include <linux/elf.h>
2728
#include <linux/futex.h>
2829
#include <time.h>
2930
#include <sys/time.h>
@@ -1550,6 +1551,129 @@ void test_implicit_mprotect_exec_only_memory(int *ptr, u16 pkey)
15501551
do_not_expect_pkey_fault("plain read on recently PROT_EXEC area");
15511552
}
15521553

1554+
#if defined(__i386__) || defined(__x86_64__)
1555+
void test_ptrace_modifies_pkru(int *ptr, u16 pkey)
1556+
{
1557+
u32 new_pkru;
1558+
pid_t child;
1559+
int status, ret;
1560+
int pkey_offset = pkey_reg_xstate_offset();
1561+
size_t xsave_size = cpu_max_xsave_size();
1562+
void *xsave;
1563+
u32 *pkey_register;
1564+
u64 *xstate_bv;
1565+
struct iovec iov;
1566+
1567+
new_pkru = ~read_pkey_reg();
1568+
/* Don't make PROT_EXEC mappings inaccessible */
1569+
new_pkru &= ~3;
1570+
1571+
child = fork();
1572+
pkey_assert(child >= 0);
1573+
dprintf3("[%d] fork() ret: %d\n", getpid(), child);
1574+
if (!child) {
1575+
ptrace(PTRACE_TRACEME, 0, 0, 0);
1576+
/* Stop and allow the tracer to modify PKRU directly */
1577+
raise(SIGSTOP);
1578+
1579+
/*
1580+
* need __read_pkey_reg() version so we do not do shadow_pkey_reg
1581+
* checking
1582+
*/
1583+
if (__read_pkey_reg() != new_pkru)
1584+
exit(1);
1585+
1586+
/* Stop and allow the tracer to clear XSTATE_BV for PKRU */
1587+
raise(SIGSTOP);
1588+
1589+
if (__read_pkey_reg() != 0)
1590+
exit(1);
1591+
1592+
/* Stop and allow the tracer to examine PKRU */
1593+
raise(SIGSTOP);
1594+
1595+
exit(0);
1596+
}
1597+
1598+
pkey_assert(child == waitpid(child, &status, 0));
1599+
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
1600+
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
1601+
1602+
xsave = (void *)malloc(xsave_size);
1603+
pkey_assert(xsave > 0);
1604+
1605+
/* Modify the PKRU register directly */
1606+
iov.iov_base = xsave;
1607+
iov.iov_len = xsave_size;
1608+
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1609+
pkey_assert(ret == 0);
1610+
1611+
pkey_register = (u32 *)(xsave + pkey_offset);
1612+
pkey_assert(*pkey_register == read_pkey_reg());
1613+
1614+
*pkey_register = new_pkru;
1615+
1616+
ret = ptrace(PTRACE_SETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1617+
pkey_assert(ret == 0);
1618+
1619+
/* Test that the modification is visible in ptrace before any execution */
1620+
memset(xsave, 0xCC, xsave_size);
1621+
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1622+
pkey_assert(ret == 0);
1623+
pkey_assert(*pkey_register == new_pkru);
1624+
1625+
/* Execute the tracee */
1626+
ret = ptrace(PTRACE_CONT, child, 0, 0);
1627+
pkey_assert(ret == 0);
1628+
1629+
/* Test that the tracee saw the PKRU value change */
1630+
pkey_assert(child == waitpid(child, &status, 0));
1631+
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
1632+
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
1633+
1634+
/* Test that the modification is visible in ptrace after execution */
1635+
memset(xsave, 0xCC, xsave_size);
1636+
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1637+
pkey_assert(ret == 0);
1638+
pkey_assert(*pkey_register == new_pkru);
1639+
1640+
/* Clear the PKRU bit from XSTATE_BV */
1641+
xstate_bv = (u64 *)(xsave + 512);
1642+
*xstate_bv &= ~(1 << 9);
1643+
1644+
ret = ptrace(PTRACE_SETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1645+
pkey_assert(ret == 0);
1646+
1647+
/* Test that the modification is visible in ptrace before any execution */
1648+
memset(xsave, 0xCC, xsave_size);
1649+
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1650+
pkey_assert(ret == 0);
1651+
pkey_assert(*pkey_register == 0);
1652+
1653+
ret = ptrace(PTRACE_CONT, child, 0, 0);
1654+
pkey_assert(ret == 0);
1655+
1656+
/* Test that the tracee saw the PKRU value go to 0 */
1657+
pkey_assert(child == waitpid(child, &status, 0));
1658+
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
1659+
pkey_assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
1660+
1661+
/* Test that the modification is visible in ptrace after execution */
1662+
memset(xsave, 0xCC, xsave_size);
1663+
ret = ptrace(PTRACE_GETREGSET, child, (void *)NT_X86_XSTATE, &iov);
1664+
pkey_assert(ret == 0);
1665+
pkey_assert(*pkey_register == 0);
1666+
1667+
ret = ptrace(PTRACE_CONT, child, 0, 0);
1668+
pkey_assert(ret == 0);
1669+
pkey_assert(child == waitpid(child, &status, 0));
1670+
dprintf3("[%d] waitpid(%d) status: %x\n", getpid(), child, status);
1671+
pkey_assert(WIFEXITED(status));
1672+
pkey_assert(WEXITSTATUS(status) == 0);
1673+
free(xsave);
1674+
}
1675+
#endif
1676+
15531677
void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
15541678
{
15551679
int size = PAGE_SIZE;
@@ -1585,6 +1709,9 @@ void (*pkey_tests[])(int *ptr, u16 pkey) = {
15851709
test_pkey_syscalls_bad_args,
15861710
test_pkey_alloc_exhaust,
15871711
test_pkey_alloc_free_attach_pkey0,
1712+
#if defined(__i386__) || defined(__x86_64__)
1713+
test_ptrace_modifies_pkru,
1714+
#endif
15881715
};
15891716

15901717
void run_tests_once(void)

0 commit comments

Comments
 (0)