arm: Preserve the user r/w register TPIDRURW on context switch and fork
Subsystems (3)
PATCH v4 1/1 arm: Preserve the user r/w register TPIDRURW on context switch and fork
From: =?UTF-8?q?Andr=C3=A9=20Hentschel?= <[email protected]>
Since commit 6a1c53124aa1 the user writeable TLS register was zeroed to prevent it from being used as a covert channel between two tasks.
There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW.
This patch preserves that register per thread instead of clearing it. Unlike the TPIDRURO, which is already switched, the TPIDRURW
can be updated from userspace so needs careful treatment in the case that we modify TPIDRURW and call fork(). To avoid this we must always read TPIDRURW in copy_thread.
---
This patch is against a86d52667d8eda5de39393ce737794403bdce1eb
Why so much Signed-off-bys? Some History:
The first patch had performance issues pointed out by Russel King, so Will Deacon jumped in to help me with that. The second one again had performance issues and the missing copy_thread part was uncovered. After some iterations by me, Jonathan Austin proposed a patch and Russel King sent his idea of the assembler part. All this was finally merged and refined into this patch.
Thanks to everyone!
arch/arm/include/asm/thread_info.h | 2 +- arch/arm/include/asm/tls.h | 40 ++++++++++++++++++++++++------------ arch/arm/kernel/entry-armv.S | 4 ++-- arch/arm/kernel/process.c | 4 +++- arch/arm/kernel/ptrace.c | 2 +- arch/arm/kernel/traps.c | 4 ++-- 6 files changed, 36 insertions(+), 20 deletions(-)
arch/arm/include/asm/thread_info.h +1 -1
@@ -58,7 +58,7 @@ struct thread_info {struct cpu_context_save cpu_context;/* cpu context */ __u32 syscall;/* syscall number */ __u8 used_cp[16 ];/* thread used copro */ -unsigned long tp_value; +unsigned long tp_value[2 ];/* TLS registers */ #ifdef CONFIG_CRUNCH struct crunch_state crunchstate;#endif
arch/arm/include/asm/tls.h +27 -13
@@ -2,27 +2,30 @@#define __ASMARM_TLS_H#ifdef __ASSEMBLY__ - .macro set_tls_none, tp, tmp1, tmp2 +#include <asm/asm-offsets.h> + .macro switch_tls_none, base, tp, tpuser, tmp1, tmp2 .endm - .macro set_tls_v6k, tp, tmp1, tmp2 + .macro switch_tls_v6k, base, tp, tpuser, tmp1, tmp2 + mrc p15, 0, \tmp2, c13, c0, 2 @ get the user r/w register mcr p15, 0, \tp, c13, c0, 3 @ set TLS register - mov \tmp1, #0 - mcr p15, 0, \tmp1, c13, c0, 2 @ clear user r/w TLS register + mcr p15, 0, \tpuser, c13, c0, 2 @ and the user r/w register + str \tmp2, [\base, #TI_TP_VALUE + 4] @ save it .endm - .macro set_tls_v6, tp, tmp1, tmp2 + .macro switch_tls_v6, base, tp, tpuser, tmp1, tmp2 ldr \tmp1, =elf_hwcap ldr \tmp1, [\tmp1, #0] mov \tmp2, #0xffff0fff tst \tmp1, #HWCAP_TLS @ hardware TLS available? - mcrne p15, 0, \tp, c13, c0, 3 @ yes, set TLS register - movne \tmp1, #0 - mcrne p15, 0, \tmp1, c13, c0, 2 @ clear user r/w TLS register streq \tp, [\tmp2, #-15] @ set TLS value at 0xffff0ff0 + mrcne p15, 0, \tmp2, c13, c0, 2 @ get the user r/w register + mcrne p15, 0, \tp, c13, c0, 3 @ yes, set TLS register + mcrne p15, 0, \tpuser, c13, c0, 2 @ set user r/w register + strne \tmp2, [\base, #TI_TP_VALUE + 4] @ save it .endm - .macro set_tls_software, tp, tmp1, tmp2 + .macro switch_tls_software, base, tp, tpuser, tmp1, tmp2 mov \tmp1, #0xffff0fff str \tp, [\tmp1, #-15] @ set TLS value at 0xffff0ff0 .endm
@@ -31,19 +34,30 @@#ifdef CONFIG_TLS_REG_EMUL #define tls_emu 1#define has_tls_reg 1 -#define set_tls set_tls_none +#define switch_tls switch_tls_none#elif defined(CONFIG_CPU_V6 )#define tls_emu 0#define has_tls_reg (elf_hwcap & HWCAP_TLS) -#define set_tls set_tls_v6 +#define switch_tls switch_tls_v6#elif defined(CONFIG_CPU_32v6K)#define tls_emu 0#define has_tls_reg 1 -#define set_tls set_tls_v6k +#define switch_tls switch_tls_v6k#else #define tls_emu 0#define has_tls_reg 0 -#define set_tls set_tls_software +#define switch_tls switch_tls_software#endif +#ifndef __ASSEMBLY__ +static inline unsigned long get_tpuser (void ) +{ +unsigned long reg =0 ; + +if (has_tls_reg && !tls_emu) + __asm__("mrc p15, 0, %0, c13, c0, 2" :"=r" (reg)); + +return reg; +} +#endif #endif /* __ASMARM_TLS_H */
arch/arm/kernel/entry-armv.S +2 -2
@@ -728,15 +728,15 @@ ENTRY(__switch_to) UNWIND(.fnstart ) UNWIND(.cantunwind ) add ip, r1, #TI_CPU_SAVE - ldr r3, [r2, #TI_TP_VALUE] ARM( stmia ip!, {r4 - sl, fp, sp, lr} ) @ Store most regs on stack THUMB( stmia ip!, {r4 - sl, fp} ) @ Store most regs on stack THUMB( str sp, [ip], #4 ) THUMB( str lr, [ip], #4 ) + ldrd r4, r5, [r2, #TI_TP_VALUE] #ifdef CONFIG_CPU_USE_DOMAINS ldr r6, [r2, #TI_CPU_DOMAIN] #endif - set_tls r3, r4, r5 + switch_tls r1, r4, r5, r3, r7 #if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP) ldr r7, [r2, #TI_TASK] ldr r8, =__stack_chk_guard
arch/arm/kernel/process.c +3 -1
@@ -39,6 +39,7 @@#include <asm/thread_notify.h> #include <asm/stacktrace.h> #include <asm/mach/time.h> +#include <asm/tls.h> #ifdef CONFIG_CC_STACKPROTECTOR #include <linux/stackprotector.h>
@@ -395,7 +396,8 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start,clear_ptrace_hw_breakpoint (p);if (clone_flags &CLONE_SETTLS ) - thread->tp_value = childregs->ARM_r3; + thread->tp_value[0 ] = childregs->ARM_r3; + thread->tp_value[1 ] =get_tpuser ();thread_notify (THREAD_NOTIFY_COPY , thread);
arch/arm/kernel/ptrace.c +1 -1
@@ -849,7 +849,7 @@ long arch_ptrace(struct task_struct *child, long request,#endif case PTRACE_GET_THREAD_AREA : - ret =put_user (task_thread_info (child)->tp_value, + ret =put_user (task_thread_info (child)->tp_value[0 ], datap);break ;
arch/arm/kernel/traps.c +2 -2
@@ -588,7 +588,7 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)return regs->ARM_r0;case NR (set_tls): - thread->tp_value = regs->ARM_r0; + thread->tp_value[0 ] = regs->ARM_r0;if (tls_emu)return 0 ;if (has_tls_reg) {
@@ -706,7 +706,7 @@ static int get_tp_trap(struct pt_regs *regs, unsigned int instr)int reg = (instr >>12 ) &15 ;if (reg ==15 )return 1 ; - regs->uregs[reg] =current_thread_info ()->tp_value; + regs->uregs[reg] =current_thread_info ()->tp_value[0 ]; regs->ARM_pc +=4 ;return 0 ; }
Am 08.05.2013 21:03, schrieb Andr? Hentschel:
Quoted 14 lines
From: =?UTF-8?q?Andr=C3=A9=20Hentschel?= <[email protected]>
Since commit
6a1c53124aa1the user writeable TLS register was zeroed to prevent it from being used as a covert channel between two tasks.There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW.
This patch preserves that register per thread instead of clearing it. Unlike the TPIDRURO, which is already switched, the TPIDRURW
can be updated from userspace so needs careful treatment in the case that we modify TPIDRURW and callfork(). To avoid this we must always read TPIDRURW incopy_thread.
Quoted 4 lines
Signed-off-by Andr? Hentschel <[email protected]>Signed-off-by Will Deacon <[email protected]>Signed-off-by Jonathan Austin <[email protected]>
Hi,
I'm not yet very familiar with the development process here, am i getting no feedback on v4 because of the mergewindow being closed? Or is there another reason? Sry for being impatient.
Hi André
Quoted 3 lines · André Hentschel
Am 08.05.2013 21:03, schrieb André Hentschel:
Quoted 2 lines
From: =?UTF-8?q?Andr=C3=A9=20Hentschel?= <[email protected]>
This is strangely formatted for me too, and I use a different client from Will so I'm not sure that the problem is just at our end...
(Also see that the list archive has weird formatting:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/167325.html )
Your first couple of patches didn't come out strangely formatted, so I'm not really sure what's going on.
Quoted 17 lines
Quoted 12 lines
Since commit
6a1c53124aa1the user writeable TLS register was zeroed to prevent it from being used as a covert channel between two tasks.There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW.
This patch preserves that register per thread instead of clearing it. Unlike the TPIDRURO, which is already switched, the TPIDRURW
can be updated from userspace so needs careful treatment in the case that we modify TPIDRURW and callfork(). To avoid this we must always read TPIDRURW incopy_thread.Quoted 4 lines
Signed-off-by André Hentschel <[email protected]>Signed-off-by Will Deacon <[email protected]>Signed-off-by Jonathan Austin <[email protected]>
Quoted 5 lines
Hi,
I'm not yet very familiar with the development process here, am i getting no feedback on v4 because of the mergewindow being closed? Or is there another reason? Sry for being impatient.
This is a feature, not a fix, so most likely it'll be included at the next merge window. For that to happen it should be in Russell's tree around the middle of this cycle.
Can you please rebase on 3.10-rc2 (when it happens) and post one more version? After that, assuming nobody else has any final comments, you could put it in to Russell's patch system...
Just as a hint - one thing you might have done to increase the chances of getting comments to clarify what's different between v3 and v4 - as a way to make life easier for reviewers you can highlight the differences between versions after the "---" (where you currently have the description of why there are so many S-o-Bs).
Hope that helps,
Jonny
Am 20.05.2013 13:03, schrieb Jonathan Austin:
Quoted 7 lines
Hi Andr?
Quoted 3 lines · Andr? Hentschel
Am 08.05.2013 21:03, schrieb Andr? Hentschel:
Quoted 2 lines
From: =?UTF-8?q?Andr=C3=A9=20Hentschel?= <[email protected]>
Quoted 6 lines
This is strangely formatted for me too, and I use a different client from Will so I'm not sure that the problem is just at our end...
(Also see that the list archive has weird formatting:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/167325.html )Your first couple of patches didn't come out strangely formatted, so I'm not really sure what's going on.
I'll retry it with plain Text or something. AFAIK git should understand this =UTF= style, though.
Quoted 23 lines
Quoted 17 lines
Quoted 12 lines
Since commit
6a1c53124aa1the user writeable TLS register was zeroed to prevent it from being used as a covert channel between two tasks.There are more and more applications coming to WinRT, Wine could support them, but mostly they expect to have the thread environment block (TEB) in TPIDRURW.
This patch preserves that register per thread instead of clearing it. Unlike the TPIDRURO, which is already switched, the TPIDRURW
can be updated from userspace so needs careful treatment in the case that we modify TPIDRURW and callfork(). To avoid this we must always read TPIDRURW incopy_thread.Quoted 4 lines
Signed-off-by Andr? Hentschel <[email protected]>Signed-off-by Will Deacon <[email protected]>Signed-off-by Jonathan Austin <[email protected]>Quoted 5 lines
Hi,
I'm not yet very familiar with the development process here, am i getting no feedback on v4 because of the mergewindow being closed? Or is there another reason? Sry for being impatient.
Quoted 5 lines
This is a feature, not a fix, so most likely it'll be included at the next merge window. For that to happen it should be in Russell's tree around the middle of this cycle.
Can you please rebase on 3.10-rc2 (when it happens) and post one more version? After that, assuming nobody else has any final comments, you could put it in to Russell's patch system...
Just as a hint - one thing you might have done to increase the chances of getting comments to clarify what's different between v3 and v4 - as a way to make life easier for reviewers you can highlight the differences between versions after the "---" (where you currently have the description of why there are so many S-o-Bs).
I'll do, thank you very much for the reply and the clarification.