* [RFC PATCH v3 0/2] MIPS non-executable stack support
@ 2016-06-29 14:38 ` Paul Burton
0 siblings, 0 replies; 22+ messages in thread
From: Paul Burton @ 2016-06-29 14:38 UTC (permalink / raw)
To: linux-mips, Ralf Baechle
Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham, Paul Burton
This series allows us to support non-executable stacks on systems with
RIXI by moving delay slot instruction emulation off of the user stack &
into a dedicated page.
This is a revision of patches 6114/6125 & 6115 from a few years back:
https://patchwork.linux-mips.org/patch/6114
https://patchwork.linux-mips.org/patch/6125
https://patchwork.linux-mips.org/patch/6115
The series applies atop v4.7-rc5, and is marked RFC as it could use a
little more testing before I'd be happy with it being merged.
Paul Burton (2):
MIPS: use per-mm page to execute branch delay slot instructions
MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present
arch/mips/Kconfig | 1 +
arch/mips/include/asm/elf.h | 5 +
arch/mips/include/asm/fpu_emulator.h | 17 +-
arch/mips/include/asm/mmu.h | 11 ++
arch/mips/include/asm/mmu_context.h | 7 +
arch/mips/include/asm/page.h | 6 +-
arch/mips/include/asm/processor.h | 18 +-
arch/mips/kernel/elf.c | 19 ++
arch/mips/kernel/mips-r2-to-r6-emul.c | 8 +-
arch/mips/kernel/process.c | 6 +
arch/mips/kernel/signal.c | 8 +
arch/mips/math-emu/cp1emu.c | 8 +-
arch/mips/math-emu/dsemul.c | 343 +++++++++++++++++++++++-----------
13 files changed, 322 insertions(+), 135 deletions(-)
--
2.9.0
^ permalink raw reply [flat|nested] 22+ messages in thread* [RFC PATCH v3 0/2] MIPS non-executable stack support @ 2016-06-29 14:38 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-29 14:38 UTC (permalink / raw) To: linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham, Paul Burton This series allows us to support non-executable stacks on systems with RIXI by moving delay slot instruction emulation off of the user stack & into a dedicated page. This is a revision of patches 6114/6125 & 6115 from a few years back: https://patchwork.linux-mips.org/patch/6114 https://patchwork.linux-mips.org/patch/6125 https://patchwork.linux-mips.org/patch/6115 The series applies atop v4.7-rc5, and is marked RFC as it could use a little more testing before I'd be happy with it being merged. Paul Burton (2): MIPS: use per-mm page to execute branch delay slot instructions MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present arch/mips/Kconfig | 1 + arch/mips/include/asm/elf.h | 5 + arch/mips/include/asm/fpu_emulator.h | 17 +- arch/mips/include/asm/mmu.h | 11 ++ arch/mips/include/asm/mmu_context.h | 7 + arch/mips/include/asm/page.h | 6 +- arch/mips/include/asm/processor.h | 18 +- arch/mips/kernel/elf.c | 19 ++ arch/mips/kernel/mips-r2-to-r6-emul.c | 8 +- arch/mips/kernel/process.c | 6 + arch/mips/kernel/signal.c | 8 + arch/mips/math-emu/cp1emu.c | 8 +- arch/mips/math-emu/dsemul.c | 343 +++++++++++++++++++++++----------- 13 files changed, 322 insertions(+), 135 deletions(-) -- 2.9.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-29 14:38 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-29 14:38 UTC (permalink / raw) To: linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham, Paul Burton In some cases the kernel needs to execute an instruction from the delay slot of an emulated branch instruction. These cases include: - Emulated floating point branch instructions (bc1[ft]l?) for systems which don't include an FPU, or upon which the kernel is run with the "nofpu" parameter. - MIPSr6 systems running binaries targeting older revisions of the architecture, which may include branch instructions whose encodings are no longer valid in MIPSr6. Executing instructions from such delay slots is done by writing the instruction to memory followed by a trap, as part of an "emuframe", and executing it. This avoids the requirement of an emulator for the entire MIPS instruction set. Prior to this patch such emuframes are written to the user stack and executed from there. This patch moves FP branch delay emuframes off of the user stack and into a per-mm page. Allocating a page per-mm leaves userland with access to only what it had access to previously, and compared to other solutions is relatively simple. When a thread requires a delay slot emulation, it is allocated a frame. A thread may only have one frame allocated at any one time, since it may only ever be executing one instruction at any one time. In order to ensure that we can free up allocated frame later, its index is recorded in struct thread_struct. In the typical case, after executing the delay slot instruction we'll execute a break instruction with the BRK_MEMU code. This traps back to the kernel & leads to a call to do_dsemulret which frees the allocated frame & moves the user PC back to the instruction that would have executed following the emulated branch. In some cases the delay slot instruction may be invalid, such as a branch, or may trigger an exception. In these cases the BRK_MEMU break instruction will not be hit. In order to ensure that frames are freed this patch introduces dsemul_thread_cleanup() and calls it to free any allocated frame upon thread exit. If the instruction generated an exception & leads to a signal being delivered to the thread, or indeed if a signal simply happens to be delivered to the thread whilst it is executing from the struct emuframe, then we need to take care to exit the frame appropriately. This is done by either rolling back the user PC to the branch or advancing it to the continuation PC prior to signal delivery, using dsemul_thread_rollback(). If this were not done then a sigreturn would return to the struct emuframe, and if that frame had meanwhile been used in response to an emulated branch instruction within the signal handler then we would execute the wrong user code. Whilst a user could theoretically place something like a compact branch to self in a delay slot and cause their thread to become stuck in an infinite loop with the frame never being deallocated, this would: - Only affect the users single process. - Be architecturally invalid since there would be a branch in the delay slot, which is forbidden. - Be extremely unlikely to happen by mistake, and provide a program with no more ability to harm the system than a simple infinite loop would. If a thread requires a delay slot emulation & no frame is available to it (ie. the process has enough other threads that all frames are currently in use) then the thread joins a waitqueue. It will sleep until a frame is freed by another thread in the process. Since we now know whether a thread has an allocated frame due to our tracking of its index, the cookie field of struct emuframe is removed as we can be more certain whether we have a valid frame. Since a thread may only ever have a single frame at any given time, the epc field of struct emuframe is also removed & the PC to continue from is instead stored in struct thread_struct. Together these changes simplify & shrink struct emuframe somewhat, allowing twice as many frames to fit into the page allocated for them. The primary benefit of this patch is that we are now free to mark the user stack non-executable where that is possible. Signed-off-by: Paul Burton <paul.burton@imgtec.com> --- v2 of this patch can be found here: https://patchwork.linux-mips.org/patch/6125/ This has become a higher priority than it was at the time of v2 since Android has begun marking its stacks non-executable & on MIPSr6 devices we use mips_dsemul() in the emulation of pre-r6 instructions. Since the Android NDK MIPS target is MIPS32, this is important to backwards compatibility for apps on MIPSr6 systems. Changes in v3: - Rebase atop v4.7-rc5. - Select CONFIG_HAVE_EXIT_THREAD. - Track allocated frames per thread, allowing cleanup on exit or signal delivery. - Remove signal blocking & thread information flag now we have other means to ensure frames are cleaned up. - Introduce asm/dsemul.h to group prototypes & definitions logically. - Avoid using 'fp' in names, since this isn't exclusive to FP branch emulation. - Document with kerneldoc. - Return a frame index from alloc_emuframe such that mips_dsemul can record it in struct thread_struct. - Use bool return type for do_dsemulret rather than a bool-like int. - Rework commit message. Changes in v2: - s/kernels/kernel's/ - Use (mm_)isBranchInstr in mips_dsemul rather than duplicating similar logic. arch/mips/Kconfig | 1 + arch/mips/include/asm/fpu_emulator.h | 17 +- arch/mips/include/asm/mmu.h | 11 ++ arch/mips/include/asm/mmu_context.h | 7 + arch/mips/include/asm/processor.h | 18 +- arch/mips/kernel/mips-r2-to-r6-emul.c | 8 +- arch/mips/kernel/process.c | 6 + arch/mips/kernel/signal.c | 8 + arch/mips/math-emu/cp1emu.c | 8 +- arch/mips/math-emu/dsemul.c | 343 +++++++++++++++++++++++----------- 10 files changed, 294 insertions(+), 133 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index ac91939..49a5396 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -64,6 +64,7 @@ config MIPS select GENERIC_TIME_VSYSCALL select ARCH_CLOCKSOURCE_DATA select HANDLE_DOMAIN_IRQ + select HAVE_EXIT_THREAD menu "Machine selection" diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h index 3225c3c..355dc25 100644 --- a/arch/mips/include/asm/fpu_emulator.h +++ b/arch/mips/include/asm/fpu_emulator.h @@ -24,7 +24,7 @@ #define _ASM_FPU_EMULATOR_H #include <linux/sched.h> -#include <asm/break.h> +#include <asm/dsemul.h> #include <asm/thread_info.h> #include <asm/inst.h> #include <asm/local.h> @@ -60,27 +60,16 @@ do { \ #define MIPS_FPU_EMU_INC_STATS(M) do { } while (0) #endif /* CONFIG_DEBUG_FS */ -extern int mips_dsemul(struct pt_regs *regs, mips_instruction ir, - unsigned long cpc); -extern int do_dsemulret(struct pt_regs *xcp); extern int fpu_emulator_cop1Handler(struct pt_regs *xcp, struct mips_fpu_struct *ctx, int has_fpu, void *__user *fault_addr); int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31); +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, + unsigned long *contpc); int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, unsigned long *contpc); -/* - * Instruction inserted following the badinst to further tag the sequence - */ -#define BD_COOKIE 0x0000bd36 /* tne $0, $0 with baggage */ - -/* - * Break instruction with special math emu break code set - */ -#define BREAK_MATH(micromips) (((micromips) ? 0x7 : 0xd) | (BRK_MEMU << 16)) - #define SIGNALLING_NAN 0x7ff800007ff80000LL static inline void fpu_emulator_init_fpu(void) diff --git a/arch/mips/include/asm/mmu.h b/arch/mips/include/asm/mmu.h index 1afa1f9..df6ec07 100644 --- a/arch/mips/include/asm/mmu.h +++ b/arch/mips/include/asm/mmu.h @@ -2,11 +2,22 @@ #define __ASM_MMU_H #include <linux/atomic.h> +#include <linux/mutex.h> +#include <linux/wait.h> typedef struct { unsigned long asid[NR_CPUS]; void *vdso; atomic_t fp_mode_switching; + + /* address of page used to hold FP branch delay emulation frames */ + unsigned long bd_emupage; + /* bitmap tracking allocation of fp_bd_emupage */ + unsigned long *bd_emupage_allocmap; + /* mutex to be held whilst modifying fp_bd_emupage(_allocmap) */ + struct mutex bd_emupage_mutex; + /* wait queue for threads requiring an emuframe */ + wait_queue_head_t bd_emupage_queue; } mm_context_t; #endif /* __ASM_MMU_H */ diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h index fc57e13..174d68a 100644 --- a/arch/mips/include/asm/mmu_context.h +++ b/arch/mips/include/asm/mmu_context.h @@ -16,6 +16,7 @@ #include <linux/smp.h> #include <linux/slab.h> #include <asm/cacheflush.h> +#include <asm/dsemul.h> #include <asm/hazards.h> #include <asm/tlbflush.h> #include <asm-generic/mm_hooks.h> @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct mm_struct *mm) atomic_set(&mm->context.fp_mode_switching, 0); + mm->context.bd_emupage = 0; + mm->context.bd_emupage_allocmap = NULL; + mutex_init(&mm->context.bd_emupage_mutex); + init_waitqueue_head(&mm->context.bd_emupage_queue); + return 0; } @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, */ static inline void destroy_context(struct mm_struct *mm) { + dsemul_mm_cleanup(mm); } #define deactivate_mm(tsk, mm) do { } while (0) diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h index 7e78b62..0d36c87 100644 --- a/arch/mips/include/asm/processor.h +++ b/arch/mips/include/asm/processor.h @@ -11,12 +11,14 @@ #ifndef _ASM_PROCESSOR_H #define _ASM_PROCESSOR_H +#include <linux/atomic.h> #include <linux/cpumask.h> #include <linux/threads.h> #include <asm/cachectl.h> #include <asm/cpu.h> #include <asm/cpu-info.h> +#include <asm/dsemul.h> #include <asm/mipsregs.h> #include <asm/prefetch.h> @@ -78,7 +80,11 @@ extern unsigned int vced_count, vcei_count; #endif -#define STACK_TOP (TASK_SIZE & PAGE_MASK) +/* + * One page above the stack is used for branch delay slot "emulation". + * See dsemul.c for details. + */ +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) /* * This decides where the kernel will search for a free chunk of vm @@ -256,6 +262,12 @@ struct thread_struct { /* Saved fpu/fpu emulator stuff. */ struct mips_fpu_struct fpu FPU_ALIGN; + /* Assigned branch delay slot 'emulation' frame */ + atomic_t bd_emu_frame; + /* PC of the branch from a branch delay slot 'emulation' */ + unsigned long bd_emu_branch_pc; + /* PC to continue from following a branch delay slot 'emulation' */ + unsigned long bd_emu_cont_pc; #ifdef CONFIG_MIPS_MT_FPAFF /* Emulated instruction count */ unsigned long emulated_fp; @@ -323,6 +335,10 @@ struct thread_struct { * FPU affinity state (null if not FPAFF) \ */ \ FPAFF_INIT \ + /* Delay slot emulation */ \ + .bd_emu_frame = ATOMIC_INIT(BD_EMUFRAME_NONE), \ + .bd_emu_branch_pc = 0, \ + .bd_emu_cont_pc = 0, \ /* \ * Saved DSP stuff \ */ \ diff --git a/arch/mips/kernel/mips-r2-to-r6-emul.c b/arch/mips/kernel/mips-r2-to-r6-emul.c index 7ff2a55..ef23c61 100644 --- a/arch/mips/kernel/mips-r2-to-r6-emul.c +++ b/arch/mips/kernel/mips-r2-to-r6-emul.c @@ -283,7 +283,7 @@ static int jr_func(struct pt_regs *regs, u32 ir) err = mipsr6_emul(regs, nir); if (err > 0) { regs->cp0_epc = nepc; - err = mips_dsemul(regs, nir, cepc); + err = mips_dsemul(regs, nir, epc, cepc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); @@ -1033,7 +1033,7 @@ repeat: if (nir) { err = mipsr6_emul(regs, nir); if (err > 0) { - err = mips_dsemul(regs, nir, cpc); + err = mips_dsemul(regs, nir, epc, cpc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); @@ -1082,7 +1082,7 @@ repeat: if (nir) { err = mipsr6_emul(regs, nir); if (err > 0) { - err = mips_dsemul(regs, nir, cpc); + err = mips_dsemul(regs, nir, epc, cpc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); @@ -1149,7 +1149,7 @@ repeat: if (nir) { err = mipsr6_emul(regs, nir); if (err > 0) { - err = mips_dsemul(regs, nir, cpc); + err = mips_dsemul(regs, nir, epc, cpc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 813ed78..0fb1e8c 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -30,6 +30,7 @@ #include <asm/asm.h> #include <asm/bootinfo.h> #include <asm/cpu.h> +#include <asm/dsemul.h> #include <asm/dsp.h> #include <asm/fpu.h> #include <asm/msa.h> @@ -73,6 +74,11 @@ void start_thread(struct pt_regs * regs, unsigned long pc, unsigned long sp) regs->regs[29] = sp; } +void exit_thread(struct task_struct *tsk) +{ + dsemul_thread_cleanup(); +} + int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) { /* diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c index ae42314..9383635 100644 --- a/arch/mips/kernel/signal.c +++ b/arch/mips/kernel/signal.c @@ -772,6 +772,14 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) struct mips_abi *abi = current->thread.abi; void *vdso = current->mm->context.vdso; + /* + * If we were emulating a delay slot instruction, exit that frame such + * that addresses in the sigframe are as expected for userland and we + * don't have a problem if we reuse the thread's frame for an + * instruction within the signal handler. + */ + dsemul_thread_rollback(regs); + if (regs->regs[0]) { switch(regs->regs[2]) { case ERESTART_RESTARTBLOCK: diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c index d96e912..8afa090 100644 --- a/arch/mips/math-emu/cp1emu.c +++ b/arch/mips/math-emu/cp1emu.c @@ -434,8 +434,8 @@ static int microMIPS32_to_MIPS32(union mips_instruction *insn_ptr) * a single subroutine should be used across both * modules. */ -static int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, - unsigned long *contpc) +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, + unsigned long *contpc) { union mips_instruction insn = (union mips_instruction)dec_insn.insn; unsigned int fcr31; @@ -1268,7 +1268,7 @@ branch_common: * instruction in the dslot. */ sig = mips_dsemul(xcp, ir, - contpc); + bcpc, contpc); if (sig < 0) break; if (sig) @@ -1323,7 +1323,7 @@ branch_common: * Single step the non-cp1 * instruction in the dslot */ - sig = mips_dsemul(xcp, ir, contpc); + sig = mips_dsemul(xcp, ir, bcpc, contpc); if (sig < 0) break; if (sig) diff --git a/arch/mips/math-emu/dsemul.c b/arch/mips/math-emu/dsemul.c index 4707488..d21af2f 100644 --- a/arch/mips/math-emu/dsemul.c +++ b/arch/mips/math-emu/dsemul.c @@ -1,3 +1,6 @@ +#include <linux/err.h> +#include <linux/slab.h> + #include <asm/branch.h> #include <asm/cacheflush.h> #include <asm/fpu_emulator.h> @@ -5,43 +8,221 @@ #include <asm/mipsregs.h> #include <asm/uaccess.h> -#include "ieee754.h" - -/* - * Emulate the arbitrary instruction ir at xcp->cp0_epc. Required when - * we have to emulate the instruction in a COP1 branch delay slot. Do - * not change cp0_epc due to the instruction +/** + * struct emuframe - The 'emulation' frame structure + * @emul: The instruction to 'emulate'. + * @badinst: A break instruction to cause a return to the kernel. * - * According to the spec: - * 1) it shouldn't be a branch :-) - * 2) it can be a COP instruction :-( - * 3) if we are tring to run a protected memory space we must take - * special care on memory access instructions :-( - */ - -/* - * "Trampoline" return routine to catch exception following - * execution of delay-slot instruction execution. + * This structure defines the frames placed within the delay slot emulation + * page in response to a call to mips_dsemul(). Each thread may be allocated + * only one frame at any given time. The kernel stores within it the + * instruction to be 'emulated' followed by a break instruction, then + * executes the frame in user mode. The break causes a trap to the kernel + * which leads to do_dsemulret() being called unless the instruction in + * @emul causes a trap itself, is a branch, or a signal is delivered to + * the thread. In these cases the allocated frame will either be reused by + * a subsequent delay slot 'emulation', or be freed during signal delivery or + * upon thread exit. + * + * This approach is used because: + * + * - Actually emulating all instructions isn't feasible. We would need to + * be able to handle instructions from all revisions of the MIPS ISA, + * all ASEs & all vendor instruction set extensions. This would be a + * whole lot of work & continual maintenance burden as new instructions + * are introduced, and in the case of some vendor extensions may not + * even be possible. Thus we need to take the approach of actually + * executing the instruction. + * + * - We must execute the instruction within user context. If we were to + * execute the instruction in kernel mode then it would have access to + * kernel resources without very careful checks, leaving us with a + * high potential for security or stability issues to arise. + * + * - We used to place the frame on the users stack, but this requires + * that the stack be executable. This is bad for security so the + * per-process page is now used instead. + * + * - The instruction in @emul may be something entirely invalid for a + * delay slot. The user may (intentionally or otherwise) place a branch + * in a delay slot, or a kernel mode instruction, or something else + * which generates an exception. Thus we can't rely upon the break in + * @badinst always being hit. For this reason we track the index of the + * frame allocated to each thread, allowing us to clean it up at later + * points such as signal delivery or thread exit. + * + * - The user may generate a fake struct emuframe if they wish, invoking + * the BRK_MEMU break instruction themselves. We must therefore not + * trust that BRK_MEMU means there's actually a valid frame allocated + * to the thread, and must not allow the user to do anything they + * couldn't already. */ - struct emuframe { mips_instruction emul; mips_instruction badinst; - mips_instruction cookie; - unsigned long epc; }; -/* - * Set up an emulation frame for instruction IR, from a delay slot of - * a branch jumping to CPC. Return 0 if successful, -1 if no emulation - * required, otherwise a signal number causing a frame setup failure. - */ -int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) +static const int emupage_frame_count = PAGE_SIZE / sizeof(struct emuframe); + +static int alloc_emuframe(void) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + unsigned long addr; + int idx; + +retry: + mutex_lock(&mm_ctx->bd_emupage_mutex); + + /* Ensure we have a page allocated for emuframes */ + if (!mm_ctx->bd_emupage) { + addr = mmap_region(NULL, STACK_TOP, PAGE_SIZE, + VM_READ|VM_WRITE|VM_EXEC| + VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, + 0); + if (IS_ERR_VALUE(addr)) { + idx = BD_EMUFRAME_NONE; + goto out_unlock; + } + + mm_ctx->bd_emupage = addr; + pr_debug("allocate emupage at 0x%08lx to %d\n", addr, + current->pid); + } + + /* Ensure we have an allocation bitmap */ + if (!mm_ctx->bd_emupage_allocmap) { + mm_ctx->bd_emupage_allocmap = + kcalloc(BITS_TO_LONGS(emupage_frame_count), + sizeof(unsigned long), + GFP_KERNEL); + + if (!mm_ctx->bd_emupage_allocmap) { + idx = BD_EMUFRAME_NONE; + goto out_unlock; + } + } + + /* Attempt to allocate a single bit/frame */ + idx = bitmap_find_free_region(mm_ctx->bd_emupage_allocmap, + emupage_frame_count, 0); + if (idx < 0) { + /* + * Failed to allocate a frame. We'll wait until one becomes + * available. The mutex is unlocked so that other threads + * actually get the opportunity to free their frames, which + * means technically the result of bitmap_full may be incorrect. + * However the worst case is that we repeat all this and end up + * back here again. + */ + mutex_unlock(&mm_ctx->bd_emupage_mutex); + if (!wait_event_killable(mm_ctx->bd_emupage_queue, + !bitmap_full(mm_ctx->bd_emupage_allocmap, + emupage_frame_count))) + goto retry; + + /* Received a fatal signal - just give in */ + return BD_EMUFRAME_NONE; + } + + /* Success! */ + pr_debug("allocate emuframe %d to %d\n", idx, current->pid); +out_unlock: + mutex_unlock(&mm_ctx->bd_emupage_mutex); + return idx; +} + +static void free_emuframe(int idx) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + + mutex_lock(&mm_ctx->bd_emupage_mutex); + + pr_debug("free emuframe %d from %d\n", idx, current->pid); + bitmap_clear(mm_ctx->bd_emupage_allocmap, idx, 1); + + /* If some thread is waiting for a frame, now's its chance */ + wake_up(&mm_ctx->bd_emupage_queue); + + mutex_unlock(&mm_ctx->bd_emupage_mutex); +} + +static bool within_emuframe(struct pt_regs *regs) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + + if (regs->cp0_epc < mm_ctx->bd_emupage) + return false; + if (regs->cp0_epc >= (mm_ctx->bd_emupage + PAGE_SIZE)) + return false; + + return true; +} + +bool dsemul_thread_cleanup(void) +{ + int fr_idx; + + /* Clear any allocated frame, retrieving its index */ + fr_idx = atomic_xchg(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); + + /* If no frame was allocated, we're done */ + if (fr_idx == BD_EMUFRAME_NONE) + return false; + + /* Free the frame that this thread had allocated */ + free_emuframe(fr_idx); + return true; +} + +bool dsemul_thread_rollback(struct pt_regs *regs) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + struct emuframe __user *fr; + int fr_idx; + + /* Do nothing if we're not executing from a frame */ + if (!within_emuframe(regs)) + return false; + + /* Find the frame being executed */ + fr_idx = atomic_read(¤t->thread.bd_emu_frame); + if (fr_idx == BD_EMUFRAME_NONE) + return false; + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; + + /* + * If the PC is at the emul instruction, roll back to the branch. If + * PC is at the badinst (break) instruction, we've already emulated the + * instruction so progress to the continue PC. If it's anything else + * then something is amiss. + */ + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) + regs->cp0_epc = current->thread.bd_emu_branch_pc; + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->badinst) + regs->cp0_epc = current->thread.bd_emu_cont_pc; + else + return false; + + atomic_set(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); + free_emuframe(fr_idx); + return true; +} + +void dsemul_mm_cleanup(struct mm_struct *mm) +{ + mm_context_t *mm_ctx = &mm->context; + + kfree(mm_ctx->bd_emupage_allocmap); +} + +int mips_dsemul(struct pt_regs *regs, mips_instruction ir, + unsigned long branch_pc, unsigned long cont_pc) { int isa16 = get_isa16_mode(regs->cp0_epc); mips_instruction break_math; + mm_context_t *mm_ctx = ¤t->mm->context; struct emuframe __user *fr; - int err; + int err, fr_idx; /* NOP is easy */ if (ir == 0) @@ -68,30 +249,20 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) } } - pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc); + pr_debug("dsemul 0x%08lx cont at 0x%08lx\n", regs->cp0_epc, cont_pc); - /* - * The strategy is to push the instruction onto the user stack - * and put a trap after it which we can catch and jump to - * the required address any alternative apart from full - * instruction emulation!!. - * - * Algorithmics used a system call instruction, and - * borrowed that vector. MIPS/Linux version is a bit - * more heavyweight in the interests of portability and - * multiprocessor support. For Linux we use a BREAK 514 - * instruction causing a breakpoint exception. - */ - break_math = BREAK_MATH(isa16); - - /* Ensure that the two instructions are in the same cache line */ - fr = (struct emuframe __user *) - ((regs->regs[29] - sizeof(struct emuframe)) & ~0x7); - - /* Verify that the stack pointer is not completely insane */ - if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe)))) + /* Allocate a frame if we don't already have one */ + fr_idx = atomic_read(¤t->thread.bd_emu_frame); + if (fr_idx == BD_EMUFRAME_NONE) + fr_idx = alloc_emuframe(); + if (fr_idx == BD_EMUFRAME_NONE) return SIGBUS; + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; + + /* Retrieve the appropriately encoded break instruction */ + break_math = BREAK_MATH(isa16); + /* Write the instructions to the frame */ if (isa16) { err = __put_user(ir >> 16, (u16 __user *)(&fr->emul)); @@ -106,84 +277,36 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) err |= __put_user(break_math, &fr->badinst); } - err |= __put_user((mips_instruction)BD_COOKIE, &fr->cookie); - err |= __put_user(cpc, &fr->epc); - if (unlikely(err)) { MIPS_FPU_EMU_INC_STATS(errors); + free_emuframe(fr_idx); return SIGBUS; } + /* Record the PC of the branch, PC to continue from & frame index */ + current->thread.bd_emu_branch_pc = branch_pc; + current->thread.bd_emu_cont_pc = cont_pc; + atomic_set(¤t->thread.bd_emu_frame, fr_idx); + + /* Change user register context to execute the frame */ regs->cp0_epc = (unsigned long)&fr->emul | isa16; + /* Ensure the icache observes our newly written frame */ flush_cache_sigtramp((unsigned long)&fr->emul); return 0; } -int do_dsemulret(struct pt_regs *xcp) +bool do_dsemulret(struct pt_regs *xcp) { - int isa16 = get_isa16_mode(xcp->cp0_epc); - struct emuframe __user *fr; - unsigned long epc; - u32 insn, cookie; - int err = 0; - u16 instr[2]; - - fr = (struct emuframe __user *) - (msk_isa16_mode(xcp->cp0_epc) - sizeof(mips_instruction)); - - /* - * If we can't even access the area, something is very wrong, but we'll - * leave that to the default handling - */ - if (!access_ok(VERIFY_READ, fr, sizeof(struct emuframe))) - return 0; - - /* - * Do some sanity checking on the stackframe: - * - * - Is the instruction pointed to by the EPC an BREAK_MATH? - * - Is the following memory word the BD_COOKIE? - */ - if (isa16) { - err = __get_user(instr[0], - (u16 __user *)(&fr->badinst)); - err |= __get_user(instr[1], - (u16 __user *)((long)(&fr->badinst) + 2)); - insn = (instr[0] << 16) | instr[1]; - } else { - err = __get_user(insn, &fr->badinst); - } - err |= __get_user(cookie, &fr->cookie); - - if (unlikely(err || - insn != BREAK_MATH(isa16) || cookie != BD_COOKIE)) { + /* Cleanup the allocated frame, returning if there wasn't one */ + if (!dsemul_thread_cleanup()) { MIPS_FPU_EMU_INC_STATS(errors); - return 0; - } - - /* - * At this point, we are satisfied that it's a BD emulation trap. Yes, - * a user might have deliberately put two malformed and useless - * instructions in a row in his program, in which case he's in for a - * nasty surprise - the next instruction will be treated as a - * continuation address! Alas, this seems to be the only way that we - * can handle signals, recursion, and longjmps() in the context of - * emulating the branch delay instruction. - */ - - pr_debug("dsemulret\n"); - - if (__get_user(epc, &fr->epc)) { /* Saved EPC */ - /* This is not a good situation to be in */ - force_sig(SIGBUS, current); - - return 0; + return false; } /* Set EPC to return to post-branch instruction */ - xcp->cp0_epc = epc; - MIPS_FPU_EMU_INC_STATS(ds_emul); - return 1; + xcp->cp0_epc = current->thread.bd_emu_cont_pc; + pr_debug("dsemulret to 0x%08lx\n", xcp->cp0_epc); + return true; } -- 2.9.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-29 14:38 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-29 14:38 UTC (permalink / raw) To: linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham, Paul Burton In some cases the kernel needs to execute an instruction from the delay slot of an emulated branch instruction. These cases include: - Emulated floating point branch instructions (bc1[ft]l?) for systems which don't include an FPU, or upon which the kernel is run with the "nofpu" parameter. - MIPSr6 systems running binaries targeting older revisions of the architecture, which may include branch instructions whose encodings are no longer valid in MIPSr6. Executing instructions from such delay slots is done by writing the instruction to memory followed by a trap, as part of an "emuframe", and executing it. This avoids the requirement of an emulator for the entire MIPS instruction set. Prior to this patch such emuframes are written to the user stack and executed from there. This patch moves FP branch delay emuframes off of the user stack and into a per-mm page. Allocating a page per-mm leaves userland with access to only what it had access to previously, and compared to other solutions is relatively simple. When a thread requires a delay slot emulation, it is allocated a frame. A thread may only have one frame allocated at any one time, since it may only ever be executing one instruction at any one time. In order to ensure that we can free up allocated frame later, its index is recorded in struct thread_struct. In the typical case, after executing the delay slot instruction we'll execute a break instruction with the BRK_MEMU code. This traps back to the kernel & leads to a call to do_dsemulret which frees the allocated frame & moves the user PC back to the instruction that would have executed following the emulated branch. In some cases the delay slot instruction may be invalid, such as a branch, or may trigger an exception. In these cases the BRK_MEMU break instruction will not be hit. In order to ensure that frames are freed this patch introduces dsemul_thread_cleanup() and calls it to free any allocated frame upon thread exit. If the instruction generated an exception & leads to a signal being delivered to the thread, or indeed if a signal simply happens to be delivered to the thread whilst it is executing from the struct emuframe, then we need to take care to exit the frame appropriately. This is done by either rolling back the user PC to the branch or advancing it to the continuation PC prior to signal delivery, using dsemul_thread_rollback(). If this were not done then a sigreturn would return to the struct emuframe, and if that frame had meanwhile been used in response to an emulated branch instruction within the signal handler then we would execute the wrong user code. Whilst a user could theoretically place something like a compact branch to self in a delay slot and cause their thread to become stuck in an infinite loop with the frame never being deallocated, this would: - Only affect the users single process. - Be architecturally invalid since there would be a branch in the delay slot, which is forbidden. - Be extremely unlikely to happen by mistake, and provide a program with no more ability to harm the system than a simple infinite loop would. If a thread requires a delay slot emulation & no frame is available to it (ie. the process has enough other threads that all frames are currently in use) then the thread joins a waitqueue. It will sleep until a frame is freed by another thread in the process. Since we now know whether a thread has an allocated frame due to our tracking of its index, the cookie field of struct emuframe is removed as we can be more certain whether we have a valid frame. Since a thread may only ever have a single frame at any given time, the epc field of struct emuframe is also removed & the PC to continue from is instead stored in struct thread_struct. Together these changes simplify & shrink struct emuframe somewhat, allowing twice as many frames to fit into the page allocated for them. The primary benefit of this patch is that we are now free to mark the user stack non-executable where that is possible. Signed-off-by: Paul Burton <paul.burton@imgtec.com> --- v2 of this patch can be found here: https://patchwork.linux-mips.org/patch/6125/ This has become a higher priority than it was at the time of v2 since Android has begun marking its stacks non-executable & on MIPSr6 devices we use mips_dsemul() in the emulation of pre-r6 instructions. Since the Android NDK MIPS target is MIPS32, this is important to backwards compatibility for apps on MIPSr6 systems. Changes in v3: - Rebase atop v4.7-rc5. - Select CONFIG_HAVE_EXIT_THREAD. - Track allocated frames per thread, allowing cleanup on exit or signal delivery. - Remove signal blocking & thread information flag now we have other means to ensure frames are cleaned up. - Introduce asm/dsemul.h to group prototypes & definitions logically. - Avoid using 'fp' in names, since this isn't exclusive to FP branch emulation. - Document with kerneldoc. - Return a frame index from alloc_emuframe such that mips_dsemul can record it in struct thread_struct. - Use bool return type for do_dsemulret rather than a bool-like int. - Rework commit message. Changes in v2: - s/kernels/kernel's/ - Use (mm_)isBranchInstr in mips_dsemul rather than duplicating similar logic. arch/mips/Kconfig | 1 + arch/mips/include/asm/fpu_emulator.h | 17 +- arch/mips/include/asm/mmu.h | 11 ++ arch/mips/include/asm/mmu_context.h | 7 + arch/mips/include/asm/processor.h | 18 +- arch/mips/kernel/mips-r2-to-r6-emul.c | 8 +- arch/mips/kernel/process.c | 6 + arch/mips/kernel/signal.c | 8 + arch/mips/math-emu/cp1emu.c | 8 +- arch/mips/math-emu/dsemul.c | 343 +++++++++++++++++++++++----------- 10 files changed, 294 insertions(+), 133 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index ac91939..49a5396 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -64,6 +64,7 @@ config MIPS select GENERIC_TIME_VSYSCALL select ARCH_CLOCKSOURCE_DATA select HANDLE_DOMAIN_IRQ + select HAVE_EXIT_THREAD menu "Machine selection" diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h index 3225c3c..355dc25 100644 --- a/arch/mips/include/asm/fpu_emulator.h +++ b/arch/mips/include/asm/fpu_emulator.h @@ -24,7 +24,7 @@ #define _ASM_FPU_EMULATOR_H #include <linux/sched.h> -#include <asm/break.h> +#include <asm/dsemul.h> #include <asm/thread_info.h> #include <asm/inst.h> #include <asm/local.h> @@ -60,27 +60,16 @@ do { \ #define MIPS_FPU_EMU_INC_STATS(M) do { } while (0) #endif /* CONFIG_DEBUG_FS */ -extern int mips_dsemul(struct pt_regs *regs, mips_instruction ir, - unsigned long cpc); -extern int do_dsemulret(struct pt_regs *xcp); extern int fpu_emulator_cop1Handler(struct pt_regs *xcp, struct mips_fpu_struct *ctx, int has_fpu, void *__user *fault_addr); int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31); +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, + unsigned long *contpc); int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, unsigned long *contpc); -/* - * Instruction inserted following the badinst to further tag the sequence - */ -#define BD_COOKIE 0x0000bd36 /* tne $0, $0 with baggage */ - -/* - * Break instruction with special math emu break code set - */ -#define BREAK_MATH(micromips) (((micromips) ? 0x7 : 0xd) | (BRK_MEMU << 16)) - #define SIGNALLING_NAN 0x7ff800007ff80000LL static inline void fpu_emulator_init_fpu(void) diff --git a/arch/mips/include/asm/mmu.h b/arch/mips/include/asm/mmu.h index 1afa1f9..df6ec07 100644 --- a/arch/mips/include/asm/mmu.h +++ b/arch/mips/include/asm/mmu.h @@ -2,11 +2,22 @@ #define __ASM_MMU_H #include <linux/atomic.h> +#include <linux/mutex.h> +#include <linux/wait.h> typedef struct { unsigned long asid[NR_CPUS]; void *vdso; atomic_t fp_mode_switching; + + /* address of page used to hold FP branch delay emulation frames */ + unsigned long bd_emupage; + /* bitmap tracking allocation of fp_bd_emupage */ + unsigned long *bd_emupage_allocmap; + /* mutex to be held whilst modifying fp_bd_emupage(_allocmap) */ + struct mutex bd_emupage_mutex; + /* wait queue for threads requiring an emuframe */ + wait_queue_head_t bd_emupage_queue; } mm_context_t; #endif /* __ASM_MMU_H */ diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h index fc57e13..174d68a 100644 --- a/arch/mips/include/asm/mmu_context.h +++ b/arch/mips/include/asm/mmu_context.h @@ -16,6 +16,7 @@ #include <linux/smp.h> #include <linux/slab.h> #include <asm/cacheflush.h> +#include <asm/dsemul.h> #include <asm/hazards.h> #include <asm/tlbflush.h> #include <asm-generic/mm_hooks.h> @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct mm_struct *mm) atomic_set(&mm->context.fp_mode_switching, 0); + mm->context.bd_emupage = 0; + mm->context.bd_emupage_allocmap = NULL; + mutex_init(&mm->context.bd_emupage_mutex); + init_waitqueue_head(&mm->context.bd_emupage_queue); + return 0; } @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, */ static inline void destroy_context(struct mm_struct *mm) { + dsemul_mm_cleanup(mm); } #define deactivate_mm(tsk, mm) do { } while (0) diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h index 7e78b62..0d36c87 100644 --- a/arch/mips/include/asm/processor.h +++ b/arch/mips/include/asm/processor.h @@ -11,12 +11,14 @@ #ifndef _ASM_PROCESSOR_H #define _ASM_PROCESSOR_H +#include <linux/atomic.h> #include <linux/cpumask.h> #include <linux/threads.h> #include <asm/cachectl.h> #include <asm/cpu.h> #include <asm/cpu-info.h> +#include <asm/dsemul.h> #include <asm/mipsregs.h> #include <asm/prefetch.h> @@ -78,7 +80,11 @@ extern unsigned int vced_count, vcei_count; #endif -#define STACK_TOP (TASK_SIZE & PAGE_MASK) +/* + * One page above the stack is used for branch delay slot "emulation". + * See dsemul.c for details. + */ +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) /* * This decides where the kernel will search for a free chunk of vm @@ -256,6 +262,12 @@ struct thread_struct { /* Saved fpu/fpu emulator stuff. */ struct mips_fpu_struct fpu FPU_ALIGN; + /* Assigned branch delay slot 'emulation' frame */ + atomic_t bd_emu_frame; + /* PC of the branch from a branch delay slot 'emulation' */ + unsigned long bd_emu_branch_pc; + /* PC to continue from following a branch delay slot 'emulation' */ + unsigned long bd_emu_cont_pc; #ifdef CONFIG_MIPS_MT_FPAFF /* Emulated instruction count */ unsigned long emulated_fp; @@ -323,6 +335,10 @@ struct thread_struct { * FPU affinity state (null if not FPAFF) \ */ \ FPAFF_INIT \ + /* Delay slot emulation */ \ + .bd_emu_frame = ATOMIC_INIT(BD_EMUFRAME_NONE), \ + .bd_emu_branch_pc = 0, \ + .bd_emu_cont_pc = 0, \ /* \ * Saved DSP stuff \ */ \ diff --git a/arch/mips/kernel/mips-r2-to-r6-emul.c b/arch/mips/kernel/mips-r2-to-r6-emul.c index 7ff2a55..ef23c61 100644 --- a/arch/mips/kernel/mips-r2-to-r6-emul.c +++ b/arch/mips/kernel/mips-r2-to-r6-emul.c @@ -283,7 +283,7 @@ static int jr_func(struct pt_regs *regs, u32 ir) err = mipsr6_emul(regs, nir); if (err > 0) { regs->cp0_epc = nepc; - err = mips_dsemul(regs, nir, cepc); + err = mips_dsemul(regs, nir, epc, cepc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); @@ -1033,7 +1033,7 @@ repeat: if (nir) { err = mipsr6_emul(regs, nir); if (err > 0) { - err = mips_dsemul(regs, nir, cpc); + err = mips_dsemul(regs, nir, epc, cpc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); @@ -1082,7 +1082,7 @@ repeat: if (nir) { err = mipsr6_emul(regs, nir); if (err > 0) { - err = mips_dsemul(regs, nir, cpc); + err = mips_dsemul(regs, nir, epc, cpc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); @@ -1149,7 +1149,7 @@ repeat: if (nir) { err = mipsr6_emul(regs, nir); if (err > 0) { - err = mips_dsemul(regs, nir, cpc); + err = mips_dsemul(regs, nir, epc, cpc); if (err == SIGILL) err = SIGEMT; MIPS_R2_STATS(dsemul); diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 813ed78..0fb1e8c 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -30,6 +30,7 @@ #include <asm/asm.h> #include <asm/bootinfo.h> #include <asm/cpu.h> +#include <asm/dsemul.h> #include <asm/dsp.h> #include <asm/fpu.h> #include <asm/msa.h> @@ -73,6 +74,11 @@ void start_thread(struct pt_regs * regs, unsigned long pc, unsigned long sp) regs->regs[29] = sp; } +void exit_thread(struct task_struct *tsk) +{ + dsemul_thread_cleanup(); +} + int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) { /* diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c index ae42314..9383635 100644 --- a/arch/mips/kernel/signal.c +++ b/arch/mips/kernel/signal.c @@ -772,6 +772,14 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) struct mips_abi *abi = current->thread.abi; void *vdso = current->mm->context.vdso; + /* + * If we were emulating a delay slot instruction, exit that frame such + * that addresses in the sigframe are as expected for userland and we + * don't have a problem if we reuse the thread's frame for an + * instruction within the signal handler. + */ + dsemul_thread_rollback(regs); + if (regs->regs[0]) { switch(regs->regs[2]) { case ERESTART_RESTARTBLOCK: diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c index d96e912..8afa090 100644 --- a/arch/mips/math-emu/cp1emu.c +++ b/arch/mips/math-emu/cp1emu.c @@ -434,8 +434,8 @@ static int microMIPS32_to_MIPS32(union mips_instruction *insn_ptr) * a single subroutine should be used across both * modules. */ -static int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, - unsigned long *contpc) +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, + unsigned long *contpc) { union mips_instruction insn = (union mips_instruction)dec_insn.insn; unsigned int fcr31; @@ -1268,7 +1268,7 @@ branch_common: * instruction in the dslot. */ sig = mips_dsemul(xcp, ir, - contpc); + bcpc, contpc); if (sig < 0) break; if (sig) @@ -1323,7 +1323,7 @@ branch_common: * Single step the non-cp1 * instruction in the dslot */ - sig = mips_dsemul(xcp, ir, contpc); + sig = mips_dsemul(xcp, ir, bcpc, contpc); if (sig < 0) break; if (sig) diff --git a/arch/mips/math-emu/dsemul.c b/arch/mips/math-emu/dsemul.c index 4707488..d21af2f 100644 --- a/arch/mips/math-emu/dsemul.c +++ b/arch/mips/math-emu/dsemul.c @@ -1,3 +1,6 @@ +#include <linux/err.h> +#include <linux/slab.h> + #include <asm/branch.h> #include <asm/cacheflush.h> #include <asm/fpu_emulator.h> @@ -5,43 +8,221 @@ #include <asm/mipsregs.h> #include <asm/uaccess.h> -#include "ieee754.h" - -/* - * Emulate the arbitrary instruction ir at xcp->cp0_epc. Required when - * we have to emulate the instruction in a COP1 branch delay slot. Do - * not change cp0_epc due to the instruction +/** + * struct emuframe - The 'emulation' frame structure + * @emul: The instruction to 'emulate'. + * @badinst: A break instruction to cause a return to the kernel. * - * According to the spec: - * 1) it shouldn't be a branch :-) - * 2) it can be a COP instruction :-( - * 3) if we are tring to run a protected memory space we must take - * special care on memory access instructions :-( - */ - -/* - * "Trampoline" return routine to catch exception following - * execution of delay-slot instruction execution. + * This structure defines the frames placed within the delay slot emulation + * page in response to a call to mips_dsemul(). Each thread may be allocated + * only one frame at any given time. The kernel stores within it the + * instruction to be 'emulated' followed by a break instruction, then + * executes the frame in user mode. The break causes a trap to the kernel + * which leads to do_dsemulret() being called unless the instruction in + * @emul causes a trap itself, is a branch, or a signal is delivered to + * the thread. In these cases the allocated frame will either be reused by + * a subsequent delay slot 'emulation', or be freed during signal delivery or + * upon thread exit. + * + * This approach is used because: + * + * - Actually emulating all instructions isn't feasible. We would need to + * be able to handle instructions from all revisions of the MIPS ISA, + * all ASEs & all vendor instruction set extensions. This would be a + * whole lot of work & continual maintenance burden as new instructions + * are introduced, and in the case of some vendor extensions may not + * even be possible. Thus we need to take the approach of actually + * executing the instruction. + * + * - We must execute the instruction within user context. If we were to + * execute the instruction in kernel mode then it would have access to + * kernel resources without very careful checks, leaving us with a + * high potential for security or stability issues to arise. + * + * - We used to place the frame on the users stack, but this requires + * that the stack be executable. This is bad for security so the + * per-process page is now used instead. + * + * - The instruction in @emul may be something entirely invalid for a + * delay slot. The user may (intentionally or otherwise) place a branch + * in a delay slot, or a kernel mode instruction, or something else + * which generates an exception. Thus we can't rely upon the break in + * @badinst always being hit. For this reason we track the index of the + * frame allocated to each thread, allowing us to clean it up at later + * points such as signal delivery or thread exit. + * + * - The user may generate a fake struct emuframe if they wish, invoking + * the BRK_MEMU break instruction themselves. We must therefore not + * trust that BRK_MEMU means there's actually a valid frame allocated + * to the thread, and must not allow the user to do anything they + * couldn't already. */ - struct emuframe { mips_instruction emul; mips_instruction badinst; - mips_instruction cookie; - unsigned long epc; }; -/* - * Set up an emulation frame for instruction IR, from a delay slot of - * a branch jumping to CPC. Return 0 if successful, -1 if no emulation - * required, otherwise a signal number causing a frame setup failure. - */ -int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) +static const int emupage_frame_count = PAGE_SIZE / sizeof(struct emuframe); + +static int alloc_emuframe(void) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + unsigned long addr; + int idx; + +retry: + mutex_lock(&mm_ctx->bd_emupage_mutex); + + /* Ensure we have a page allocated for emuframes */ + if (!mm_ctx->bd_emupage) { + addr = mmap_region(NULL, STACK_TOP, PAGE_SIZE, + VM_READ|VM_WRITE|VM_EXEC| + VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, + 0); + if (IS_ERR_VALUE(addr)) { + idx = BD_EMUFRAME_NONE; + goto out_unlock; + } + + mm_ctx->bd_emupage = addr; + pr_debug("allocate emupage at 0x%08lx to %d\n", addr, + current->pid); + } + + /* Ensure we have an allocation bitmap */ + if (!mm_ctx->bd_emupage_allocmap) { + mm_ctx->bd_emupage_allocmap = + kcalloc(BITS_TO_LONGS(emupage_frame_count), + sizeof(unsigned long), + GFP_KERNEL); + + if (!mm_ctx->bd_emupage_allocmap) { + idx = BD_EMUFRAME_NONE; + goto out_unlock; + } + } + + /* Attempt to allocate a single bit/frame */ + idx = bitmap_find_free_region(mm_ctx->bd_emupage_allocmap, + emupage_frame_count, 0); + if (idx < 0) { + /* + * Failed to allocate a frame. We'll wait until one becomes + * available. The mutex is unlocked so that other threads + * actually get the opportunity to free their frames, which + * means technically the result of bitmap_full may be incorrect. + * However the worst case is that we repeat all this and end up + * back here again. + */ + mutex_unlock(&mm_ctx->bd_emupage_mutex); + if (!wait_event_killable(mm_ctx->bd_emupage_queue, + !bitmap_full(mm_ctx->bd_emupage_allocmap, + emupage_frame_count))) + goto retry; + + /* Received a fatal signal - just give in */ + return BD_EMUFRAME_NONE; + } + + /* Success! */ + pr_debug("allocate emuframe %d to %d\n", idx, current->pid); +out_unlock: + mutex_unlock(&mm_ctx->bd_emupage_mutex); + return idx; +} + +static void free_emuframe(int idx) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + + mutex_lock(&mm_ctx->bd_emupage_mutex); + + pr_debug("free emuframe %d from %d\n", idx, current->pid); + bitmap_clear(mm_ctx->bd_emupage_allocmap, idx, 1); + + /* If some thread is waiting for a frame, now's its chance */ + wake_up(&mm_ctx->bd_emupage_queue); + + mutex_unlock(&mm_ctx->bd_emupage_mutex); +} + +static bool within_emuframe(struct pt_regs *regs) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + + if (regs->cp0_epc < mm_ctx->bd_emupage) + return false; + if (regs->cp0_epc >= (mm_ctx->bd_emupage + PAGE_SIZE)) + return false; + + return true; +} + +bool dsemul_thread_cleanup(void) +{ + int fr_idx; + + /* Clear any allocated frame, retrieving its index */ + fr_idx = atomic_xchg(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); + + /* If no frame was allocated, we're done */ + if (fr_idx == BD_EMUFRAME_NONE) + return false; + + /* Free the frame that this thread had allocated */ + free_emuframe(fr_idx); + return true; +} + +bool dsemul_thread_rollback(struct pt_regs *regs) +{ + mm_context_t *mm_ctx = ¤t->mm->context; + struct emuframe __user *fr; + int fr_idx; + + /* Do nothing if we're not executing from a frame */ + if (!within_emuframe(regs)) + return false; + + /* Find the frame being executed */ + fr_idx = atomic_read(¤t->thread.bd_emu_frame); + if (fr_idx == BD_EMUFRAME_NONE) + return false; + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; + + /* + * If the PC is at the emul instruction, roll back to the branch. If + * PC is at the badinst (break) instruction, we've already emulated the + * instruction so progress to the continue PC. If it's anything else + * then something is amiss. + */ + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) + regs->cp0_epc = current->thread.bd_emu_branch_pc; + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->badinst) + regs->cp0_epc = current->thread.bd_emu_cont_pc; + else + return false; + + atomic_set(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); + free_emuframe(fr_idx); + return true; +} + +void dsemul_mm_cleanup(struct mm_struct *mm) +{ + mm_context_t *mm_ctx = &mm->context; + + kfree(mm_ctx->bd_emupage_allocmap); +} + +int mips_dsemul(struct pt_regs *regs, mips_instruction ir, + unsigned long branch_pc, unsigned long cont_pc) { int isa16 = get_isa16_mode(regs->cp0_epc); mips_instruction break_math; + mm_context_t *mm_ctx = ¤t->mm->context; struct emuframe __user *fr; - int err; + int err, fr_idx; /* NOP is easy */ if (ir == 0) @@ -68,30 +249,20 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) } } - pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc); + pr_debug("dsemul 0x%08lx cont at 0x%08lx\n", regs->cp0_epc, cont_pc); - /* - * The strategy is to push the instruction onto the user stack - * and put a trap after it which we can catch and jump to - * the required address any alternative apart from full - * instruction emulation!!. - * - * Algorithmics used a system call instruction, and - * borrowed that vector. MIPS/Linux version is a bit - * more heavyweight in the interests of portability and - * multiprocessor support. For Linux we use a BREAK 514 - * instruction causing a breakpoint exception. - */ - break_math = BREAK_MATH(isa16); - - /* Ensure that the two instructions are in the same cache line */ - fr = (struct emuframe __user *) - ((regs->regs[29] - sizeof(struct emuframe)) & ~0x7); - - /* Verify that the stack pointer is not completely insane */ - if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe)))) + /* Allocate a frame if we don't already have one */ + fr_idx = atomic_read(¤t->thread.bd_emu_frame); + if (fr_idx == BD_EMUFRAME_NONE) + fr_idx = alloc_emuframe(); + if (fr_idx == BD_EMUFRAME_NONE) return SIGBUS; + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; + + /* Retrieve the appropriately encoded break instruction */ + break_math = BREAK_MATH(isa16); + /* Write the instructions to the frame */ if (isa16) { err = __put_user(ir >> 16, (u16 __user *)(&fr->emul)); @@ -106,84 +277,36 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) err |= __put_user(break_math, &fr->badinst); } - err |= __put_user((mips_instruction)BD_COOKIE, &fr->cookie); - err |= __put_user(cpc, &fr->epc); - if (unlikely(err)) { MIPS_FPU_EMU_INC_STATS(errors); + free_emuframe(fr_idx); return SIGBUS; } + /* Record the PC of the branch, PC to continue from & frame index */ + current->thread.bd_emu_branch_pc = branch_pc; + current->thread.bd_emu_cont_pc = cont_pc; + atomic_set(¤t->thread.bd_emu_frame, fr_idx); + + /* Change user register context to execute the frame */ regs->cp0_epc = (unsigned long)&fr->emul | isa16; + /* Ensure the icache observes our newly written frame */ flush_cache_sigtramp((unsigned long)&fr->emul); return 0; } -int do_dsemulret(struct pt_regs *xcp) +bool do_dsemulret(struct pt_regs *xcp) { - int isa16 = get_isa16_mode(xcp->cp0_epc); - struct emuframe __user *fr; - unsigned long epc; - u32 insn, cookie; - int err = 0; - u16 instr[2]; - - fr = (struct emuframe __user *) - (msk_isa16_mode(xcp->cp0_epc) - sizeof(mips_instruction)); - - /* - * If we can't even access the area, something is very wrong, but we'll - * leave that to the default handling - */ - if (!access_ok(VERIFY_READ, fr, sizeof(struct emuframe))) - return 0; - - /* - * Do some sanity checking on the stackframe: - * - * - Is the instruction pointed to by the EPC an BREAK_MATH? - * - Is the following memory word the BD_COOKIE? - */ - if (isa16) { - err = __get_user(instr[0], - (u16 __user *)(&fr->badinst)); - err |= __get_user(instr[1], - (u16 __user *)((long)(&fr->badinst) + 2)); - insn = (instr[0] << 16) | instr[1]; - } else { - err = __get_user(insn, &fr->badinst); - } - err |= __get_user(cookie, &fr->cookie); - - if (unlikely(err || - insn != BREAK_MATH(isa16) || cookie != BD_COOKIE)) { + /* Cleanup the allocated frame, returning if there wasn't one */ + if (!dsemul_thread_cleanup()) { MIPS_FPU_EMU_INC_STATS(errors); - return 0; - } - - /* - * At this point, we are satisfied that it's a BD emulation trap. Yes, - * a user might have deliberately put two malformed and useless - * instructions in a row in his program, in which case he's in for a - * nasty surprise - the next instruction will be treated as a - * continuation address! Alas, this seems to be the only way that we - * can handle signals, recursion, and longjmps() in the context of - * emulating the branch delay instruction. - */ - - pr_debug("dsemulret\n"); - - if (__get_user(epc, &fr->epc)) { /* Saved EPC */ - /* This is not a good situation to be in */ - force_sig(SIGBUS, current); - - return 0; + return false; } /* Set EPC to return to post-branch instruction */ - xcp->cp0_epc = epc; - MIPS_FPU_EMU_INC_STATS(ds_emul); - return 1; + xcp->cp0_epc = current->thread.bd_emu_cont_pc; + pr_debug("dsemulret to 0x%08lx\n", xcp->cp0_epc); + return true; } -- 2.9.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 9:01 ` Matt Redfearn 0 siblings, 0 replies; 22+ messages in thread From: Matt Redfearn @ 2016-06-30 9:01 UTC (permalink / raw) To: Paul Burton, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham Hi Paul On 29/06/16 15:38, Paul Burton wrote: > In some cases the kernel needs to execute an instruction from the delay > slot of an emulated branch instruction. These cases include: > > - Emulated floating point branch instructions (bc1[ft]l?) for systems > which don't include an FPU, or upon which the kernel is run with the > "nofpu" parameter. > > - MIPSr6 systems running binaries targeting older revisions of the > architecture, which may include branch instructions whose encodings > are no longer valid in MIPSr6. > > Executing instructions from such delay slots is done by writing the > instruction to memory followed by a trap, as part of an "emuframe", and > executing it. This avoids the requirement of an emulator for the entire > MIPS instruction set. Prior to this patch such emuframes are written to > the user stack and executed from there. > > This patch moves FP branch delay emuframes off of the user stack and > into a per-mm page. Allocating a page per-mm leaves userland with access > to only what it had access to previously, and compared to other > solutions is relatively simple. > > When a thread requires a delay slot emulation, it is allocated a frame. > A thread may only have one frame allocated at any one time, since it may > only ever be executing one instruction at any one time. In order to > ensure that we can free up allocated frame later, its index is recorded > in struct thread_struct. In the typical case, after executing the delay > slot instruction we'll execute a break instruction with the BRK_MEMU > code. This traps back to the kernel & leads to a call to do_dsemulret > which frees the allocated frame & moves the user PC back to the > instruction that would have executed following the emulated branch. > In some cases the delay slot instruction may be invalid, such as a > branch, or may trigger an exception. In these cases the BRK_MEMU break > instruction will not be hit. In order to ensure that frames are freed > this patch introduces dsemul_thread_cleanup() and calls it to free any > allocated frame upon thread exit. If the instruction generated an > exception & leads to a signal being delivered to the thread, or indeed > if a signal simply happens to be delivered to the thread whilst it is > executing from the struct emuframe, then we need to take care to exit > the frame appropriately. This is done by either rolling back the user PC > to the branch or advancing it to the continuation PC prior to signal > delivery, using dsemul_thread_rollback(). If this were not done then a > sigreturn would return to the struct emuframe, and if that frame had > meanwhile been used in response to an emulated branch instruction within > the signal handler then we would execute the wrong user code. > > Whilst a user could theoretically place something like a compact branch > to self in a delay slot and cause their thread to become stuck in an > infinite loop with the frame never being deallocated, this would: > > - Only affect the users single process. > > - Be architecturally invalid since there would be a branch in the > delay slot, which is forbidden. > > - Be extremely unlikely to happen by mistake, and provide a program > with no more ability to harm the system than a simple infinite loop > would. > > If a thread requires a delay slot emulation & no frame is available to > it (ie. the process has enough other threads that all frames are > currently in use) then the thread joins a waitqueue. It will sleep until > a frame is freed by another thread in the process. > > Since we now know whether a thread has an allocated frame due to our > tracking of its index, the cookie field of struct emuframe is removed as > we can be more certain whether we have a valid frame. Since a thread may > only ever have a single frame at any given time, the epc field of struct > emuframe is also removed & the PC to continue from is instead stored in > struct thread_struct. Together these changes simplify & shrink struct > emuframe somewhat, allowing twice as many frames to fit into the page > allocated for them. > > The primary benefit of this patch is that we are now free to mark the > user stack non-executable where that is possible. > > Signed-off-by: Paul Burton <paul.burton@imgtec.com> > > --- > v2 of this patch can be found here: > > https://patchwork.linux-mips.org/patch/6125/ > > This has become a higher priority than it was at the time of v2 since > Android has begun marking its stacks non-executable & on MIPSr6 devices > we use mips_dsemul() in the emulation of pre-r6 instructions. Since the > Android NDK MIPS target is MIPS32, this is important to backwards > compatibility for apps on MIPSr6 systems. > > Changes in v3: > - Rebase atop v4.7-rc5. > - Select CONFIG_HAVE_EXIT_THREAD. > - Track allocated frames per thread, allowing cleanup on exit or signal delivery. > - Remove signal blocking & thread information flag now we have other means to ensure frames are cleaned up. > - Introduce asm/dsemul.h to group prototypes & definitions logically. asm/dsemul.h seems to be missing from the patch? > - Avoid using 'fp' in names, since this isn't exclusive to FP branch emulation. > - Document with kerneldoc. > - Return a frame index from alloc_emuframe such that mips_dsemul can record it in struct thread_struct. > - Use bool return type for do_dsemulret rather than a bool-like int. > - Rework commit message. > > Changes in v2: > - s/kernels/kernel's/ > - Use (mm_)isBranchInstr in mips_dsemul rather than duplicating similar logic. > > arch/mips/Kconfig | 1 + > arch/mips/include/asm/fpu_emulator.h | 17 +- > arch/mips/include/asm/mmu.h | 11 ++ > arch/mips/include/asm/mmu_context.h | 7 + > arch/mips/include/asm/processor.h | 18 +- > arch/mips/kernel/mips-r2-to-r6-emul.c | 8 +- > arch/mips/kernel/process.c | 6 + > arch/mips/kernel/signal.c | 8 + > arch/mips/math-emu/cp1emu.c | 8 +- > arch/mips/math-emu/dsemul.c | 343 +++++++++++++++++++++++----------- > 10 files changed, 294 insertions(+), 133 deletions(-) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index ac91939..49a5396 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -64,6 +64,7 @@ config MIPS > select GENERIC_TIME_VSYSCALL > select ARCH_CLOCKSOURCE_DATA > select HANDLE_DOMAIN_IRQ > + select HAVE_EXIT_THREAD > > menu "Machine selection" > > diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h > index 3225c3c..355dc25 100644 > --- a/arch/mips/include/asm/fpu_emulator.h > +++ b/arch/mips/include/asm/fpu_emulator.h > @@ -24,7 +24,7 @@ > #define _ASM_FPU_EMULATOR_H > > #include <linux/sched.h> > -#include <asm/break.h> > +#include <asm/dsemul.h> > #include <asm/thread_info.h> > #include <asm/inst.h> > #include <asm/local.h> > @@ -60,27 +60,16 @@ do { \ > #define MIPS_FPU_EMU_INC_STATS(M) do { } while (0) > #endif /* CONFIG_DEBUG_FS */ > > -extern int mips_dsemul(struct pt_regs *regs, mips_instruction ir, > - unsigned long cpc); > -extern int do_dsemulret(struct pt_regs *xcp); > extern int fpu_emulator_cop1Handler(struct pt_regs *xcp, > struct mips_fpu_struct *ctx, int has_fpu, > void *__user *fault_addr); > int process_fpemu_return(int sig, void __user *fault_addr, > unsigned long fcr31); > +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > + unsigned long *contpc); > int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > unsigned long *contpc); > > -/* > - * Instruction inserted following the badinst to further tag the sequence > - */ > -#define BD_COOKIE 0x0000bd36 /* tne $0, $0 with baggage */ > - > -/* > - * Break instruction with special math emu break code set > - */ > -#define BREAK_MATH(micromips) (((micromips) ? 0x7 : 0xd) | (BRK_MEMU << 16)) > - > #define SIGNALLING_NAN 0x7ff800007ff80000LL > > static inline void fpu_emulator_init_fpu(void) > diff --git a/arch/mips/include/asm/mmu.h b/arch/mips/include/asm/mmu.h > index 1afa1f9..df6ec07 100644 > --- a/arch/mips/include/asm/mmu.h > +++ b/arch/mips/include/asm/mmu.h > @@ -2,11 +2,22 @@ > #define __ASM_MMU_H > > #include <linux/atomic.h> > +#include <linux/mutex.h> > +#include <linux/wait.h> > > typedef struct { > unsigned long asid[NR_CPUS]; > void *vdso; > atomic_t fp_mode_switching; > + > + /* address of page used to hold FP branch delay emulation frames */ > + unsigned long bd_emupage; > + /* bitmap tracking allocation of fp_bd_emupage */ > + unsigned long *bd_emupage_allocmap; > + /* mutex to be held whilst modifying fp_bd_emupage(_allocmap) */ > + struct mutex bd_emupage_mutex; > + /* wait queue for threads requiring an emuframe */ > + wait_queue_head_t bd_emupage_queue; > } mm_context_t; > > #endif /* __ASM_MMU_H */ > diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h > index fc57e13..174d68a 100644 > --- a/arch/mips/include/asm/mmu_context.h > +++ b/arch/mips/include/asm/mmu_context.h > @@ -16,6 +16,7 @@ > #include <linux/smp.h> > #include <linux/slab.h> > #include <asm/cacheflush.h> > +#include <asm/dsemul.h> > #include <asm/hazards.h> > #include <asm/tlbflush.h> > #include <asm-generic/mm_hooks.h> > @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct mm_struct *mm) > > atomic_set(&mm->context.fp_mode_switching, 0); > > + mm->context.bd_emupage = 0; > + mm->context.bd_emupage_allocmap = NULL; > + mutex_init(&mm->context.bd_emupage_mutex); > + init_waitqueue_head(&mm->context.bd_emupage_queue); > + Should this be wrapped in a CONFIG? We're introducing overhead to every tsk creation even if we won't be making use of the emulation. > return 0; > } > > @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, > */ > static inline void destroy_context(struct mm_struct *mm) > { > + dsemul_mm_cleanup(mm); Ditto. > } > > #define deactivate_mm(tsk, mm) do { } while (0) > diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h > index 7e78b62..0d36c87 100644 > --- a/arch/mips/include/asm/processor.h > +++ b/arch/mips/include/asm/processor.h > @@ -11,12 +11,14 @@ > #ifndef _ASM_PROCESSOR_H > #define _ASM_PROCESSOR_H > > +#include <linux/atomic.h> > #include <linux/cpumask.h> > #include <linux/threads.h> > > #include <asm/cachectl.h> > #include <asm/cpu.h> > #include <asm/cpu-info.h> > +#include <asm/dsemul.h> > #include <asm/mipsregs.h> > #include <asm/prefetch.h> > > @@ -78,7 +80,11 @@ extern unsigned int vced_count, vcei_count; > > #endif > > -#define STACK_TOP (TASK_SIZE & PAGE_MASK) > +/* > + * One page above the stack is used for branch delay slot "emulation". > + * See dsemul.c for details. > + */ > +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) Again, should this be dependent on config? Otherwise we waste a page for every task. > > /* > * This decides where the kernel will search for a free chunk of vm > @@ -256,6 +262,12 @@ struct thread_struct { > > /* Saved fpu/fpu emulator stuff. */ > struct mips_fpu_struct fpu FPU_ALIGN; > + /* Assigned branch delay slot 'emulation' frame */ > + atomic_t bd_emu_frame; > + /* PC of the branch from a branch delay slot 'emulation' */ > + unsigned long bd_emu_branch_pc; > + /* PC to continue from following a branch delay slot 'emulation' */ > + unsigned long bd_emu_cont_pc; > #ifdef CONFIG_MIPS_MT_FPAFF > /* Emulated instruction count */ > unsigned long emulated_fp; > @@ -323,6 +335,10 @@ struct thread_struct { > * FPU affinity state (null if not FPAFF) \ > */ \ > FPAFF_INIT \ > + /* Delay slot emulation */ \ > + .bd_emu_frame = ATOMIC_INIT(BD_EMUFRAME_NONE), \ > + .bd_emu_branch_pc = 0, \ > + .bd_emu_cont_pc = 0, \ > /* \ > * Saved DSP stuff \ > */ \ > diff --git a/arch/mips/kernel/mips-r2-to-r6-emul.c b/arch/mips/kernel/mips-r2-to-r6-emul.c > index 7ff2a55..ef23c61 100644 > --- a/arch/mips/kernel/mips-r2-to-r6-emul.c > +++ b/arch/mips/kernel/mips-r2-to-r6-emul.c > @@ -283,7 +283,7 @@ static int jr_func(struct pt_regs *regs, u32 ir) > err = mipsr6_emul(regs, nir); > if (err > 0) { > regs->cp0_epc = nepc; > - err = mips_dsemul(regs, nir, cepc); > + err = mips_dsemul(regs, nir, epc, cepc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > @@ -1033,7 +1033,7 @@ repeat: > if (nir) { > err = mipsr6_emul(regs, nir); > if (err > 0) { > - err = mips_dsemul(regs, nir, cpc); > + err = mips_dsemul(regs, nir, epc, cpc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > @@ -1082,7 +1082,7 @@ repeat: > if (nir) { > err = mipsr6_emul(regs, nir); > if (err > 0) { > - err = mips_dsemul(regs, nir, cpc); > + err = mips_dsemul(regs, nir, epc, cpc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > @@ -1149,7 +1149,7 @@ repeat: > if (nir) { > err = mipsr6_emul(regs, nir); > if (err > 0) { > - err = mips_dsemul(regs, nir, cpc); > + err = mips_dsemul(regs, nir, epc, cpc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > index 813ed78..0fb1e8c 100644 > --- a/arch/mips/kernel/process.c > +++ b/arch/mips/kernel/process.c > @@ -30,6 +30,7 @@ > #include <asm/asm.h> > #include <asm/bootinfo.h> > #include <asm/cpu.h> > +#include <asm/dsemul.h> > #include <asm/dsp.h> > #include <asm/fpu.h> > #include <asm/msa.h> > @@ -73,6 +74,11 @@ void start_thread(struct pt_regs * regs, unsigned long pc, unsigned long sp) > regs->regs[29] = sp; > } > > +void exit_thread(struct task_struct *tsk) > +{ > + dsemul_thread_cleanup(); > +} > + > int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) > { > /* > diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c > index ae42314..9383635 100644 > --- a/arch/mips/kernel/signal.c > +++ b/arch/mips/kernel/signal.c > @@ -772,6 +772,14 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) > struct mips_abi *abi = current->thread.abi; > void *vdso = current->mm->context.vdso; > > + /* > + * If we were emulating a delay slot instruction, exit that frame such > + * that addresses in the sigframe are as expected for userland and we > + * don't have a problem if we reuse the thread's frame for an > + * instruction within the signal handler. > + */ > + dsemul_thread_rollback(regs); > + > if (regs->regs[0]) { > switch(regs->regs[2]) { > case ERESTART_RESTARTBLOCK: > diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c > index d96e912..8afa090 100644 > --- a/arch/mips/math-emu/cp1emu.c > +++ b/arch/mips/math-emu/cp1emu.c > @@ -434,8 +434,8 @@ static int microMIPS32_to_MIPS32(union mips_instruction *insn_ptr) > * a single subroutine should be used across both > * modules. > */ > -static int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > - unsigned long *contpc) > +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > + unsigned long *contpc) > { > union mips_instruction insn = (union mips_instruction)dec_insn.insn; > unsigned int fcr31; > @@ -1268,7 +1268,7 @@ branch_common: > * instruction in the dslot. > */ > sig = mips_dsemul(xcp, ir, > - contpc); > + bcpc, contpc); > if (sig < 0) > break; > if (sig) > @@ -1323,7 +1323,7 @@ branch_common: > * Single step the non-cp1 > * instruction in the dslot > */ > - sig = mips_dsemul(xcp, ir, contpc); > + sig = mips_dsemul(xcp, ir, bcpc, contpc); > if (sig < 0) > break; > if (sig) > diff --git a/arch/mips/math-emu/dsemul.c b/arch/mips/math-emu/dsemul.c > index 4707488..d21af2f 100644 > --- a/arch/mips/math-emu/dsemul.c > +++ b/arch/mips/math-emu/dsemul.c > @@ -1,3 +1,6 @@ > +#include <linux/err.h> > +#include <linux/slab.h> > + > #include <asm/branch.h> > #include <asm/cacheflush.h> > #include <asm/fpu_emulator.h> > @@ -5,43 +8,221 @@ > #include <asm/mipsregs.h> > #include <asm/uaccess.h> > > -#include "ieee754.h" > - > -/* > - * Emulate the arbitrary instruction ir at xcp->cp0_epc. Required when > - * we have to emulate the instruction in a COP1 branch delay slot. Do > - * not change cp0_epc due to the instruction > +/** > + * struct emuframe - The 'emulation' frame structure > + * @emul: The instruction to 'emulate'. > + * @badinst: A break instruction to cause a return to the kernel. > * > - * According to the spec: > - * 1) it shouldn't be a branch :-) > - * 2) it can be a COP instruction :-( > - * 3) if we are tring to run a protected memory space we must take > - * special care on memory access instructions :-( > - */ > - > -/* > - * "Trampoline" return routine to catch exception following > - * execution of delay-slot instruction execution. > + * This structure defines the frames placed within the delay slot emulation > + * page in response to a call to mips_dsemul(). Each thread may be allocated > + * only one frame at any given time. The kernel stores within it the > + * instruction to be 'emulated' followed by a break instruction, then > + * executes the frame in user mode. The break causes a trap to the kernel > + * which leads to do_dsemulret() being called unless the instruction in > + * @emul causes a trap itself, is a branch, or a signal is delivered to > + * the thread. In these cases the allocated frame will either be reused by > + * a subsequent delay slot 'emulation', or be freed during signal delivery or > + * upon thread exit. > + * > + * This approach is used because: > + * > + * - Actually emulating all instructions isn't feasible. We would need to > + * be able to handle instructions from all revisions of the MIPS ISA, > + * all ASEs & all vendor instruction set extensions. This would be a > + * whole lot of work & continual maintenance burden as new instructions > + * are introduced, and in the case of some vendor extensions may not > + * even be possible. Thus we need to take the approach of actually > + * executing the instruction. > + * > + * - We must execute the instruction within user context. If we were to > + * execute the instruction in kernel mode then it would have access to > + * kernel resources without very careful checks, leaving us with a > + * high potential for security or stability issues to arise. > + * > + * - We used to place the frame on the users stack, but this requires > + * that the stack be executable. This is bad for security so the > + * per-process page is now used instead. > + * > + * - The instruction in @emul may be something entirely invalid for a > + * delay slot. The user may (intentionally or otherwise) place a branch > + * in a delay slot, or a kernel mode instruction, or something else > + * which generates an exception. Thus we can't rely upon the break in > + * @badinst always being hit. For this reason we track the index of the > + * frame allocated to each thread, allowing us to clean it up at later > + * points such as signal delivery or thread exit. > + * > + * - The user may generate a fake struct emuframe if they wish, invoking > + * the BRK_MEMU break instruction themselves. We must therefore not > + * trust that BRK_MEMU means there's actually a valid frame allocated > + * to the thread, and must not allow the user to do anything they > + * couldn't already. > */ > - > struct emuframe { > mips_instruction emul; > mips_instruction badinst; > - mips_instruction cookie; > - unsigned long epc; > }; > > -/* > - * Set up an emulation frame for instruction IR, from a delay slot of > - * a branch jumping to CPC. Return 0 if successful, -1 if no emulation > - * required, otherwise a signal number causing a frame setup failure. > - */ > -int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) > +static const int emupage_frame_count = PAGE_SIZE / sizeof(struct emuframe); > + > +static int alloc_emuframe(void) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + unsigned long addr; > + int idx; > + > +retry: > + mutex_lock(&mm_ctx->bd_emupage_mutex); > + > + /* Ensure we have a page allocated for emuframes */ > + if (!mm_ctx->bd_emupage) { > + addr = mmap_region(NULL, STACK_TOP, PAGE_SIZE, > + VM_READ|VM_WRITE|VM_EXEC| > + VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, > + 0); > + if (IS_ERR_VALUE(addr)) { > + idx = BD_EMUFRAME_NONE; > + goto out_unlock; > + } > + > + mm_ctx->bd_emupage = addr; > + pr_debug("allocate emupage at 0x%08lx to %d\n", addr, > + current->pid); > + } > + > + /* Ensure we have an allocation bitmap */ > + if (!mm_ctx->bd_emupage_allocmap) { > + mm_ctx->bd_emupage_allocmap = > + kcalloc(BITS_TO_LONGS(emupage_frame_count), > + sizeof(unsigned long), > + GFP_KERNEL); > + > + if (!mm_ctx->bd_emupage_allocmap) { > + idx = BD_EMUFRAME_NONE; > + goto out_unlock; > + } > + } > + > + /* Attempt to allocate a single bit/frame */ > + idx = bitmap_find_free_region(mm_ctx->bd_emupage_allocmap, > + emupage_frame_count, 0); > + if (idx < 0) { > + /* > + * Failed to allocate a frame. We'll wait until one becomes > + * available. The mutex is unlocked so that other threads > + * actually get the opportunity to free their frames, which > + * means technically the result of bitmap_full may be incorrect. > + * However the worst case is that we repeat all this and end up > + * back here again. > + */ > + mutex_unlock(&mm_ctx->bd_emupage_mutex); > + if (!wait_event_killable(mm_ctx->bd_emupage_queue, > + !bitmap_full(mm_ctx->bd_emupage_allocmap, > + emupage_frame_count))) > + goto retry; > + > + /* Received a fatal signal - just give in */ > + return BD_EMUFRAME_NONE; > + } > + > + /* Success! */ > + pr_debug("allocate emuframe %d to %d\n", idx, current->pid); > +out_unlock: > + mutex_unlock(&mm_ctx->bd_emupage_mutex); > + return idx; > +} > + > +static void free_emuframe(int idx) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + > + mutex_lock(&mm_ctx->bd_emupage_mutex); > + > + pr_debug("free emuframe %d from %d\n", idx, current->pid); > + bitmap_clear(mm_ctx->bd_emupage_allocmap, idx, 1); > + > + /* If some thread is waiting for a frame, now's its chance */ > + wake_up(&mm_ctx->bd_emupage_queue); > + > + mutex_unlock(&mm_ctx->bd_emupage_mutex); > +} > + > +static bool within_emuframe(struct pt_regs *regs) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + > + if (regs->cp0_epc < mm_ctx->bd_emupage) > + return false; > + if (regs->cp0_epc >= (mm_ctx->bd_emupage + PAGE_SIZE)) > + return false; > + > + return true; > +} > + > +bool dsemul_thread_cleanup(void) > +{ > + int fr_idx; > + > + /* Clear any allocated frame, retrieving its index */ > + fr_idx = atomic_xchg(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); > + > + /* If no frame was allocated, we're done */ > + if (fr_idx == BD_EMUFRAME_NONE) > + return false; > + > + /* Free the frame that this thread had allocated */ > + free_emuframe(fr_idx); > + return true; > +} > + > +bool dsemul_thread_rollback(struct pt_regs *regs) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + struct emuframe __user *fr; > + int fr_idx; > + > + /* Do nothing if we're not executing from a frame */ > + if (!within_emuframe(regs)) > + return false; > + > + /* Find the frame being executed */ > + fr_idx = atomic_read(¤t->thread.bd_emu_frame); > + if (fr_idx == BD_EMUFRAME_NONE) > + return false; > + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; > + > + /* > + * If the PC is at the emul instruction, roll back to the branch. If > + * PC is at the badinst (break) instruction, we've already emulated the > + * instruction so progress to the continue PC. If it's anything else > + * then something is amiss. > + */ > + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) > + regs->cp0_epc = current->thread.bd_emu_branch_pc; > + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->badinst) > + regs->cp0_epc = current->thread.bd_emu_cont_pc; > + else > + return false; Would that lead to bd_emu_frame getting stuck? Matt > + > + atomic_set(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); > + free_emuframe(fr_idx); > + return true; > +} > + > +void dsemul_mm_cleanup(struct mm_struct *mm) > +{ > + mm_context_t *mm_ctx = &mm->context; > + > + kfree(mm_ctx->bd_emupage_allocmap); > +} > + > +int mips_dsemul(struct pt_regs *regs, mips_instruction ir, > + unsigned long branch_pc, unsigned long cont_pc) > { > int isa16 = get_isa16_mode(regs->cp0_epc); > mips_instruction break_math; > + mm_context_t *mm_ctx = ¤t->mm->context; > struct emuframe __user *fr; > - int err; > + int err, fr_idx; > > /* NOP is easy */ > if (ir == 0) > @@ -68,30 +249,20 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) > } > } > > - pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc); > + pr_debug("dsemul 0x%08lx cont at 0x%08lx\n", regs->cp0_epc, cont_pc); > > - /* > - * The strategy is to push the instruction onto the user stack > - * and put a trap after it which we can catch and jump to > - * the required address any alternative apart from full > - * instruction emulation!!. > - * > - * Algorithmics used a system call instruction, and > - * borrowed that vector. MIPS/Linux version is a bit > - * more heavyweight in the interests of portability and > - * multiprocessor support. For Linux we use a BREAK 514 > - * instruction causing a breakpoint exception. > - */ > - break_math = BREAK_MATH(isa16); > - > - /* Ensure that the two instructions are in the same cache line */ > - fr = (struct emuframe __user *) > - ((regs->regs[29] - sizeof(struct emuframe)) & ~0x7); > - > - /* Verify that the stack pointer is not completely insane */ > - if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe)))) > + /* Allocate a frame if we don't already have one */ > + fr_idx = atomic_read(¤t->thread.bd_emu_frame); > + if (fr_idx == BD_EMUFRAME_NONE) > + fr_idx = alloc_emuframe(); > + if (fr_idx == BD_EMUFRAME_NONE) > return SIGBUS; > + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; > + > + /* Retrieve the appropriately encoded break instruction */ > + break_math = BREAK_MATH(isa16); > > + /* Write the instructions to the frame */ > if (isa16) { > err = __put_user(ir >> 16, > (u16 __user *)(&fr->emul)); > @@ -106,84 +277,36 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) > err |= __put_user(break_math, &fr->badinst); > } > > - err |= __put_user((mips_instruction)BD_COOKIE, &fr->cookie); > - err |= __put_user(cpc, &fr->epc); > - > if (unlikely(err)) { > MIPS_FPU_EMU_INC_STATS(errors); > + free_emuframe(fr_idx); > return SIGBUS; > } > > + /* Record the PC of the branch, PC to continue from & frame index */ > + current->thread.bd_emu_branch_pc = branch_pc; > + current->thread.bd_emu_cont_pc = cont_pc; > + atomic_set(¤t->thread.bd_emu_frame, fr_idx); > + > + /* Change user register context to execute the frame */ > regs->cp0_epc = (unsigned long)&fr->emul | isa16; > > + /* Ensure the icache observes our newly written frame */ > flush_cache_sigtramp((unsigned long)&fr->emul); > > return 0; > } > > -int do_dsemulret(struct pt_regs *xcp) > +bool do_dsemulret(struct pt_regs *xcp) > { > - int isa16 = get_isa16_mode(xcp->cp0_epc); > - struct emuframe __user *fr; > - unsigned long epc; > - u32 insn, cookie; > - int err = 0; > - u16 instr[2]; > - > - fr = (struct emuframe __user *) > - (msk_isa16_mode(xcp->cp0_epc) - sizeof(mips_instruction)); > - > - /* > - * If we can't even access the area, something is very wrong, but we'll > - * leave that to the default handling > - */ > - if (!access_ok(VERIFY_READ, fr, sizeof(struct emuframe))) > - return 0; > - > - /* > - * Do some sanity checking on the stackframe: > - * > - * - Is the instruction pointed to by the EPC an BREAK_MATH? > - * - Is the following memory word the BD_COOKIE? > - */ > - if (isa16) { > - err = __get_user(instr[0], > - (u16 __user *)(&fr->badinst)); > - err |= __get_user(instr[1], > - (u16 __user *)((long)(&fr->badinst) + 2)); > - insn = (instr[0] << 16) | instr[1]; > - } else { > - err = __get_user(insn, &fr->badinst); > - } > - err |= __get_user(cookie, &fr->cookie); > - > - if (unlikely(err || > - insn != BREAK_MATH(isa16) || cookie != BD_COOKIE)) { > + /* Cleanup the allocated frame, returning if there wasn't one */ > + if (!dsemul_thread_cleanup()) { > MIPS_FPU_EMU_INC_STATS(errors); > - return 0; > - } > - > - /* > - * At this point, we are satisfied that it's a BD emulation trap. Yes, > - * a user might have deliberately put two malformed and useless > - * instructions in a row in his program, in which case he's in for a > - * nasty surprise - the next instruction will be treated as a > - * continuation address! Alas, this seems to be the only way that we > - * can handle signals, recursion, and longjmps() in the context of > - * emulating the branch delay instruction. > - */ > - > - pr_debug("dsemulret\n"); > - > - if (__get_user(epc, &fr->epc)) { /* Saved EPC */ > - /* This is not a good situation to be in */ > - force_sig(SIGBUS, current); > - > - return 0; > + return false; > } > > /* Set EPC to return to post-branch instruction */ > - xcp->cp0_epc = epc; > - MIPS_FPU_EMU_INC_STATS(ds_emul); > - return 1; > + xcp->cp0_epc = current->thread.bd_emu_cont_pc; > + pr_debug("dsemulret to 0x%08lx\n", xcp->cp0_epc); > + return true; > } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 9:01 ` Matt Redfearn 0 siblings, 0 replies; 22+ messages in thread From: Matt Redfearn @ 2016-06-30 9:01 UTC (permalink / raw) To: Paul Burton, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham Hi Paul On 29/06/16 15:38, Paul Burton wrote: > In some cases the kernel needs to execute an instruction from the delay > slot of an emulated branch instruction. These cases include: > > - Emulated floating point branch instructions (bc1[ft]l?) for systems > which don't include an FPU, or upon which the kernel is run with the > "nofpu" parameter. > > - MIPSr6 systems running binaries targeting older revisions of the > architecture, which may include branch instructions whose encodings > are no longer valid in MIPSr6. > > Executing instructions from such delay slots is done by writing the > instruction to memory followed by a trap, as part of an "emuframe", and > executing it. This avoids the requirement of an emulator for the entire > MIPS instruction set. Prior to this patch such emuframes are written to > the user stack and executed from there. > > This patch moves FP branch delay emuframes off of the user stack and > into a per-mm page. Allocating a page per-mm leaves userland with access > to only what it had access to previously, and compared to other > solutions is relatively simple. > > When a thread requires a delay slot emulation, it is allocated a frame. > A thread may only have one frame allocated at any one time, since it may > only ever be executing one instruction at any one time. In order to > ensure that we can free up allocated frame later, its index is recorded > in struct thread_struct. In the typical case, after executing the delay > slot instruction we'll execute a break instruction with the BRK_MEMU > code. This traps back to the kernel & leads to a call to do_dsemulret > which frees the allocated frame & moves the user PC back to the > instruction that would have executed following the emulated branch. > In some cases the delay slot instruction may be invalid, such as a > branch, or may trigger an exception. In these cases the BRK_MEMU break > instruction will not be hit. In order to ensure that frames are freed > this patch introduces dsemul_thread_cleanup() and calls it to free any > allocated frame upon thread exit. If the instruction generated an > exception & leads to a signal being delivered to the thread, or indeed > if a signal simply happens to be delivered to the thread whilst it is > executing from the struct emuframe, then we need to take care to exit > the frame appropriately. This is done by either rolling back the user PC > to the branch or advancing it to the continuation PC prior to signal > delivery, using dsemul_thread_rollback(). If this were not done then a > sigreturn would return to the struct emuframe, and if that frame had > meanwhile been used in response to an emulated branch instruction within > the signal handler then we would execute the wrong user code. > > Whilst a user could theoretically place something like a compact branch > to self in a delay slot and cause their thread to become stuck in an > infinite loop with the frame never being deallocated, this would: > > - Only affect the users single process. > > - Be architecturally invalid since there would be a branch in the > delay slot, which is forbidden. > > - Be extremely unlikely to happen by mistake, and provide a program > with no more ability to harm the system than a simple infinite loop > would. > > If a thread requires a delay slot emulation & no frame is available to > it (ie. the process has enough other threads that all frames are > currently in use) then the thread joins a waitqueue. It will sleep until > a frame is freed by another thread in the process. > > Since we now know whether a thread has an allocated frame due to our > tracking of its index, the cookie field of struct emuframe is removed as > we can be more certain whether we have a valid frame. Since a thread may > only ever have a single frame at any given time, the epc field of struct > emuframe is also removed & the PC to continue from is instead stored in > struct thread_struct. Together these changes simplify & shrink struct > emuframe somewhat, allowing twice as many frames to fit into the page > allocated for them. > > The primary benefit of this patch is that we are now free to mark the > user stack non-executable where that is possible. > > Signed-off-by: Paul Burton <paul.burton@imgtec.com> > > --- > v2 of this patch can be found here: > > https://patchwork.linux-mips.org/patch/6125/ > > This has become a higher priority than it was at the time of v2 since > Android has begun marking its stacks non-executable & on MIPSr6 devices > we use mips_dsemul() in the emulation of pre-r6 instructions. Since the > Android NDK MIPS target is MIPS32, this is important to backwards > compatibility for apps on MIPSr6 systems. > > Changes in v3: > - Rebase atop v4.7-rc5. > - Select CONFIG_HAVE_EXIT_THREAD. > - Track allocated frames per thread, allowing cleanup on exit or signal delivery. > - Remove signal blocking & thread information flag now we have other means to ensure frames are cleaned up. > - Introduce asm/dsemul.h to group prototypes & definitions logically. asm/dsemul.h seems to be missing from the patch? > - Avoid using 'fp' in names, since this isn't exclusive to FP branch emulation. > - Document with kerneldoc. > - Return a frame index from alloc_emuframe such that mips_dsemul can record it in struct thread_struct. > - Use bool return type for do_dsemulret rather than a bool-like int. > - Rework commit message. > > Changes in v2: > - s/kernels/kernel's/ > - Use (mm_)isBranchInstr in mips_dsemul rather than duplicating similar logic. > > arch/mips/Kconfig | 1 + > arch/mips/include/asm/fpu_emulator.h | 17 +- > arch/mips/include/asm/mmu.h | 11 ++ > arch/mips/include/asm/mmu_context.h | 7 + > arch/mips/include/asm/processor.h | 18 +- > arch/mips/kernel/mips-r2-to-r6-emul.c | 8 +- > arch/mips/kernel/process.c | 6 + > arch/mips/kernel/signal.c | 8 + > arch/mips/math-emu/cp1emu.c | 8 +- > arch/mips/math-emu/dsemul.c | 343 +++++++++++++++++++++++----------- > 10 files changed, 294 insertions(+), 133 deletions(-) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index ac91939..49a5396 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -64,6 +64,7 @@ config MIPS > select GENERIC_TIME_VSYSCALL > select ARCH_CLOCKSOURCE_DATA > select HANDLE_DOMAIN_IRQ > + select HAVE_EXIT_THREAD > > menu "Machine selection" > > diff --git a/arch/mips/include/asm/fpu_emulator.h b/arch/mips/include/asm/fpu_emulator.h > index 3225c3c..355dc25 100644 > --- a/arch/mips/include/asm/fpu_emulator.h > +++ b/arch/mips/include/asm/fpu_emulator.h > @@ -24,7 +24,7 @@ > #define _ASM_FPU_EMULATOR_H > > #include <linux/sched.h> > -#include <asm/break.h> > +#include <asm/dsemul.h> > #include <asm/thread_info.h> > #include <asm/inst.h> > #include <asm/local.h> > @@ -60,27 +60,16 @@ do { \ > #define MIPS_FPU_EMU_INC_STATS(M) do { } while (0) > #endif /* CONFIG_DEBUG_FS */ > > -extern int mips_dsemul(struct pt_regs *regs, mips_instruction ir, > - unsigned long cpc); > -extern int do_dsemulret(struct pt_regs *xcp); > extern int fpu_emulator_cop1Handler(struct pt_regs *xcp, > struct mips_fpu_struct *ctx, int has_fpu, > void *__user *fault_addr); > int process_fpemu_return(int sig, void __user *fault_addr, > unsigned long fcr31); > +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > + unsigned long *contpc); > int mm_isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > unsigned long *contpc); > > -/* > - * Instruction inserted following the badinst to further tag the sequence > - */ > -#define BD_COOKIE 0x0000bd36 /* tne $0, $0 with baggage */ > - > -/* > - * Break instruction with special math emu break code set > - */ > -#define BREAK_MATH(micromips) (((micromips) ? 0x7 : 0xd) | (BRK_MEMU << 16)) > - > #define SIGNALLING_NAN 0x7ff800007ff80000LL > > static inline void fpu_emulator_init_fpu(void) > diff --git a/arch/mips/include/asm/mmu.h b/arch/mips/include/asm/mmu.h > index 1afa1f9..df6ec07 100644 > --- a/arch/mips/include/asm/mmu.h > +++ b/arch/mips/include/asm/mmu.h > @@ -2,11 +2,22 @@ > #define __ASM_MMU_H > > #include <linux/atomic.h> > +#include <linux/mutex.h> > +#include <linux/wait.h> > > typedef struct { > unsigned long asid[NR_CPUS]; > void *vdso; > atomic_t fp_mode_switching; > + > + /* address of page used to hold FP branch delay emulation frames */ > + unsigned long bd_emupage; > + /* bitmap tracking allocation of fp_bd_emupage */ > + unsigned long *bd_emupage_allocmap; > + /* mutex to be held whilst modifying fp_bd_emupage(_allocmap) */ > + struct mutex bd_emupage_mutex; > + /* wait queue for threads requiring an emuframe */ > + wait_queue_head_t bd_emupage_queue; > } mm_context_t; > > #endif /* __ASM_MMU_H */ > diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h > index fc57e13..174d68a 100644 > --- a/arch/mips/include/asm/mmu_context.h > +++ b/arch/mips/include/asm/mmu_context.h > @@ -16,6 +16,7 @@ > #include <linux/smp.h> > #include <linux/slab.h> > #include <asm/cacheflush.h> > +#include <asm/dsemul.h> > #include <asm/hazards.h> > #include <asm/tlbflush.h> > #include <asm-generic/mm_hooks.h> > @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct mm_struct *mm) > > atomic_set(&mm->context.fp_mode_switching, 0); > > + mm->context.bd_emupage = 0; > + mm->context.bd_emupage_allocmap = NULL; > + mutex_init(&mm->context.bd_emupage_mutex); > + init_waitqueue_head(&mm->context.bd_emupage_queue); > + Should this be wrapped in a CONFIG? We're introducing overhead to every tsk creation even if we won't be making use of the emulation. > return 0; > } > > @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, > */ > static inline void destroy_context(struct mm_struct *mm) > { > + dsemul_mm_cleanup(mm); Ditto. > } > > #define deactivate_mm(tsk, mm) do { } while (0) > diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h > index 7e78b62..0d36c87 100644 > --- a/arch/mips/include/asm/processor.h > +++ b/arch/mips/include/asm/processor.h > @@ -11,12 +11,14 @@ > #ifndef _ASM_PROCESSOR_H > #define _ASM_PROCESSOR_H > > +#include <linux/atomic.h> > #include <linux/cpumask.h> > #include <linux/threads.h> > > #include <asm/cachectl.h> > #include <asm/cpu.h> > #include <asm/cpu-info.h> > +#include <asm/dsemul.h> > #include <asm/mipsregs.h> > #include <asm/prefetch.h> > > @@ -78,7 +80,11 @@ extern unsigned int vced_count, vcei_count; > > #endif > > -#define STACK_TOP (TASK_SIZE & PAGE_MASK) > +/* > + * One page above the stack is used for branch delay slot "emulation". > + * See dsemul.c for details. > + */ > +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) Again, should this be dependent on config? Otherwise we waste a page for every task. > > /* > * This decides where the kernel will search for a free chunk of vm > @@ -256,6 +262,12 @@ struct thread_struct { > > /* Saved fpu/fpu emulator stuff. */ > struct mips_fpu_struct fpu FPU_ALIGN; > + /* Assigned branch delay slot 'emulation' frame */ > + atomic_t bd_emu_frame; > + /* PC of the branch from a branch delay slot 'emulation' */ > + unsigned long bd_emu_branch_pc; > + /* PC to continue from following a branch delay slot 'emulation' */ > + unsigned long bd_emu_cont_pc; > #ifdef CONFIG_MIPS_MT_FPAFF > /* Emulated instruction count */ > unsigned long emulated_fp; > @@ -323,6 +335,10 @@ struct thread_struct { > * FPU affinity state (null if not FPAFF) \ > */ \ > FPAFF_INIT \ > + /* Delay slot emulation */ \ > + .bd_emu_frame = ATOMIC_INIT(BD_EMUFRAME_NONE), \ > + .bd_emu_branch_pc = 0, \ > + .bd_emu_cont_pc = 0, \ > /* \ > * Saved DSP stuff \ > */ \ > diff --git a/arch/mips/kernel/mips-r2-to-r6-emul.c b/arch/mips/kernel/mips-r2-to-r6-emul.c > index 7ff2a55..ef23c61 100644 > --- a/arch/mips/kernel/mips-r2-to-r6-emul.c > +++ b/arch/mips/kernel/mips-r2-to-r6-emul.c > @@ -283,7 +283,7 @@ static int jr_func(struct pt_regs *regs, u32 ir) > err = mipsr6_emul(regs, nir); > if (err > 0) { > regs->cp0_epc = nepc; > - err = mips_dsemul(regs, nir, cepc); > + err = mips_dsemul(regs, nir, epc, cepc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > @@ -1033,7 +1033,7 @@ repeat: > if (nir) { > err = mipsr6_emul(regs, nir); > if (err > 0) { > - err = mips_dsemul(regs, nir, cpc); > + err = mips_dsemul(regs, nir, epc, cpc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > @@ -1082,7 +1082,7 @@ repeat: > if (nir) { > err = mipsr6_emul(regs, nir); > if (err > 0) { > - err = mips_dsemul(regs, nir, cpc); > + err = mips_dsemul(regs, nir, epc, cpc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > @@ -1149,7 +1149,7 @@ repeat: > if (nir) { > err = mipsr6_emul(regs, nir); > if (err > 0) { > - err = mips_dsemul(regs, nir, cpc); > + err = mips_dsemul(regs, nir, epc, cpc); > if (err == SIGILL) > err = SIGEMT; > MIPS_R2_STATS(dsemul); > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > index 813ed78..0fb1e8c 100644 > --- a/arch/mips/kernel/process.c > +++ b/arch/mips/kernel/process.c > @@ -30,6 +30,7 @@ > #include <asm/asm.h> > #include <asm/bootinfo.h> > #include <asm/cpu.h> > +#include <asm/dsemul.h> > #include <asm/dsp.h> > #include <asm/fpu.h> > #include <asm/msa.h> > @@ -73,6 +74,11 @@ void start_thread(struct pt_regs * regs, unsigned long pc, unsigned long sp) > regs->regs[29] = sp; > } > > +void exit_thread(struct task_struct *tsk) > +{ > + dsemul_thread_cleanup(); > +} > + > int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) > { > /* > diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c > index ae42314..9383635 100644 > --- a/arch/mips/kernel/signal.c > +++ b/arch/mips/kernel/signal.c > @@ -772,6 +772,14 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs) > struct mips_abi *abi = current->thread.abi; > void *vdso = current->mm->context.vdso; > > + /* > + * If we were emulating a delay slot instruction, exit that frame such > + * that addresses in the sigframe are as expected for userland and we > + * don't have a problem if we reuse the thread's frame for an > + * instruction within the signal handler. > + */ > + dsemul_thread_rollback(regs); > + > if (regs->regs[0]) { > switch(regs->regs[2]) { > case ERESTART_RESTARTBLOCK: > diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c > index d96e912..8afa090 100644 > --- a/arch/mips/math-emu/cp1emu.c > +++ b/arch/mips/math-emu/cp1emu.c > @@ -434,8 +434,8 @@ static int microMIPS32_to_MIPS32(union mips_instruction *insn_ptr) > * a single subroutine should be used across both > * modules. > */ > -static int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > - unsigned long *contpc) > +int isBranchInstr(struct pt_regs *regs, struct mm_decoded_insn dec_insn, > + unsigned long *contpc) > { > union mips_instruction insn = (union mips_instruction)dec_insn.insn; > unsigned int fcr31; > @@ -1268,7 +1268,7 @@ branch_common: > * instruction in the dslot. > */ > sig = mips_dsemul(xcp, ir, > - contpc); > + bcpc, contpc); > if (sig < 0) > break; > if (sig) > @@ -1323,7 +1323,7 @@ branch_common: > * Single step the non-cp1 > * instruction in the dslot > */ > - sig = mips_dsemul(xcp, ir, contpc); > + sig = mips_dsemul(xcp, ir, bcpc, contpc); > if (sig < 0) > break; > if (sig) > diff --git a/arch/mips/math-emu/dsemul.c b/arch/mips/math-emu/dsemul.c > index 4707488..d21af2f 100644 > --- a/arch/mips/math-emu/dsemul.c > +++ b/arch/mips/math-emu/dsemul.c > @@ -1,3 +1,6 @@ > +#include <linux/err.h> > +#include <linux/slab.h> > + > #include <asm/branch.h> > #include <asm/cacheflush.h> > #include <asm/fpu_emulator.h> > @@ -5,43 +8,221 @@ > #include <asm/mipsregs.h> > #include <asm/uaccess.h> > > -#include "ieee754.h" > - > -/* > - * Emulate the arbitrary instruction ir at xcp->cp0_epc. Required when > - * we have to emulate the instruction in a COP1 branch delay slot. Do > - * not change cp0_epc due to the instruction > +/** > + * struct emuframe - The 'emulation' frame structure > + * @emul: The instruction to 'emulate'. > + * @badinst: A break instruction to cause a return to the kernel. > * > - * According to the spec: > - * 1) it shouldn't be a branch :-) > - * 2) it can be a COP instruction :-( > - * 3) if we are tring to run a protected memory space we must take > - * special care on memory access instructions :-( > - */ > - > -/* > - * "Trampoline" return routine to catch exception following > - * execution of delay-slot instruction execution. > + * This structure defines the frames placed within the delay slot emulation > + * page in response to a call to mips_dsemul(). Each thread may be allocated > + * only one frame at any given time. The kernel stores within it the > + * instruction to be 'emulated' followed by a break instruction, then > + * executes the frame in user mode. The break causes a trap to the kernel > + * which leads to do_dsemulret() being called unless the instruction in > + * @emul causes a trap itself, is a branch, or a signal is delivered to > + * the thread. In these cases the allocated frame will either be reused by > + * a subsequent delay slot 'emulation', or be freed during signal delivery or > + * upon thread exit. > + * > + * This approach is used because: > + * > + * - Actually emulating all instructions isn't feasible. We would need to > + * be able to handle instructions from all revisions of the MIPS ISA, > + * all ASEs & all vendor instruction set extensions. This would be a > + * whole lot of work & continual maintenance burden as new instructions > + * are introduced, and in the case of some vendor extensions may not > + * even be possible. Thus we need to take the approach of actually > + * executing the instruction. > + * > + * - We must execute the instruction within user context. If we were to > + * execute the instruction in kernel mode then it would have access to > + * kernel resources without very careful checks, leaving us with a > + * high potential for security or stability issues to arise. > + * > + * - We used to place the frame on the users stack, but this requires > + * that the stack be executable. This is bad for security so the > + * per-process page is now used instead. > + * > + * - The instruction in @emul may be something entirely invalid for a > + * delay slot. The user may (intentionally or otherwise) place a branch > + * in a delay slot, or a kernel mode instruction, or something else > + * which generates an exception. Thus we can't rely upon the break in > + * @badinst always being hit. For this reason we track the index of the > + * frame allocated to each thread, allowing us to clean it up at later > + * points such as signal delivery or thread exit. > + * > + * - The user may generate a fake struct emuframe if they wish, invoking > + * the BRK_MEMU break instruction themselves. We must therefore not > + * trust that BRK_MEMU means there's actually a valid frame allocated > + * to the thread, and must not allow the user to do anything they > + * couldn't already. > */ > - > struct emuframe { > mips_instruction emul; > mips_instruction badinst; > - mips_instruction cookie; > - unsigned long epc; > }; > > -/* > - * Set up an emulation frame for instruction IR, from a delay slot of > - * a branch jumping to CPC. Return 0 if successful, -1 if no emulation > - * required, otherwise a signal number causing a frame setup failure. > - */ > -int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) > +static const int emupage_frame_count = PAGE_SIZE / sizeof(struct emuframe); > + > +static int alloc_emuframe(void) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + unsigned long addr; > + int idx; > + > +retry: > + mutex_lock(&mm_ctx->bd_emupage_mutex); > + > + /* Ensure we have a page allocated for emuframes */ > + if (!mm_ctx->bd_emupage) { > + addr = mmap_region(NULL, STACK_TOP, PAGE_SIZE, > + VM_READ|VM_WRITE|VM_EXEC| > + VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, > + 0); > + if (IS_ERR_VALUE(addr)) { > + idx = BD_EMUFRAME_NONE; > + goto out_unlock; > + } > + > + mm_ctx->bd_emupage = addr; > + pr_debug("allocate emupage at 0x%08lx to %d\n", addr, > + current->pid); > + } > + > + /* Ensure we have an allocation bitmap */ > + if (!mm_ctx->bd_emupage_allocmap) { > + mm_ctx->bd_emupage_allocmap = > + kcalloc(BITS_TO_LONGS(emupage_frame_count), > + sizeof(unsigned long), > + GFP_KERNEL); > + > + if (!mm_ctx->bd_emupage_allocmap) { > + idx = BD_EMUFRAME_NONE; > + goto out_unlock; > + } > + } > + > + /* Attempt to allocate a single bit/frame */ > + idx = bitmap_find_free_region(mm_ctx->bd_emupage_allocmap, > + emupage_frame_count, 0); > + if (idx < 0) { > + /* > + * Failed to allocate a frame. We'll wait until one becomes > + * available. The mutex is unlocked so that other threads > + * actually get the opportunity to free their frames, which > + * means technically the result of bitmap_full may be incorrect. > + * However the worst case is that we repeat all this and end up > + * back here again. > + */ > + mutex_unlock(&mm_ctx->bd_emupage_mutex); > + if (!wait_event_killable(mm_ctx->bd_emupage_queue, > + !bitmap_full(mm_ctx->bd_emupage_allocmap, > + emupage_frame_count))) > + goto retry; > + > + /* Received a fatal signal - just give in */ > + return BD_EMUFRAME_NONE; > + } > + > + /* Success! */ > + pr_debug("allocate emuframe %d to %d\n", idx, current->pid); > +out_unlock: > + mutex_unlock(&mm_ctx->bd_emupage_mutex); > + return idx; > +} > + > +static void free_emuframe(int idx) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + > + mutex_lock(&mm_ctx->bd_emupage_mutex); > + > + pr_debug("free emuframe %d from %d\n", idx, current->pid); > + bitmap_clear(mm_ctx->bd_emupage_allocmap, idx, 1); > + > + /* If some thread is waiting for a frame, now's its chance */ > + wake_up(&mm_ctx->bd_emupage_queue); > + > + mutex_unlock(&mm_ctx->bd_emupage_mutex); > +} > + > +static bool within_emuframe(struct pt_regs *regs) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + > + if (regs->cp0_epc < mm_ctx->bd_emupage) > + return false; > + if (regs->cp0_epc >= (mm_ctx->bd_emupage + PAGE_SIZE)) > + return false; > + > + return true; > +} > + > +bool dsemul_thread_cleanup(void) > +{ > + int fr_idx; > + > + /* Clear any allocated frame, retrieving its index */ > + fr_idx = atomic_xchg(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); > + > + /* If no frame was allocated, we're done */ > + if (fr_idx == BD_EMUFRAME_NONE) > + return false; > + > + /* Free the frame that this thread had allocated */ > + free_emuframe(fr_idx); > + return true; > +} > + > +bool dsemul_thread_rollback(struct pt_regs *regs) > +{ > + mm_context_t *mm_ctx = ¤t->mm->context; > + struct emuframe __user *fr; > + int fr_idx; > + > + /* Do nothing if we're not executing from a frame */ > + if (!within_emuframe(regs)) > + return false; > + > + /* Find the frame being executed */ > + fr_idx = atomic_read(¤t->thread.bd_emu_frame); > + if (fr_idx == BD_EMUFRAME_NONE) > + return false; > + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; > + > + /* > + * If the PC is at the emul instruction, roll back to the branch. If > + * PC is at the badinst (break) instruction, we've already emulated the > + * instruction so progress to the continue PC. If it's anything else > + * then something is amiss. > + */ > + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) > + regs->cp0_epc = current->thread.bd_emu_branch_pc; > + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->badinst) > + regs->cp0_epc = current->thread.bd_emu_cont_pc; > + else > + return false; Would that lead to bd_emu_frame getting stuck? Matt > + > + atomic_set(¤t->thread.bd_emu_frame, BD_EMUFRAME_NONE); > + free_emuframe(fr_idx); > + return true; > +} > + > +void dsemul_mm_cleanup(struct mm_struct *mm) > +{ > + mm_context_t *mm_ctx = &mm->context; > + > + kfree(mm_ctx->bd_emupage_allocmap); > +} > + > +int mips_dsemul(struct pt_regs *regs, mips_instruction ir, > + unsigned long branch_pc, unsigned long cont_pc) > { > int isa16 = get_isa16_mode(regs->cp0_epc); > mips_instruction break_math; > + mm_context_t *mm_ctx = ¤t->mm->context; > struct emuframe __user *fr; > - int err; > + int err, fr_idx; > > /* NOP is easy */ > if (ir == 0) > @@ -68,30 +249,20 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) > } > } > > - pr_debug("dsemul %lx %lx\n", regs->cp0_epc, cpc); > + pr_debug("dsemul 0x%08lx cont at 0x%08lx\n", regs->cp0_epc, cont_pc); > > - /* > - * The strategy is to push the instruction onto the user stack > - * and put a trap after it which we can catch and jump to > - * the required address any alternative apart from full > - * instruction emulation!!. > - * > - * Algorithmics used a system call instruction, and > - * borrowed that vector. MIPS/Linux version is a bit > - * more heavyweight in the interests of portability and > - * multiprocessor support. For Linux we use a BREAK 514 > - * instruction causing a breakpoint exception. > - */ > - break_math = BREAK_MATH(isa16); > - > - /* Ensure that the two instructions are in the same cache line */ > - fr = (struct emuframe __user *) > - ((regs->regs[29] - sizeof(struct emuframe)) & ~0x7); > - > - /* Verify that the stack pointer is not completely insane */ > - if (unlikely(!access_ok(VERIFY_WRITE, fr, sizeof(struct emuframe)))) > + /* Allocate a frame if we don't already have one */ > + fr_idx = atomic_read(¤t->thread.bd_emu_frame); > + if (fr_idx == BD_EMUFRAME_NONE) > + fr_idx = alloc_emuframe(); > + if (fr_idx == BD_EMUFRAME_NONE) > return SIGBUS; > + fr = &((struct emuframe __user *)mm_ctx->bd_emupage)[fr_idx]; > + > + /* Retrieve the appropriately encoded break instruction */ > + break_math = BREAK_MATH(isa16); > > + /* Write the instructions to the frame */ > if (isa16) { > err = __put_user(ir >> 16, > (u16 __user *)(&fr->emul)); > @@ -106,84 +277,36 @@ int mips_dsemul(struct pt_regs *regs, mips_instruction ir, unsigned long cpc) > err |= __put_user(break_math, &fr->badinst); > } > > - err |= __put_user((mips_instruction)BD_COOKIE, &fr->cookie); > - err |= __put_user(cpc, &fr->epc); > - > if (unlikely(err)) { > MIPS_FPU_EMU_INC_STATS(errors); > + free_emuframe(fr_idx); > return SIGBUS; > } > > + /* Record the PC of the branch, PC to continue from & frame index */ > + current->thread.bd_emu_branch_pc = branch_pc; > + current->thread.bd_emu_cont_pc = cont_pc; > + atomic_set(¤t->thread.bd_emu_frame, fr_idx); > + > + /* Change user register context to execute the frame */ > regs->cp0_epc = (unsigned long)&fr->emul | isa16; > > + /* Ensure the icache observes our newly written frame */ > flush_cache_sigtramp((unsigned long)&fr->emul); > > return 0; > } > > -int do_dsemulret(struct pt_regs *xcp) > +bool do_dsemulret(struct pt_regs *xcp) > { > - int isa16 = get_isa16_mode(xcp->cp0_epc); > - struct emuframe __user *fr; > - unsigned long epc; > - u32 insn, cookie; > - int err = 0; > - u16 instr[2]; > - > - fr = (struct emuframe __user *) > - (msk_isa16_mode(xcp->cp0_epc) - sizeof(mips_instruction)); > - > - /* > - * If we can't even access the area, something is very wrong, but we'll > - * leave that to the default handling > - */ > - if (!access_ok(VERIFY_READ, fr, sizeof(struct emuframe))) > - return 0; > - > - /* > - * Do some sanity checking on the stackframe: > - * > - * - Is the instruction pointed to by the EPC an BREAK_MATH? > - * - Is the following memory word the BD_COOKIE? > - */ > - if (isa16) { > - err = __get_user(instr[0], > - (u16 __user *)(&fr->badinst)); > - err |= __get_user(instr[1], > - (u16 __user *)((long)(&fr->badinst) + 2)); > - insn = (instr[0] << 16) | instr[1]; > - } else { > - err = __get_user(insn, &fr->badinst); > - } > - err |= __get_user(cookie, &fr->cookie); > - > - if (unlikely(err || > - insn != BREAK_MATH(isa16) || cookie != BD_COOKIE)) { > + /* Cleanup the allocated frame, returning if there wasn't one */ > + if (!dsemul_thread_cleanup()) { > MIPS_FPU_EMU_INC_STATS(errors); > - return 0; > - } > - > - /* > - * At this point, we are satisfied that it's a BD emulation trap. Yes, > - * a user might have deliberately put two malformed and useless > - * instructions in a row in his program, in which case he's in for a > - * nasty surprise - the next instruction will be treated as a > - * continuation address! Alas, this seems to be the only way that we > - * can handle signals, recursion, and longjmps() in the context of > - * emulating the branch delay instruction. > - */ > - > - pr_debug("dsemulret\n"); > - > - if (__get_user(epc, &fr->epc)) { /* Saved EPC */ > - /* This is not a good situation to be in */ > - force_sig(SIGBUS, current); > - > - return 0; > + return false; > } > > /* Set EPC to return to post-branch instruction */ > - xcp->cp0_epc = epc; > - MIPS_FPU_EMU_INC_STATS(ds_emul); > - return 1; > + xcp->cp0_epc = current->thread.bd_emu_cont_pc; > + pr_debug("dsemulret to 0x%08lx\n", xcp->cp0_epc); > + return true; > } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 10:17 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-30 10:17 UTC (permalink / raw) To: Matt Redfearn, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham On 30/06/16 10:01, Matt Redfearn wrote: > Hi Paul Hi Matt :) >> @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct >> mm_struct *mm) >> atomic_set(&mm->context.fp_mode_switching, 0); >> + mm->context.bd_emupage = 0; >> + mm->context.bd_emupage_allocmap = NULL; >> + mutex_init(&mm->context.bd_emupage_mutex); >> + init_waitqueue_head(&mm->context.bd_emupage_queue); >> + > > Should this be wrapped in a CONFIG? We're introducing overhead to every > tsk creation even if we won't be making use of the emulation. Since this is used by the FPU emulator, which we always include in the kernel, no I'd say not. The overhead here should be very small - all the actual memory allocation & book-keeping is left until a delay slot emulation is actually performed, we're effectively just setting some variables to sane & constant initial values here. In order to guarantee that we don't use this code we'd need to guarantee that we don't use the FPU emulator (or MIPSr2 emulation on a MIPSr6 system) and right now at least there is no way for us to do that. We could add an option to build the kernel without the emulator, but then we'd have issues even if we run on a system with an FPU that doesn't implement certain operations (eg. denormals), so in practice I think that would mean we'd be much better with an option to disable use of FP entirely. That is probably something it would be useful to have anyway for people who know their userland is entirely soft-float, but it's not something I think this patch should do. >> @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct >> *prev, struct mm_struct *next, >> */ >> static inline void destroy_context(struct mm_struct *mm) >> { >> + dsemul_mm_cleanup(mm); > Ditto. Likewise. >> -#define STACK_TOP (TASK_SIZE & PAGE_MASK) >> +/* >> + * One page above the stack is used for branch delay slot "emulation". >> + * See dsemul.c for details. >> + */ >> +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) > > Again, should this be dependent on config? Otherwise we waste a page for > every task. Likewise. Note that we don't actually use a page of memory for every process, we just reserve a page of its virtual address space. The actual memory allocation only occurs (in alloc_emuframe) if we perform a delay slot emulation. >> + /* >> + * If the PC is at the emul instruction, roll back to the branch. If >> + * PC is at the badinst (break) instruction, we've already >> emulated the >> + * instruction so progress to the continue PC. If it's anything else >> + * then something is amiss. >> + */ >> + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) >> + regs->cp0_epc = current->thread.bd_emu_branch_pc; >> + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned >> long)&fr->badinst) >> + regs->cp0_epc = current->thread.bd_emu_cont_pc; >> + else >> + return false; > Would that lead to bd_emu_frame getting stuck? The only way I can think of this case possibly being hit would be if the user code tried to trick the kernel into thinking it was executing from a struct emuframe when it actually wasn't - ie. some time after executing an actual delay slot emulation & preventing the badinst break instruction being hit (either by a well timed signal or an exception & signal generating instruction) the user would need to either manually write to the page & jump into it, or jump to a frame that some other thread had set up (and either way another thread may clobber what it's executing at any moment). If it does try that then yes, the thread's actual frame might persist until the thread either requires another emulation or exits. Worst case scenario the process has one less frame available to it, which I don't see as being an issue. We probably could just free the allocated frame anyway in this case, but actually perhaps SIGKILL would be more suitable. BTW, a way to prevent one of the above scenarios would be ideal future work: it would be good for security if the user could only execute the page, and the kernel could only write it. That would probably involve having aliased mappings, but would be nice to add later. Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 10:17 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-30 10:17 UTC (permalink / raw) To: Matt Redfearn, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham On 30/06/16 10:01, Matt Redfearn wrote: > Hi Paul Hi Matt :) >> @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct >> mm_struct *mm) >> atomic_set(&mm->context.fp_mode_switching, 0); >> + mm->context.bd_emupage = 0; >> + mm->context.bd_emupage_allocmap = NULL; >> + mutex_init(&mm->context.bd_emupage_mutex); >> + init_waitqueue_head(&mm->context.bd_emupage_queue); >> + > > Should this be wrapped in a CONFIG? We're introducing overhead to every > tsk creation even if we won't be making use of the emulation. Since this is used by the FPU emulator, which we always include in the kernel, no I'd say not. The overhead here should be very small - all the actual memory allocation & book-keeping is left until a delay slot emulation is actually performed, we're effectively just setting some variables to sane & constant initial values here. In order to guarantee that we don't use this code we'd need to guarantee that we don't use the FPU emulator (or MIPSr2 emulation on a MIPSr6 system) and right now at least there is no way for us to do that. We could add an option to build the kernel without the emulator, but then we'd have issues even if we run on a system with an FPU that doesn't implement certain operations (eg. denormals), so in practice I think that would mean we'd be much better with an option to disable use of FP entirely. That is probably something it would be useful to have anyway for people who know their userland is entirely soft-float, but it's not something I think this patch should do. >> @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct >> *prev, struct mm_struct *next, >> */ >> static inline void destroy_context(struct mm_struct *mm) >> { >> + dsemul_mm_cleanup(mm); > Ditto. Likewise. >> -#define STACK_TOP (TASK_SIZE & PAGE_MASK) >> +/* >> + * One page above the stack is used for branch delay slot "emulation". >> + * See dsemul.c for details. >> + */ >> +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) > > Again, should this be dependent on config? Otherwise we waste a page for > every task. Likewise. Note that we don't actually use a page of memory for every process, we just reserve a page of its virtual address space. The actual memory allocation only occurs (in alloc_emuframe) if we perform a delay slot emulation. >> + /* >> + * If the PC is at the emul instruction, roll back to the branch. If >> + * PC is at the badinst (break) instruction, we've already >> emulated the >> + * instruction so progress to the continue PC. If it's anything else >> + * then something is amiss. >> + */ >> + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) >> + regs->cp0_epc = current->thread.bd_emu_branch_pc; >> + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned >> long)&fr->badinst) >> + regs->cp0_epc = current->thread.bd_emu_cont_pc; >> + else >> + return false; > Would that lead to bd_emu_frame getting stuck? The only way I can think of this case possibly being hit would be if the user code tried to trick the kernel into thinking it was executing from a struct emuframe when it actually wasn't - ie. some time after executing an actual delay slot emulation & preventing the badinst break instruction being hit (either by a well timed signal or an exception & signal generating instruction) the user would need to either manually write to the page & jump into it, or jump to a frame that some other thread had set up (and either way another thread may clobber what it's executing at any moment). If it does try that then yes, the thread's actual frame might persist until the thread either requires another emulation or exits. Worst case scenario the process has one less frame available to it, which I don't see as being an issue. We probably could just free the allocated frame anyway in this case, but actually perhaps SIGKILL would be more suitable. BTW, a way to prevent one of the above scenarios would be ideal future work: it would be good for security if the user could only execute the page, and the kernel could only write it. That would probably involve having aliased mappings, but would be nice to add later. Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 10:40 ` Matt Redfearn 0 siblings, 0 replies; 22+ messages in thread From: Matt Redfearn @ 2016-06-30 10:40 UTC (permalink / raw) To: Paul Burton, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham On 30/06/16 11:17, Paul Burton wrote: > On 30/06/16 10:01, Matt Redfearn wrote: >> Hi Paul > > Hi Matt :) > >>> @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct >>> mm_struct *mm) >>> atomic_set(&mm->context.fp_mode_switching, 0); >>> + mm->context.bd_emupage = 0; >>> + mm->context.bd_emupage_allocmap = NULL; >>> + mutex_init(&mm->context.bd_emupage_mutex); >>> + init_waitqueue_head(&mm->context.bd_emupage_queue); >>> + >> >> Should this be wrapped in a CONFIG? We're introducing overhead to every >> tsk creation even if we won't be making use of the emulation. > > Since this is used by the FPU emulator, which we always include in the > kernel, no I'd say not. The overhead here should be very small - all > the actual memory allocation & book-keeping is left until a delay slot > emulation is actually performed, we're effectively just setting some > variables to sane & constant initial values here. > > In order to guarantee that we don't use this code we'd need to > guarantee that we don't use the FPU emulator (or MIPSr2 emulation on a > MIPSr6 system) and right now at least there is no way for us to do > that. We could add an option to build the kernel without the emulator, > but then we'd have issues even if we run on a system with an FPU that > doesn't implement certain operations (eg. denormals), so in practice I > think that would mean we'd be much better with an option to disable > use of FP entirely. That is probably something it would be useful to > have anyway for people who know their userland is entirely soft-float, > but it's not something I think this patch should do. > >>> @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct >>> *prev, struct mm_struct *next, >>> */ >>> static inline void destroy_context(struct mm_struct *mm) >>> { >>> + dsemul_mm_cleanup(mm); >> Ditto. > > Likewise. > >>> -#define STACK_TOP (TASK_SIZE & PAGE_MASK) >>> +/* >>> + * One page above the stack is used for branch delay slot "emulation". >>> + * See dsemul.c for details. >>> + */ >>> +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) >> >> Again, should this be dependent on config? Otherwise we waste a page for >> every task. > > Likewise. Note that we don't actually use a page of memory for every > process, we just reserve a page of its virtual address space. The > actual memory allocation only occurs (in alloc_emuframe) if we perform > a delay slot emulation. > >>> + /* >>> + * If the PC is at the emul instruction, roll back to the >>> branch. If >>> + * PC is at the badinst (break) instruction, we've already >>> emulated the >>> + * instruction so progress to the continue PC. If it's anything >>> else >>> + * then something is amiss. >>> + */ >>> + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) >>> + regs->cp0_epc = current->thread.bd_emu_branch_pc; >>> + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned >>> long)&fr->badinst) >>> + regs->cp0_epc = current->thread.bd_emu_cont_pc; >>> + else >>> + return false; >> Would that lead to bd_emu_frame getting stuck? > > The only way I can think of this case possibly being hit would be if > the user code tried to trick the kernel into thinking it was executing > from a struct emuframe when it actually wasn't - ie. some time after > executing an actual delay slot emulation & preventing the badinst > break instruction being hit (either by a well timed signal or an > exception & signal generating instruction) the user would need to > either manually write to the page & jump into it, or jump to a frame > that some other thread had set up (and either way another thread may > clobber what it's executing at any moment). If it does try that then > yes, the thread's actual frame might persist until the thread either > requires another emulation or exits. Worst case scenario the process > has one less frame available to it, which I don't see as being an > issue. We probably could just free the allocated frame anyway in this > case, but actually perhaps SIGKILL would be more suitable. > > BTW, a way to prevent one of the above scenarios would be ideal future > work: it would be good for security if the user could only execute the > page, and the kernel could only write it. That would probably involve > having aliased mappings, but would be nice to add later. > OK cool, sounds good to me. BTW, there was an additional comment (which I failed to space correctly) that asm/dsemul.h seems to be missing from the patch? Thanks, Matt > Thanks, > Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 10:40 ` Matt Redfearn 0 siblings, 0 replies; 22+ messages in thread From: Matt Redfearn @ 2016-06-30 10:40 UTC (permalink / raw) To: Paul Burton, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham On 30/06/16 11:17, Paul Burton wrote: > On 30/06/16 10:01, Matt Redfearn wrote: >> Hi Paul > > Hi Matt :) > >>> @@ -128,6 +129,11 @@ init_new_context(struct task_struct *tsk, struct >>> mm_struct *mm) >>> atomic_set(&mm->context.fp_mode_switching, 0); >>> + mm->context.bd_emupage = 0; >>> + mm->context.bd_emupage_allocmap = NULL; >>> + mutex_init(&mm->context.bd_emupage_mutex); >>> + init_waitqueue_head(&mm->context.bd_emupage_queue); >>> + >> >> Should this be wrapped in a CONFIG? We're introducing overhead to every >> tsk creation even if we won't be making use of the emulation. > > Since this is used by the FPU emulator, which we always include in the > kernel, no I'd say not. The overhead here should be very small - all > the actual memory allocation & book-keeping is left until a delay slot > emulation is actually performed, we're effectively just setting some > variables to sane & constant initial values here. > > In order to guarantee that we don't use this code we'd need to > guarantee that we don't use the FPU emulator (or MIPSr2 emulation on a > MIPSr6 system) and right now at least there is no way for us to do > that. We could add an option to build the kernel without the emulator, > but then we'd have issues even if we run on a system with an FPU that > doesn't implement certain operations (eg. denormals), so in practice I > think that would mean we'd be much better with an option to disable > use of FP entirely. That is probably something it would be useful to > have anyway for people who know their userland is entirely soft-float, > but it's not something I think this patch should do. > >>> @@ -162,6 +168,7 @@ static inline void switch_mm(struct mm_struct >>> *prev, struct mm_struct *next, >>> */ >>> static inline void destroy_context(struct mm_struct *mm) >>> { >>> + dsemul_mm_cleanup(mm); >> Ditto. > > Likewise. > >>> -#define STACK_TOP (TASK_SIZE & PAGE_MASK) >>> +/* >>> + * One page above the stack is used for branch delay slot "emulation". >>> + * See dsemul.c for details. >>> + */ >>> +#define STACK_TOP ((TASK_SIZE & PAGE_MASK) - PAGE_SIZE) >> >> Again, should this be dependent on config? Otherwise we waste a page for >> every task. > > Likewise. Note that we don't actually use a page of memory for every > process, we just reserve a page of its virtual address space. The > actual memory allocation only occurs (in alloc_emuframe) if we perform > a delay slot emulation. > >>> + /* >>> + * If the PC is at the emul instruction, roll back to the >>> branch. If >>> + * PC is at the badinst (break) instruction, we've already >>> emulated the >>> + * instruction so progress to the continue PC. If it's anything >>> else >>> + * then something is amiss. >>> + */ >>> + if (msk_isa16_mode(regs->cp0_epc) == (unsigned long)&fr->emul) >>> + regs->cp0_epc = current->thread.bd_emu_branch_pc; >>> + else if (msk_isa16_mode(regs->cp0_epc) == (unsigned >>> long)&fr->badinst) >>> + regs->cp0_epc = current->thread.bd_emu_cont_pc; >>> + else >>> + return false; >> Would that lead to bd_emu_frame getting stuck? > > The only way I can think of this case possibly being hit would be if > the user code tried to trick the kernel into thinking it was executing > from a struct emuframe when it actually wasn't - ie. some time after > executing an actual delay slot emulation & preventing the badinst > break instruction being hit (either by a well timed signal or an > exception & signal generating instruction) the user would need to > either manually write to the page & jump into it, or jump to a frame > that some other thread had set up (and either way another thread may > clobber what it's executing at any moment). If it does try that then > yes, the thread's actual frame might persist until the thread either > requires another emulation or exits. Worst case scenario the process > has one less frame available to it, which I don't see as being an > issue. We probably could just free the allocated frame anyway in this > case, but actually perhaps SIGKILL would be more suitable. > > BTW, a way to prevent one of the above scenarios would be ideal future > work: it would be good for security if the user could only execute the > page, and the kernel could only write it. That would probably involve > having aliased mappings, but would be nice to add later. > OK cool, sounds good to me. BTW, there was an additional comment (which I failed to space correctly) that asm/dsemul.h seems to be missing from the patch? Thanks, Matt > Thanks, > Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 10:49 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-30 10:49 UTC (permalink / raw) To: Matt Redfearn Cc: linux-mips, Ralf Baechle, Leonid Yegoshin, Matthew Fortune, Raghu Gandham On 30/06/16 11:40, Matt Redfearn wrote: > OK cool, sounds good to me. BTW, there was an additional comment (which > I failed to space correctly) that asm/dsemul.h seems to be missing from > the patch? > > Thanks, > Matt D'oh! Included in v4. Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions @ 2016-06-30 10:49 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-30 10:49 UTC (permalink / raw) To: Matt Redfearn Cc: linux-mips, Ralf Baechle, Leonid Yegoshin, Matthew Fortune, Raghu Gandham On 30/06/16 11:40, Matt Redfearn wrote: > OK cool, sounds good to me. BTW, there was an additional comment (which > I failed to space correctly) that asm/dsemul.h seems to be missing from > the patch? > > Thanks, > Matt D'oh! Included in v4. Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present @ 2016-06-29 14:38 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-29 14:38 UTC (permalink / raw) To: linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham, Paul Burton The stack and heap have both been executable by default on MIPS until now. This patch changes the default to be non-executable, but only for ELF binaries with a non-executable PT_GNU_STACK header present. This does apply to both the heap & the stack, despite the name PT_GNU_STACK, and this matches the behaviour of other architectures like ARM & x86. Current MIPS toolchains do not produce the PT_GNU_STACK header, which means that we can rely upon this patch not changing the behaviour of existing binaries. The new default will only take effect for newly compiled binaries once toolchains are updated to support PT_GNU_STACK, and since those binaries are newly compiled they can be compiled expecting the change in default behaviour. Again this matches the way in which the ARM & x86 architectures handled their implementations of non-executable memory. Signed-off-by: Paul Burton <paul.burton@imgtec.com> --- Changes in v3: - Rebase atop v4.7-rc5. Changes in v2: None arch/mips/include/asm/elf.h | 5 +++++ arch/mips/include/asm/page.h | 6 ++++-- arch/mips/kernel/elf.c | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h index f5f4571..914981d 100644 --- a/arch/mips/include/asm/elf.h +++ b/arch/mips/include/asm/elf.h @@ -498,4 +498,9 @@ extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr, extern void mips_set_personality_nan(struct arch_elf_state *state); extern void mips_set_personality_fp(struct arch_elf_state *state); +#define elf_read_implies_exec(ex, stk) mips_elf_read_implies_exec(&(ex), stk) +struct elf32_hdr; +extern int mips_elf_read_implies_exec(const struct elf32_hdr *elf_ex, + int exstack); + #endif /* _ASM_ELF_H */ diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h index 21ed715..74cb004 100644 --- a/arch/mips/include/asm/page.h +++ b/arch/mips/include/asm/page.h @@ -229,8 +229,10 @@ extern int __virt_addr_valid(const volatile void *kaddr); #define virt_addr_valid(kaddr) \ __virt_addr_valid((const volatile void *) (kaddr)) -#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \ - VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) +#define VM_DATA_DEFAULT_FLAGS \ + (VM_READ | VM_WRITE | \ + ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \ + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) #define UNCAC_ADDR(addr) ((addr) - PAGE_OFFSET + UNCAC_BASE) #define CAC_ADDR(addr) ((addr) - UNCAC_BASE + PAGE_OFFSET) diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c index 891f5ee..9aa55b8 100644 --- a/arch/mips/kernel/elf.c +++ b/arch/mips/kernel/elf.c @@ -8,9 +8,12 @@ * option) any later version. */ +#include <linux/binfmts.h> #include <linux/elf.h> +#include <linux/export.h> #include <linux/sched.h> +#include <asm/cpu-features.h> #include <asm/cpu-info.h> /* Whether to accept legacy-NaN and 2008-NaN user binaries. */ @@ -326,3 +329,19 @@ void mips_set_personality_nan(struct arch_elf_state *state) BUG(); } } + +int mips_elf_read_implies_exec(const struct elf32_hdr *elf_ex, int exstack) +{ + if (exstack != EXSTACK_DISABLE_X) { + /* The binary doesn't request a non-executable stack */ + return 1; + } + + if (!cpu_has_rixi) { + /* The CPU doesn't support non-executable memory */ + return 1; + } + + return 0; +} +EXPORT_SYMBOL(mips_elf_read_implies_exec); -- 2.9.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present @ 2016-06-29 14:38 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-06-29 14:38 UTC (permalink / raw) To: linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Matthew Fortune, Raghu Gandham, Paul Burton The stack and heap have both been executable by default on MIPS until now. This patch changes the default to be non-executable, but only for ELF binaries with a non-executable PT_GNU_STACK header present. This does apply to both the heap & the stack, despite the name PT_GNU_STACK, and this matches the behaviour of other architectures like ARM & x86. Current MIPS toolchains do not produce the PT_GNU_STACK header, which means that we can rely upon this patch not changing the behaviour of existing binaries. The new default will only take effect for newly compiled binaries once toolchains are updated to support PT_GNU_STACK, and since those binaries are newly compiled they can be compiled expecting the change in default behaviour. Again this matches the way in which the ARM & x86 architectures handled their implementations of non-executable memory. Signed-off-by: Paul Burton <paul.burton@imgtec.com> --- Changes in v3: - Rebase atop v4.7-rc5. Changes in v2: None arch/mips/include/asm/elf.h | 5 +++++ arch/mips/include/asm/page.h | 6 ++++-- arch/mips/kernel/elf.c | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h index f5f4571..914981d 100644 --- a/arch/mips/include/asm/elf.h +++ b/arch/mips/include/asm/elf.h @@ -498,4 +498,9 @@ extern int arch_check_elf(void *ehdr, bool has_interpreter, void *interp_ehdr, extern void mips_set_personality_nan(struct arch_elf_state *state); extern void mips_set_personality_fp(struct arch_elf_state *state); +#define elf_read_implies_exec(ex, stk) mips_elf_read_implies_exec(&(ex), stk) +struct elf32_hdr; +extern int mips_elf_read_implies_exec(const struct elf32_hdr *elf_ex, + int exstack); + #endif /* _ASM_ELF_H */ diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h index 21ed715..74cb004 100644 --- a/arch/mips/include/asm/page.h +++ b/arch/mips/include/asm/page.h @@ -229,8 +229,10 @@ extern int __virt_addr_valid(const volatile void *kaddr); #define virt_addr_valid(kaddr) \ __virt_addr_valid((const volatile void *) (kaddr)) -#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \ - VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) +#define VM_DATA_DEFAULT_FLAGS \ + (VM_READ | VM_WRITE | \ + ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \ + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) #define UNCAC_ADDR(addr) ((addr) - PAGE_OFFSET + UNCAC_BASE) #define CAC_ADDR(addr) ((addr) - UNCAC_BASE + PAGE_OFFSET) diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c index 891f5ee..9aa55b8 100644 --- a/arch/mips/kernel/elf.c +++ b/arch/mips/kernel/elf.c @@ -8,9 +8,12 @@ * option) any later version. */ +#include <linux/binfmts.h> #include <linux/elf.h> +#include <linux/export.h> #include <linux/sched.h> +#include <asm/cpu-features.h> #include <asm/cpu-info.h> /* Whether to accept legacy-NaN and 2008-NaN user binaries. */ @@ -326,3 +329,19 @@ void mips_set_personality_nan(struct arch_elf_state *state) BUG(); } } + +int mips_elf_read_implies_exec(const struct elf32_hdr *elf_ex, int exstack) +{ + if (exstack != EXSTACK_DISABLE_X) { + /* The binary doesn't request a non-executable stack */ + return 1; + } + + if (!cpu_has_rixi) { + /* The CPU doesn't support non-executable memory */ + return 1; + } + + return 0; +} +EXPORT_SYMBOL(mips_elf_read_implies_exec); -- 2.9.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* RE: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-29 14:38 ` Paul Burton (?) @ 2016-06-30 9:25 ` Matthew Fortune 2016-06-30 10:34 ` Paul Burton -1 siblings, 1 reply; 22+ messages in thread From: Matthew Fortune @ 2016-06-30 9:25 UTC (permalink / raw) To: Paul Burton, linux-mips, Ralf Baechle Cc: Leonid Yegoshin, Raghu Gandham, Faraz Shahbazker, Maciej Rozycki Paul Burton <Paul.Burton@imgtec.com> writes: > The stack and heap have both been executable by default on MIPS until > now. This patch changes the default to be non-executable, but only for > ELF binaries with a non-executable PT_GNU_STACK header present. This > does apply to both the heap & the stack, despite the name PT_GNU_STACK, > and this matches the behaviour of other architectures like ARM & x86. > > Current MIPS toolchains do not produce the PT_GNU_STACK header, which > means that we can rely upon this patch not changing the behaviour of > existing binaries. The new default will only take effect for newly > compiled binaries once toolchains are updated to support PT_GNU_STACK, > and since those binaries are newly compiled they can be compiled > expecting the change in default behaviour. Again this matches the way in > which the ARM & x86 architectures handled their implementations of > non-executable memory. There will be some extra work on top of this to inform user-mode that no-exec-stack support is actually safe. I'm a bit fuzzy on the exact details though as I have not been directly involved for a while. https://www.sourceware.org/ml/libc-alpha/2016-01/msg00719.html Adding Faraz who worked on the user-mode side and Maciej who has been reviewing. Thanks, Matthew > > Signed-off-by: Paul Burton <paul.burton@imgtec.com> > > --- > > Changes in v3: > - Rebase atop v4.7-rc5. > > Changes in v2: None > > arch/mips/include/asm/elf.h | 5 +++++ > arch/mips/include/asm/page.h | 6 ++++-- > arch/mips/kernel/elf.c | 19 +++++++++++++++++++ > 3 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h > index f5f4571..914981d 100644 > --- a/arch/mips/include/asm/elf.h > +++ b/arch/mips/include/asm/elf.h > @@ -498,4 +498,9 @@ extern int arch_check_elf(void *ehdr, bool has_interpreter, void > *interp_ehdr, > extern void mips_set_personality_nan(struct arch_elf_state *state); > extern void mips_set_personality_fp(struct arch_elf_state *state); > > +#define elf_read_implies_exec(ex, stk) mips_elf_read_implies_exec(&(ex), stk) > +struct elf32_hdr; > +extern int mips_elf_read_implies_exec(const struct elf32_hdr *elf_ex, > + int exstack); > + > #endif /* _ASM_ELF_H */ > diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h > index 21ed715..74cb004 100644 > --- a/arch/mips/include/asm/page.h > +++ b/arch/mips/include/asm/page.h > @@ -229,8 +229,10 @@ extern int __virt_addr_valid(const volatile void *kaddr); > #define virt_addr_valid(kaddr) \ > __virt_addr_valid((const volatile void *) (kaddr)) > > -#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \ > - VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) > +#define VM_DATA_DEFAULT_FLAGS \ > + (VM_READ | VM_WRITE | \ > + ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \ > + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC) > > #define UNCAC_ADDR(addr) ((addr) - PAGE_OFFSET + UNCAC_BASE) > #define CAC_ADDR(addr) ((addr) - UNCAC_BASE + PAGE_OFFSET) > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c > index 891f5ee..9aa55b8 100644 > --- a/arch/mips/kernel/elf.c > +++ b/arch/mips/kernel/elf.c > @@ -8,9 +8,12 @@ > * option) any later version. > */ > > +#include <linux/binfmts.h> > #include <linux/elf.h> > +#include <linux/export.h> > #include <linux/sched.h> > > +#include <asm/cpu-features.h> > #include <asm/cpu-info.h> > > /* Whether to accept legacy-NaN and 2008-NaN user binaries. */ > @@ -326,3 +329,19 @@ void mips_set_personality_nan(struct arch_elf_state *state) > BUG(); > } > } > + > +int mips_elf_read_implies_exec(const struct elf32_hdr *elf_ex, int exstack) > +{ > + if (exstack != EXSTACK_DISABLE_X) { > + /* The binary doesn't request a non-executable stack */ > + return 1; > + } > + > + if (!cpu_has_rixi) { > + /* The CPU doesn't support non-executable memory */ > + return 1; > + } > + > + return 0; > +} > +EXPORT_SYMBOL(mips_elf_read_implies_exec); > -- > 2.9.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-30 9:25 ` Matthew Fortune @ 2016-06-30 10:34 ` Paul Burton 2016-06-30 12:04 ` Matthew Fortune 0 siblings, 1 reply; 22+ messages in thread From: Paul Burton @ 2016-06-30 10:34 UTC (permalink / raw) To: Matthew Fortune, Faraz Shahbazker Cc: linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham, Maciej Rozycki On 30/06/16 10:25, Matthew Fortune wrote: > Paul Burton <Paul.Burton@imgtec.com> writes: >> The stack and heap have both been executable by default on MIPS until >> now. This patch changes the default to be non-executable, but only for >> ELF binaries with a non-executable PT_GNU_STACK header present. This >> does apply to both the heap & the stack, despite the name PT_GNU_STACK, >> and this matches the behaviour of other architectures like ARM & x86. >> >> Current MIPS toolchains do not produce the PT_GNU_STACK header, which >> means that we can rely upon this patch not changing the behaviour of >> existing binaries. The new default will only take effect for newly >> compiled binaries once toolchains are updated to support PT_GNU_STACK, >> and since those binaries are newly compiled they can be compiled >> expecting the change in default behaviour. Again this matches the way in >> which the ARM & x86 architectures handled their implementations of >> non-executable memory. > > There will be some extra work on top of this to inform user-mode that > no-exec-stack support is actually safe. I'm a bit fuzzy on the exact > details though as I have not been directly involved for a while. > > https://www.sourceware.org/ml/libc-alpha/2016-01/msg00719.html > > Adding Faraz who worked on the user-mode side and Maciej who has been > reviewing. Hi Matthew, Interesting. That glibc patch seems to imply that the kernel already supports this, which isn't true. It makes use of a "AV_FLAGS_MIPS_GNU_STACK" constant & says that's taken from Linux, but it doesn't exist. Are you using an experimental patch for the kernel side (perhaps Leonid's?). I'm not sure why you'd use AT_FLAGS for this, which Linux currently unconditionally sets to 0 for all architectures. Would a HWCAP bit for RIXI perhaps be more suitable? Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-30 10:34 ` Paul Burton @ 2016-06-30 12:04 ` Matthew Fortune 2016-06-30 16:25 ` Paul Burton 0 siblings, 1 reply; 22+ messages in thread From: Matthew Fortune @ 2016-06-30 12:04 UTC (permalink / raw) To: Paul Burton, Faraz Shahbazker Cc: linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham, Maciej Rozycki Paul Burton <Paul.Burton@imgtec.com> writes: > On 30/06/16 10:25, Matthew Fortune wrote: > > Paul Burton <Paul.Burton@imgtec.com> writes: > >> The stack and heap have both been executable by default on MIPS until > >> now. This patch changes the default to be non-executable, but only > >> for ELF binaries with a non-executable PT_GNU_STACK header present. > >> This does apply to both the heap & the stack, despite the name > >> PT_GNU_STACK, and this matches the behaviour of other architectures > like ARM & x86. > >> > >> Current MIPS toolchains do not produce the PT_GNU_STACK header, which > >> means that we can rely upon this patch not changing the behaviour of > >> existing binaries. The new default will only take effect for newly > >> compiled binaries once toolchains are updated to support > >> PT_GNU_STACK, and since those binaries are newly compiled they can be > >> compiled expecting the change in default behaviour. Again this > >> matches the way in which the ARM & x86 architectures handled their > >> implementations of non-executable memory. > > > > There will be some extra work on top of this to inform user-mode that > > no-exec-stack support is actually safe. I'm a bit fuzzy on the exact > > details though as I have not been directly involved for a while. > > > > https://www.sourceware.org/ml/libc-alpha/2016-01/msg00719.html > > > > Adding Faraz who worked on the user-mode side and Maciej who has been > > reviewing. > > Hi Matthew, > > Interesting. That glibc patch seems to imply that the kernel already > supports this, which isn't true. It makes use of a > "AV_FLAGS_MIPS_GNU_STACK" constant & says that's taken from Linux, but > it doesn't exist. Are you using an experimental patch for the kernel > side (perhaps Leonid's?). I'm not sure why you'd use AT_FLAGS for this, > which Linux currently unconditionally sets to 0 for all architectures. > Would a HWCAP bit for RIXI perhaps be more suitable? We are/were under the assumption that a pre-existing kernel will honor the PT_GNU_STACK marker overriding the arch specific read-implies-exec logic: http://lxr.free-electrons.com/source/fs/binfmt_elf.c?v=3.2#L689 http://lxr.free-electrons.com/source/fs/exec.c?v=3.2#L703 This means that if we produce tools which have PT_GNU_STACK with executable stack disabled then older kernels will crash as they will obey it and the FPU emulator will break. Is this incorrect? I.e. does today's MIPS kernel do absolutely nothing when PT_GNU_STACK is seen? The "AV_FLAGS_MIPS_GNU_STACK" is a proposal of how to handle the transition from a kernel that does as I describe above to one that safely supports no-exec-stack. I don't know if this has to be an AT_FLAGS or whether a HWCAP would do. I think we perhaps latched on to the idea of AT_FLAGS as we have used that as part of the nan2008 work but we can work through that detail later. The user-mode work is still very much in review. There is no kernel with this feature yet. Faraz: Did I explain that correctly? Matthew ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-30 12:04 ` Matthew Fortune @ 2016-06-30 16:25 ` Paul Burton 2016-06-30 17:40 ` Faraz Shahbazker 0 siblings, 1 reply; 22+ messages in thread From: Paul Burton @ 2016-06-30 16:25 UTC (permalink / raw) To: Matthew Fortune, Faraz Shahbazker Cc: linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham, Maciej Rozycki On 30/06/16 13:04, Matthew Fortune wrote: >>> There will be some extra work on top of this to inform user-mode that >>> no-exec-stack support is actually safe. I'm a bit fuzzy on the exact >>> details though as I have not been directly involved for a while. >>> >>> https://www.sourceware.org/ml/libc-alpha/2016-01/msg00719.html >>> >>> Adding Faraz who worked on the user-mode side and Maciej who has been >>> reviewing. >> >> Hi Matthew, >> >> Interesting. That glibc patch seems to imply that the kernel already >> supports this, which isn't true. It makes use of a >> "AV_FLAGS_MIPS_GNU_STACK" constant & says that's taken from Linux, but >> it doesn't exist. Are you using an experimental patch for the kernel >> side (perhaps Leonid's?). I'm not sure why you'd use AT_FLAGS for this, >> which Linux currently unconditionally sets to 0 for all architectures. >> Would a HWCAP bit for RIXI perhaps be more suitable? > > We are/were under the assumption that a pre-existing kernel will honor > the PT_GNU_STACK marker overriding the arch specific read-implies-exec > logic: > > http://lxr.free-electrons.com/source/fs/binfmt_elf.c?v=3.2#L689 > http://lxr.free-electrons.com/source/fs/exec.c?v=3.2#L703 > > This means that if we produce tools which have PT_GNU_STACK with executable > stack disabled then older kernels will crash as they will obey it and > the FPU emulator will break. > > Is this incorrect? I.e. does today's MIPS kernel do absolutely nothing > when PT_GNU_STACK is seen? Hi Matthew, No I believe what you've described there is correct, existing kernels on systems with RIXI will make the stack non-executable for binaries with a non-executable PT_GNU_STACK, and that will cause problems for the kernels delay slot emulation code (used by the FPU emulator & pre-MIPSr6 emulation on MIPSr6 systems). > The "AV_FLAGS_MIPS_GNU_STACK" is a proposal of how to handle the > transition from a kernel that does as I describe above to one that safely > supports no-exec-stack. I don't know if this has to be an AT_FLAGS or > whether a HWCAP would do. I think we perhaps latched on to the idea of > AT_FLAGS as we have used that as part of the nan2008 work but we can > work through that detail later. The user-mode work is still very much in > review. There is no kernel with this feature yet. This part is probably something we need to discuss - though I suppose it could if needed go in after these kernel changes, just not before them. Having gone digging I see Maciej used AT_FLAGS in some NaN support that hasn't yet been submitted for merging. There was this RFC: https://patchwork.linux-mips.org/patch/11490/ https://patchwork.linux-mips.org/patch/11491/ https://patchwork.linux-mips.org/patch/11492/ https://patchwork.linux-mips.org/patch/11493/ ...but no non-RFC submission so the code isn't yet in the kernel. Right now AT_FLAGS is unconditionally 0 in Linux, for all architectures: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_elf.c?id=v4.7-rc5#n241 Presuming it's still the plan to get this NaN code into mainline the use of AT_FLAGS seems less odd to me. I still think a HWCAP bit would be a sane alternative though as this patchset can be seen as completing kernel support for RIXI, so indicating that a system properly supports RIXI makes sense (& implies that the stack may be non-executable). > Faraz: Did I explain that correctly? I'd love to get your input on this too Faraz. Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-30 16:25 ` Paul Burton @ 2016-06-30 17:40 ` Faraz Shahbazker 2016-06-30 18:48 ` Maciej W. Rozycki 2016-07-01 0:49 ` David Daney 0 siblings, 2 replies; 22+ messages in thread From: Faraz Shahbazker @ 2016-06-30 17:40 UTC (permalink / raw) To: Paul Burton, Matthew Fortune Cc: linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham, Maciej Rozycki Hi Paul, The user-mode patch hasn't gone upstream either, so it is open to modification. Assuming the kernel currently has no other means of publishing RIXI support, or there is no existing software relying on such publication, then a RIXI bit in HWCAP seems appropriate. @Matthew/@Maciej: If this is a priority, could we swap the libc-abi numbers for IFUNC(4) and non-exec stack(5) to push this change through before IFUNC? I think that is the only reason we've held the user-mode patches for glibc and gcc. A binutils patch with abiversion=5 was committed, so it will need fixing, but its only a one-liner. Regards, Faraz On 06/30/2016 09:25 AM, Paul Burton wrote: > On 30/06/16 13:04, Matthew Fortune wrote: >>>> There will be some extra work on top of this to inform user-mode that >>>> no-exec-stack support is actually safe. I'm a bit fuzzy on the exact >>>> details though as I have not been directly involved for a while. >>>> >>>> https://www.sourceware.org/ml/libc-alpha/2016-01/msg00719.html >>>> >>>> Adding Faraz who worked on the user-mode side and Maciej who has been >>>> reviewing. >>> >>> Hi Matthew, >>> >>> Interesting. That glibc patch seems to imply that the kernel already >>> supports this, which isn't true. It makes use of a >>> "AV_FLAGS_MIPS_GNU_STACK" constant & says that's taken from Linux, but >>> it doesn't exist. Are you using an experimental patch for the kernel >>> side (perhaps Leonid's?). I'm not sure why you'd use AT_FLAGS for this, >>> which Linux currently unconditionally sets to 0 for all architectures. >>> Would a HWCAP bit for RIXI perhaps be more suitable? >> >> We are/were under the assumption that a pre-existing kernel will honor >> the PT_GNU_STACK marker overriding the arch specific read-implies-exec >> logic: >> >> http://lxr.free-electrons.com/source/fs/binfmt_elf.c?v=3.2#L689 >> http://lxr.free-electrons.com/source/fs/exec.c?v=3.2#L703 >> >> This means that if we produce tools which have PT_GNU_STACK with executable >> stack disabled then older kernels will crash as they will obey it and >> the FPU emulator will break. >> >> Is this incorrect? I.e. does today's MIPS kernel do absolutely nothing >> when PT_GNU_STACK is seen? > > Hi Matthew, > > No I believe what you've described there is correct, existing kernels on systems with RIXI will make the stack non-executable for binaries with a non-executable PT_GNU_STACK, and that will cause problems for the kernels delay slot emulation code (used by the FPU emulator & pre-MIPSr6 emulation on MIPSr6 systems). > >> The "AV_FLAGS_MIPS_GNU_STACK" is a proposal of how to handle the >> transition from a kernel that does as I describe above to one that safely >> supports no-exec-stack. I don't know if this has to be an AT_FLAGS or >> whether a HWCAP would do. I think we perhaps latched on to the idea of >> AT_FLAGS as we have used that as part of the nan2008 work but we can >> work through that detail later. The user-mode work is still very much in >> review. There is no kernel with this feature yet. > > This part is probably something we need to discuss - though I suppose it could if needed go in after these kernel changes, just not before them. Having gone digging I see Maciej used AT_FLAGS in some NaN support that hasn't yet been submitted for merging. There was this RFC: > > https://patchwork.linux-mips.org/patch/11490/ > https://patchwork.linux-mips.org/patch/11491/ > https://patchwork.linux-mips.org/patch/11492/ > https://patchwork.linux-mips.org/patch/11493/ > > ...but no non-RFC submission so the code isn't yet in the kernel. Right now AT_FLAGS is unconditionally 0 in Linux, for all architectures: > > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_elf.c?id=v4.7-rc5#n241 > > Presuming it's still the plan to get this NaN code into mainline the use of AT_FLAGS seems less odd to me. I still think a HWCAP bit would be a sane alternative though as this patchset can be seen as completing kernel support for RIXI, so indicating that a system properly supports RIXI makes sense (& implies that the stack may be non-executable). > >> Faraz: Did I explain that correctly? > > I'd love to get your input on this too Faraz. > > Thanks, > Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-30 17:40 ` Faraz Shahbazker @ 2016-06-30 18:48 ` Maciej W. Rozycki 2016-07-01 0:49 ` David Daney 1 sibling, 0 replies; 22+ messages in thread From: Maciej W. Rozycki @ 2016-06-30 18:48 UTC (permalink / raw) To: Faraz Shahbazker Cc: Paul Burton, Matthew Fortune, linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham On Thu, 30 Jun 2016, Faraz Shahbazker wrote: > The user-mode patch hasn't gone upstream either, so it is open to > modification. Assuming the kernel currently has no other means of > publishing RIXI support, or there is no existing software relying on > such publication, then a RIXI bit in HWCAP seems appropriate. I think it depends on whether you advertise a fixed hardware feature or a software feature. For NaN interlinking it is obviously a plain software feature set on a program by program basis, and you can have either mode set depending on the kernel command line and main executable's ELF header flags, irrespectively of what hardware supports. So FLAGS was the natural choice, barring the invention of a new vector entry, and HWCAP was out of question. With HWCAP I'd only advertise fixed features, unchanged from the bootstrap till the shutdown (or crash ;) ), and global to the whole system. > @Matthew/@Maciej: If this is a priority, could we swap the libc-abi > numbers for IFUNC(4) and non-exec stack(5) to push this change through > before IFUNC? I think that is the only reason we've held the user-mode > patches for glibc and gcc. A binutils patch with abiversion=5 was > committed, so it will need fixing, but its only a one-liner. TBH I think ABI versions need to be changed incrementally or otherwise we get a broken run-time environment. E.g. if we used version 4 for IFUNC support now, then non-exec stack binaries marked with version 5 would be considered as supporting IFUNC, while in fact they may not, depending on what binutils version they have been built with. So now we need to use 6 for IFUNC anyway as people may have distributed binaries with version 5 recorded already, which obviously do not support IFUNC as it hasn't been upstreamed yet. I'd rather we didn't have this hole, but we can't change history and I just hope we won't go out of range with `e_ident[EI_ABIVERSION]', which is a byte only, anytime soon. NB from the ELF specification it looks to me like this field has been selected for recording ELF binary features in error, where `e_version' should have. This is because the former specifies the ELF *header* version (i.e. changes to the `ElfXX_Ehdr' structure itself, presumably except from `e_ident' itself or at least its first 7 bytes) while it's `e_version' that was meant for the version of the ELF object file (i.e. everything beyond the header). They just both happened to be set to EV_CURRENT or 1 in the original SVR4 ELF gABI, which may have confused someone. To say nothing of `e_version' being 32-bit, providing much more room for manoeuvre. But again, we can't change history and undo the bad choice. Maciej ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-06-30 17:40 ` Faraz Shahbazker 2016-06-30 18:48 ` Maciej W. Rozycki @ 2016-07-01 0:49 ` David Daney 2016-07-01 17:11 ` Paul Burton 1 sibling, 1 reply; 22+ messages in thread From: David Daney @ 2016-07-01 0:49 UTC (permalink / raw) To: Faraz Shahbazker Cc: Paul Burton, Matthew Fortune, linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham, Maciej Rozycki What ever happens, somebody needs to test kernels with this patch against old distros (Debian for example) to make sure you don't break them. Some of the initial non-exec-stack patch sets intentionally broke compatibility with existing binaries, and that is not acceptable. David Daney On 06/30/2016 10:40 AM, Faraz Shahbazker wrote: > Hi Paul, > > The user-mode patch hasn't gone upstream either, so it is open to modification. Assuming the kernel currently has no other means of publishing RIXI support, or there is no existing software relying on such publication, then a RIXI bit in HWCAP seems appropriate. > > @Matthew/@Maciej: If this is a priority, could we swap the libc-abi numbers for IFUNC(4) and non-exec stack(5) to push this change through before IFUNC? I think that is the only reason we've held the user-mode patches for glibc and gcc. A binutils patch with abiversion=5 was committed, so it will need fixing, but its only a one-liner. > > Regards, > Faraz > > > On 06/30/2016 09:25 AM, Paul Burton wrote: >> On 30/06/16 13:04, Matthew Fortune wrote: >>>>> There will be some extra work on top of this to inform user-mode that >>>>> no-exec-stack support is actually safe. I'm a bit fuzzy on the exact >>>>> details though as I have not been directly involved for a while. >>>>> >>>>> https://www.sourceware.org/ml/libc-alpha/2016-01/msg00719.html >>>>> >>>>> Adding Faraz who worked on the user-mode side and Maciej who has been >>>>> reviewing. >>>> >>>> Hi Matthew, >>>> >>>> Interesting. That glibc patch seems to imply that the kernel already >>>> supports this, which isn't true. It makes use of a >>>> "AV_FLAGS_MIPS_GNU_STACK" constant & says that's taken from Linux, but >>>> it doesn't exist. Are you using an experimental patch for the kernel >>>> side (perhaps Leonid's?). I'm not sure why you'd use AT_FLAGS for this, >>>> which Linux currently unconditionally sets to 0 for all architectures. >>>> Would a HWCAP bit for RIXI perhaps be more suitable? >>> >>> We are/were under the assumption that a pre-existing kernel will honor >>> the PT_GNU_STACK marker overriding the arch specific read-implies-exec >>> logic: >>> >>> http://lxr.free-electrons.com/source/fs/binfmt_elf.c?v=3.2#L689 >>> http://lxr.free-electrons.com/source/fs/exec.c?v=3.2#L703 >>> >>> This means that if we produce tools which have PT_GNU_STACK with executable >>> stack disabled then older kernels will crash as they will obey it and >>> the FPU emulator will break. >>> >>> Is this incorrect? I.e. does today's MIPS kernel do absolutely nothing >>> when PT_GNU_STACK is seen? >> >> Hi Matthew, >> >> No I believe what you've described there is correct, existing kernels on systems with RIXI will make the stack non-executable for binaries with a non-executable PT_GNU_STACK, and that will cause problems for the kernels delay slot emulation code (used by the FPU emulator & pre-MIPSr6 emulation on MIPSr6 systems). >> >>> The "AV_FLAGS_MIPS_GNU_STACK" is a proposal of how to handle the >>> transition from a kernel that does as I describe above to one that safely >>> supports no-exec-stack. I don't know if this has to be an AT_FLAGS or >>> whether a HWCAP would do. I think we perhaps latched on to the idea of >>> AT_FLAGS as we have used that as part of the nan2008 work but we can >>> work through that detail later. The user-mode work is still very much in >>> review. There is no kernel with this feature yet. >> >> This part is probably something we need to discuss - though I suppose it could if needed go in after these kernel changes, just not before them. Having gone digging I see Maciej used AT_FLAGS in some NaN support that hasn't yet been submitted for merging. There was this RFC: >> >> https://patchwork.linux-mips.org/patch/11490/ >> https://patchwork.linux-mips.org/patch/11491/ >> https://patchwork.linux-mips.org/patch/11492/ >> https://patchwork.linux-mips.org/patch/11493/ >> >> ...but no non-RFC submission so the code isn't yet in the kernel. Right now AT_FLAGS is unconditionally 0 in Linux, for all architectures: >> >> >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_elf.c?id=v4.7-rc5#n241 >> >> Presuming it's still the plan to get this NaN code into mainline the use of AT_FLAGS seems less odd to me. I still think a HWCAP bit would be a sane alternative though as this patchset can be seen as completing kernel support for RIXI, so indicating that a system properly supports RIXI makes sense (& implies that the stack may be non-executable). >> >>> Faraz: Did I explain that correctly? >> >> I'd love to get your input on this too Faraz. >> >> Thanks, >> Paul > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present 2016-07-01 0:49 ` David Daney @ 2016-07-01 17:11 ` Paul Burton 0 siblings, 0 replies; 22+ messages in thread From: Paul Burton @ 2016-07-01 17:11 UTC (permalink / raw) To: David Daney Cc: Faraz Shahbazker, Matthew Fortune, linux-mips, Ralf Baechle, Leonid Yegoshin, Raghu Gandham, Maciej Rozycki On 01/07/16 01:49, David Daney wrote: > What ever happens, somebody needs to test kernels with this patch > against old distros (Debian for example) to make sure you don't break them. > > Some of the initial non-exec-stack patch sets intentionally broke > compatibility with existing binaries, and that is not acceptable. Hi David, This patchset definitely doesn't intentionally break backwards compatibility, and I am able to run an old debian squeeze distribution with iceweasel/firefox etc just fine under QEMU with nofpu (patch 6125 which this is derived from was capable of that too). I'm planning to do more testing to try to hammer the multi-threaded case where many frames may be in use for a process simultaneously, and check the pre-MIPSr6 on MIPSr6 emulation, but besides that I think it should be good to go. Thanks, Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-07-01 17:11 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-29 14:38 [RFC PATCH v3 0/2] MIPS non-executable stack support Paul Burton 2016-06-29 14:38 ` Paul Burton 2016-06-29 14:38 ` [RFC PATCH v3 1/2] MIPS: use per-mm page to execute branch delay slot instructions Paul Burton 2016-06-29 14:38 ` Paul Burton 2016-06-30 9:01 ` Matt Redfearn 2016-06-30 9:01 ` Matt Redfearn 2016-06-30 10:17 ` Paul Burton 2016-06-30 10:17 ` Paul Burton 2016-06-30 10:40 ` Matt Redfearn 2016-06-30 10:40 ` Matt Redfearn 2016-06-30 10:49 ` Paul Burton 2016-06-30 10:49 ` Paul Burton 2016-06-29 14:38 ` [RFC PATCH v3 2/2] MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present Paul Burton 2016-06-29 14:38 ` Paul Burton 2016-06-30 9:25 ` Matthew Fortune 2016-06-30 10:34 ` Paul Burton 2016-06-30 12:04 ` Matthew Fortune 2016-06-30 16:25 ` Paul Burton 2016-06-30 17:40 ` Faraz Shahbazker 2016-06-30 18:48 ` Maciej W. Rozycki 2016-07-01 0:49 ` David Daney 2016-07-01 17:11 ` Paul Burton
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.