The attached patch addresses a problem with the current x86 pkey
implementation, which makes default-readable pkeys unusable from signal
handlers because the default init_pkru value blocks access.
With this patch, the following program:
#include <sys/syscall.h>
#include <unistd.h>
#include <stdio.h>
#include <err.h>
#include <signal.h>
#define PKEY_ALLOC_SETSIGNAL 1
#define PKEY_DISABLE_WRITE 2
static inline unsigned int
pkey_read (void)
{
unsigned int result;
__asm__ volatile (".byte 0x0f, 0x01, 0xee"
: "=a" (result) : "c" (0) : "rdx");
return result;
}
static void
print_pkru (const char *where)
{
printf ("PKRU (%s): %08x\n", where, pkey_read ());
}
static void
sigusr1 (int signo)
{
print_pkru ("signal handler");
}
int
main (void)
{
if (signal (SIGUSR1, sigusr1) == SIG_ERR)
err (1, "signal");
print_pkru ("main");
raise (SIGUSR1);
puts ("allocating key 1");
int key1 = syscall (SYS_pkey_alloc, 0, 0);
if (key1 < 0)
err (1, "pkey_alloc");
print_pkru ("main");
raise (SIGUSR1);
puts ("allocating key 2");
int key2 = syscall (SYS_pkey_alloc, PKEY_ALLOC_SETSIGNAL, 0);
if (key2 < 0)
err (1, "pkey_alloc");
print_pkru ("main");
raise (SIGUSR1);
puts ("allocating key 3");
int key3 = syscall (SYS_pkey_alloc, PKEY_ALLOC_SETSIGNAL,
PKEY_DISABLE_WRITE);
if (key3 < 0)
err (1, "pkey_alloc");
print_pkru ("main");
raise (SIGUSR1);
puts ("freeing key 3");
if (syscall (SYS_pkey_free, key3) < 0)
err (1, "pkey_free");
print_pkru ("main");
raise (SIGUSR1);
puts ("freeing key 2");
if (syscall (SYS_pkey_free, key2) < 0)
err (1, "pkey_free");
print_pkru ("main");
raise (SIGUSR1);
return 0;
}
prints this:
PKRU (main): 55555554
PKRU (signal handler): 55555554
allocating key 1
PKRU (main): 55555550
PKRU (signal handler): 55555554
allocating key 2
PKRU (main): 55555540
PKRU (signal handler): 55555544
allocating key 3
PKRU (main): 55555580
PKRU (signal handler): 55555584
freeing key 3
PKRU (main): 55555580
PKRU (signal handler): 55555544
freeing key 2
PKRU (main): 55555580
PKRU (signal handler): 55555554
Something like this is required before we can use memory protection keys
in glibc for mostly-read-only data structures which need to be
accessible from signal handlers.
I'm not sure if I got the locking for mm->context right. Please check
carefully.
Thanks,
Florian
commit 21ff3eaed8565e9b130380abf084e761f20e6fea
Author: Florian Weimer <fweimer@xxxxxxxxxx>
Date: Sat Dec 9 22:03:26 2017 +0100
pkeys: Support setting access rights for signal handlers
The new pkey_alloc flag PKEY_ALLOC_SETSIGNAL requests that the
kernel uses the specified access rights as the default when the
a signal handler is entered, instead of the access rights
specified by init_pkru.
This leads to a divergence in the FPU initialization for signal
handlers and for exeve, so a for_signal flag is added to fpu__clear.
Signed-off-by: Florian Weimer <fweimer@xxxxxxxxxx>
diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
index 2dbdf59258d9..343251fed6fc 100644
--- a/arch/alpha/include/uapi/asm/mman.h
+++ b/arch/alpha/include/uapi/asm/mman.h
@@ -72,6 +72,8 @@
/* compatibility flags */
#define MAP_FILE 0
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
index 606e02ca4b6c..be1677e31899 100644
--- a/arch/mips/include/uapi/asm/mman.h
+++ b/arch/mips/include/uapi/asm/mman.h
@@ -99,6 +99,8 @@
/* compatibility flags */
#define MAP_FILE 0
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
index 80510ba44c08..2fc9d1bf98eb 100644
--- a/arch/parisc/include/uapi/asm/mman.h
+++ b/arch/parisc/include/uapi/asm/mman.h
@@ -69,6 +69,8 @@
#define MAP_FILE 0
#define MAP_VARIABLE 0
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index a38bf5a1e37a..5a7a72dd589c 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -32,7 +32,7 @@ extern void fpu__restore(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 fpu *dst_fpu, struct fpu *src_fpu);
-extern void fpu__clear(struct fpu *fpu);
+extern void fpu__clear(struct fpu *fpu, bool for_signal);
extern int fpu__exception_code(struct fpu *fpu, int trap_nr);
extern int dump_fpu(struct pt_regs *ptregs, struct user_i387_struct *fpstate);
diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
index 9ea26f167497..0f041ed10c83 100644
--- a/arch/x86/include/asm/mmu.h
+++ b/arch/x86/include/asm/mmu.h
@@ -47,6 +47,13 @@ typedef struct {
*/
u16 pkey_allocation_map;
s16 execute_only_pkey;
+
+ /*
+ * Used to derive the PKRU register value from init_pkru in a
+ * signal handler.
+ */
+ u32 pkey_signal_mask;
+ u32 pkey_signal_value;
#endif
#ifdef CONFIG_X86_INTEL_MPX
/* address of the bounds directory */
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 6d16d15d09a0..d402496714ad 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -141,6 +141,7 @@ static inline int init_new_context(struct task_struct *tsk,
mm->context.pkey_allocation_map = 0x1;
/* -1 means unallocated or invalid */
mm->context.execute_only_pkey = -1;
+ mm->context.pkey_signal_mask = ~0U;
}
#endif
return init_new_context_ldt(tsk, mm);
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index a0ba1ffda0df..a9e20e4f6bb2 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -7,6 +7,10 @@
extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
+extern void arch_set_pkey_signal_default(struct mm_struct *mm,
+ int pkey, u32 rights);
+extern void arch_reset_pkey_signal_default(struct mm_struct *mm, int pkey);
+
/*
* Try to dedicate one of the protection keys to be used as an
* execute-only protection key.
@@ -104,6 +108,6 @@ extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
-extern void copy_init_pkru_to_fpregs(void);
+extern void copy_init_pkru_to_fpregs(bool for_signal);
#endif /*_ASM_X86_PKEYS_H */
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index f92a6593de1e..970e5b01ade4 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -362,7 +362,7 @@ void fpu__drop(struct fpu *fpu)
* Clear FPU registers by setting them up from
* the init fpstate:
*/
-static inline void copy_init_fpstate_to_fpregs(void)
+static inline void copy_init_fpstate_to_fpregs(bool for_signal)
{
if (use_xsave())
copy_kernel_to_xregs(&init_fpstate.xsave, -1);
@@ -372,7 +372,7 @@ static inline void copy_init_fpstate_to_fpregs(void)
copy_kernel_to_fregs(&init_fpstate.fsave);
if (boot_cpu_has(X86_FEATURE_OSPKE))
- copy_init_pkru_to_fpregs();
+ copy_init_pkru_to_fpregs(for_signal);
}
/*
@@ -381,7 +381,7 @@ static inline void copy_init_fpstate_to_fpregs(void)
* Called by sys_execve(), by the signal handler code and by various
* error paths.
*/
-void fpu__clear(struct fpu *fpu)
+void fpu__clear(struct fpu *fpu, bool for_signal)
{
WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an anomaly */
@@ -394,7 +394,7 @@ void fpu__clear(struct fpu *fpu)
preempt_disable();
fpu__initialize(fpu);
user_fpu_begin();
- copy_init_fpstate_to_fpregs();
+ copy_init_fpstate_to_fpregs(for_signal);
preempt_enable();
}
}
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 23f1691670b6..8e9709c1270b 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -277,7 +277,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
IS_ENABLED(CONFIG_IA32_EMULATION));
if (!buf) {
- fpu__clear(fpu);
+ fpu__clear(fpu, false);
return 0;
}
@@ -358,7 +358,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
*/
user_fpu_begin();
if (copy_user_to_fpregs_zeroing(buf_fx, xfeatures, fx_only)) {
- fpu__clear(fpu);
+ fpu__clear(fpu, false);
return -1;
}
}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index bb988a24db92..36d59d8c684a 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -129,7 +129,7 @@ void flush_thread(void)
flush_ptrace_hw_breakpoint(tsk);
memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));
- fpu__clear(&tsk->thread.fpu);
+ fpu__clear(&tsk->thread.fpu, false);
}
void disable_TSC(void)
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index b9e00e8f1c9b..86e7cf5d38e9 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -757,7 +757,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
* Ensure the signal handler starts with the new fpu state.
*/
if (fpu->initialized)
- fpu__clear(fpu);
+ fpu__clear(fpu, true);
}
signal_setup_done(failed, ksig, stepping);
}
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index d7bc0eea20a5..f7447512833c 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -19,6 +19,26 @@
#include <asm/cpufeature.h> /* boot_cpu_has, ... */
#include <asm/mmu_context.h> /* vma_pkey() */
+void arch_set_pkey_signal_default(struct mm_struct *mm, int pkey, u32 rights)
+{
+ int shift = pkey * PKRU_BITS_PER_PKEY;
+ u32 mask = ~(3U << shift);
+ u32 value = rights << shift;
+
+ mm->context.pkey_signal_mask &= mask;
+ mm->context.pkey_signal_value =
+ (mm->context.pkey_signal_value & mask) | value;
+}
+
+void arch_reset_pkey_signal_default(struct mm_struct *mm, int pkey)
+{
+ int shift = pkey * PKRU_BITS_PER_PKEY;
+ u32 mask = 3U << shift;
+
+ mm->context.pkey_signal_mask |= mask;
+ mm->context.pkey_signal_value &= ~mask;
+}
+
int __execute_only_pkey(struct mm_struct *mm)
{
bool need_to_set_mm_pkey = false;
@@ -142,21 +162,30 @@ u32 init_pkru_value = PKRU_AD_KEY( 1) | PKRU_AD_KEY( 2) | PKRU_AD_KEY( 3) |
* we know the FPU regstiers are safe for use and we can use PKRU
* directly.
*/
-void copy_init_pkru_to_fpregs(void)
+void copy_init_pkru_to_fpregs(bool for_signal)
{
u32 init_pkru_value_snapshot = READ_ONCE(init_pkru_value);
+ u32 desired_pkru;
+
+ if (for_signal)
+ desired_pkru = (init_pkru_value_snapshot
+ & current->mm->context.pkey_signal_mask)
+ | current->mm->context.pkey_signal_value;
+ else
+ desired_pkru = init_pkru_value_snapshot;
+
/*
* Any write to PKRU takes it out of the XSAVE 'init
* state' which increases context switch cost. Avoid
* writing 0 when PKRU was already 0.
*/
- if (!init_pkru_value_snapshot && !read_pkru())
+ if (!desired_pkru && !read_pkru())
return;
/*
* Override the PKRU state that came from 'init_fpstate'
* with the baseline from the process.
*/
- write_pkru(init_pkru_value_snapshot);
+ write_pkru(desired_pkru);
}
static ssize_t init_pkru_read_file(struct file *file, char __user *user_buf,
diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
index 3e9d01ada81f..5abb66526e3e 100644
--- a/arch/xtensa/include/uapi/asm/mman.h
+++ b/arch/xtensa/include/uapi/asm/mman.h
@@ -111,6 +111,8 @@
/* compatibility flags */
#define MAP_FILE 0
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 0794ca78c379..0ab73297b752 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -29,13 +29,23 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
return -EINVAL;
}
+static inline void arch_set_pkey_signal_default(struct mm_struct *mm,
+ int pkey, u32 rights)
+{
+}
+
+static inline void arch_reset_pkey_signal_default(struct mm_struct *mm,
+ int pkey)
+{
+}
+
static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val)
{
return 0;
}
-static inline void copy_init_pkru_to_fpregs(void)
+static inline void copy_init_pkru_to_fpregs(bool for_signal)
{
}
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index f8b134f5608f..78772266e3cd 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -66,6 +66,8 @@
/* compatibility flags */
#define MAP_FILE 0
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
diff --git a/mm/mprotect.c b/mm/mprotect.c
index ec39f730a0bf..021f1d465649 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -523,14 +523,17 @@ SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
return do_mprotect_pkey(start, len, prot, pkey);
}
+#define PKEY_ALLOC_FLAGS ((unsigned long) (PKEY_ALLOC_SETSIGNAL))
+
SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
{
int pkey;
int ret;
- /* No flags supported yet. */
- if (flags)
+ /* check for unsupported flags */
+ if (flags & ~PKEY_ALLOC_FLAGS)
return -EINVAL;
+
/* check for unsupported init values */
if (init_val & ~PKEY_ACCESS_MASK)
return -EINVAL;
@@ -547,6 +550,10 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
mm_pkey_free(current->mm, pkey);
goto out;
}
+
+ if (flags & PKEY_ALLOC_SETSIGNAL)
+ arch_set_pkey_signal_default(current->mm, pkey, init_val);
+
ret = pkey;
out:
up_write(¤t->mm->mmap_sem);
@@ -559,6 +566,8 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
down_write(¤t->mm->mmap_sem);
ret = mm_pkey_free(current->mm, pkey);
+ if (!ret)
+ arch_reset_pkey_signal_default(current->mm, pkey);
up_write(¤t->mm->mmap_sem);
/*
diff --git a/tools/include/uapi/asm-generic/mman-common.h b/tools/include/uapi/asm-generic/mman-common.h
index f8b134f5608f..78772266e3cd 100644
--- a/tools/include/uapi/asm-generic/mman-common.h
+++ b/tools/include/uapi/asm-generic/mman-common.h
@@ -66,6 +66,8 @@
/* compatibility flags */
#define MAP_FILE 0
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
diff --git a/tools/testing/selftests/x86/protection_keys.c b/tools/testing/selftests/x86/protection_keys.c
index bc1b0735bb50..ea161916e378 100644
--- a/tools/testing/selftests/x86/protection_keys.c
+++ b/tools/testing/selftests/x86/protection_keys.c
@@ -421,6 +421,8 @@ void dumpit(char *f)
close(fd);
}
+#define PKEY_ALLOC_SETSIGNAL 0x1
+
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
