arm: Preserve the user r/w register TPIDRURW on context, switch and fork
Subsystems (3)
PATCH v6 1/1 arm: Preserve the user r/w register TPIDRURW on context, switch and fork
From: André Hentschel <[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 Windows RT, 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 Linux 3.10-rc6 (7d132055814ef17a6c7b69f342244c410a5e000f)
v2: rework and fixup of v1, based on a suggested patch by Will Deacon
v3: total rework and fixup of v2
v4: removed condition on assembler instruction,
adapted my code to kernel-style, both based on comments by Will Deacon
v5: rebased v4 on 3.10-rc2 and adding this version history
v6: moved loading the TLS registers to the macros
(fixing the "LDRD is not supported on all the CPUs we have" problem)
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 +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 +30 -13
76 lines not on this page · raw message
arch/arm/kernel/entry-armv.S +1 -2
@@ -685,7 +685,6 @@ 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 )
@@ -693,7 +692,7 @@ ENTRY(__switch_to) #ifdef CONFIG_CPU_USE_DOMAINS ldr r6, [r2, #TI_CPU_DOMAIN] #endif - set_tls r3, r4, r5 + switch_tls r1, r2, 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>
@@ -343,7 +344,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
@@ -581,7 +581,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) {
@@ -699,7 +699,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 ; }
Signature · 1 line
1.8.1.2
Quoted 22 lines · Andr? Hentschel
From: Andr? Hentschel <[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 Windows RT, 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.Signed-off-by Andr? Hentschel <[email protected]>Signed-off-by Will Deacon <[email protected]>Signed-off-by Jonathan Austin <[email protected]>---
This patch is against Linux 3.10-rc6 (7d132055814ef17a6c7b69f342244c410a5e000f)
Quoted 7 lines
v2: rework and fixup of v1, based on a suggested patch by Will Deacon
v3: total rework and fixup of v2
v4: removed condition on assembler instruction,
adapted my code to kernel-style, both based on comments by Will Deacon
v5: rebased v4 on 3.10-rc2 and adding this version history
v6: moved loading the TLS registers to the macros
(fixing the "LDRD is not supported on all the CPUs we have" problem)
You've changed quite a lot with this version, including the way the macro parameters are passed. Why not just replace the problematic ldrd with two ldr instructions and be done with it? I don't think the simple build error warrants an overhaul of the code we already had.
Cheers,
Will
Quoted 31 lines · Will Deacon
Quoted 22 lines · Andr? Hentschel
From: Andr? Hentschel <[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 Windows RT, 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.Signed-off-by Andr? Hentschel <[email protected]>Signed-off-by Will Deacon <[email protected]>Signed-off-by Jonathan Austin <[email protected]>---
This patch is against Linux 3.10-rc6 (7d132055814ef17a6c7b69f342244c410a5e000f)Quoted 7 lines
v2: rework and fixup of v1, based on a suggested patch by Will Deacon
v3: total rework and fixup of v2
v4: removed condition on assembler instruction,
adapted my code to kernel-style, both based on comments by Will Deacon
v5: rebased v4 on 3.10-rc2 and adding this version history
v6: moved loading the TLS registers to the macros
(fixing the "LDRD is not supported on all the CPUs we have" problem)
Quoted 4 lines
You've changed quite a lot with this version, including the way the macro parameters are passed. Why not just replace the problematic ldrd with two ldr instructions and be done with it? I don't think the simple build error warrants an overhaul of the code we already had.
Wantig that patch to be in 3.11 i thought i should do more now to push it. I'm still not that familiar with the process, but i think Russell King would have done this easy change himself when he would be happy with it. Further this patch seems cleaner and much likely performing better. I'm off till saturday, hopefully i'm enlighted by feedback till then :) (just to be clear: i'm also happy with replacing ldrd with two ldr instructions)
Quoted 32 lines · André Hentschel
Quoted 31 lines · Will Deacon
Quoted 29 lines · André Hentschel
From: André Hentschel <[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 Windows RT, 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.Signed-off-by André Hentschel <[email protected]>Signed-off-by Will Deacon <[email protected]>Signed-off-by Jonathan Austin <[email protected]>---
This patch is against Linux 3.10-rc6 (7d132055814ef17a6c7b69f342244c410a5e000f)v2: rework and fixup of v1, based on a suggested patch by Will Deacon
v3: total rework and fixup of v2
v4: removed condition on assembler instruction,
adapted my code to kernel-style, both based on comments by Will Deacon
v5: rebased v4 on 3.10-rc2 and adding this version history
v6: moved loading the TLS registers to the macros
(fixing the "LDRD is not supported on all the CPUs we have" problem)
Quoted 6 lines
Quoted 4 lines
You've changed quite a lot with this version, including the way the macro parameters are passed. Why not just replace the problematic ldrd with two ldr instructions and be done with it? I don't think the simple build error warrants an overhaul of the code we already had.
Wantig that patch to be in 3.11 i thought i should do more now to push it.
It's getting pretty late for 3.11, so generally you'd want to make the most minimal changes possible, rather than redesign core parts of the patch.
Quoted 2 lines
I'm still not that familiar with the process, but i think Russell King would have done this easy change himself when he would be happy with it.
That's unlikely -- you're still the author on the patch so I wouldn't expect the gatekeeper to make any changes above merge conflicts.
Quoted 1 line
Further this patch seems cleaner and much likely performing better.
Well, it's not the approach that has been reviewed up until now and, without numbers backing up your claims, I'd be inclined simply to make the simple change of adding two ldr instructions.
Will