Re: [PATCH v5 0/9] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
On 10/08/2018 11:06 AM, Michael Ellerman wrote:
Christophe Leroy writes:
The purpose of this serie is to activate CONFIG_THREAD_INFO_IN_TASK which
moves the thread_info into task_struct.
Moving thread_info into task_struct has the following advantages:
- It protects thread_info from corruption in the case of stack
overflows.
- Its address is harder to determine if stack addresses are
leaked, making a number of attacks more difficult.
This is blowing up pretty nicely with CONFIG_KMEMLEAK enabled, haven't
had time to dig further:
Nice :)
I have the same issue on PPC32.
Seems like when descending the stack, save_context_stack() calls
validate_sp(), which in turn calls valid_irq_stack() when the first test
fails.
But than early, hardirq_ctx[cpu] is NULL.
With sp = 0, valid_irq_stack() used to return false because it expected
sp to be above the thread_info. But now that thread_info is gone, sp = 0
is valid when stack = NULL.
The following fixes it:
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index afe76f7f316c..3e534147fd8f 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2006,6 +2006,9 @@ int validate_sp(unsigned long sp, struct
task_struct *p,
{
unsigned long stack_page = (unsigned long)task_stack_page(p);
+ if (sp < THREAD_SIZE)
+ return 0;
+
if (sp >= stack_page + sizeof(struct thread_struct)
&& sp <= stack_page + THREAD_SIZE - nbytes)
return 1;
Looking at this I also realise I forgot to remove the sizeof(struct
thread_struct) from here. And this sizeof() was buggy, it should have
been thread_info instead of thread_struct, but nevermind as it is going
away.
Christophe
Unable to handle kernel paging request for data at address 0x
Faulting instruction address: 0xc0022064
Oops: Kernel access of bad area, sig: 11 [#9]
LE SMP NR_CPUS=32 NUMA
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted
4.19.0-rc3-gcc-7.3.1-00103-gc795acc08338 #268
NIP: c0022064 LR: c00220c0 CTR: c001f5c0
REGS: c1244a50 TRAP: 0380 Not tainted
(4.19.0-rc3-gcc-7.3.1-00103-gc795acc08338)
MSR: 80001033 CR: 48022244 XER: 2000
CFAR: c00220c4 IRQMASK: 1
GPR00: c00220c0 c1244cd0 c124b200 0001
GPR04: c1201180 0070 c1275ef8
GPR08: 0001 3f90 2b6e6f6d6d6f635f
GPR12: c001f5c0 c145 02e2be38
GPR16: 7dc54c70 02d854b8 c0d87f00
GPR20: c0d87ef0 c0d87ee0 c0d87f08 c006c1a8
GPR24: c0d87ec8 7265677368657265 c0062a04
GPR28: 0006 c1201180
NIP [c0022064] show_stack+0xe4/0x2b0
LR [c00220c0] show_stack+0x140/0x2b0
Call Trace:
[c1244cd0] [c002217c] show_stack+0x1fc/0x2b0 (unreliable)
[c1244da0] [c002245c] show_regs+0x22c/0x430
[c1244e50] [c002ae8c] __die+0xfc/0x140
[c1244ed0] [c002b954] die+0x74/0xf0
[c1244f10] [c006e0f8] bad_page_fault+0xe8/0x180
[c1244f80] [c0074f48] slb_miss_large_addr+0x68/0x2e0
[c1244fc0] [c0008ce8] large_addr_slb+0x158/0x160
--- interrupt: 380 at show_stack+0xe4/0x2b0
LR = show_stack+0x140/0x2b0
[c12452b0] [c002217c] show_stack+0x1fc/0x2b0 (unreliable)
[c1245380] [c002245c] show_regs+0x22c/0x430
[c1245430] [c002ae8c] __die+0xfc/0x140
[c12454b0] [c002b954] die+0x74/0xf0
[c12454f0] [c006e0f8] bad_page_fault+0xe8/0x180
[c1245560] [c0074f48] slb_miss_large_addr+0x68/0x2e0
[c12455a0] [c0008ce8] large_addr_slb+0x158/0x160
--- interrupt: 380 at show_stack+0xe4/0x2b0
LR = show_stack+0x140/0x2b0
[c1245890] [c002217c] show_stack+0x1fc/0x2b0 (unreliable)
[c1245960] [c002245c] show_regs+0x22c/0x430
[c1245a10] [c002ae8c] __die+0xfc/0x140
[c1245a90] [c002b954] die+0x74/0xf0
[c1245ad0] [c006e0f8] bad_page_fault+0xe8/0x180
[c1245b40] [c0074f48] slb_miss_large_addr+0x68/0x2e0
[c1245b80] [c0008ce8] large_addr_slb+0x158/0x160
--- interrupt: 380 at show_stack+0xe4/0x2b0
LR = show_stack+0x140/0x2b0
[c1245e70] [c002217c] show_stack+0x1fc/0x2b0 (unreliable)
[c1245f40] [c002245c] show_regs+0x22c/0x430
[c1245ff0] [c002ae8c] __die+0xfc/0x140
[c1246070] [c002b954] die+0x74/0xf0
[c12460b0] [c006e0f8] bad_page_fault+0xe8/0x180
[c1246120] [c0074f48] slb_miss_large_addr+0x68/0x2e0
[c1246160] [c0008ce8] large_addr_slb+0x158/0x160
---
Re: [PATCH v5 0/9] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
Christophe Leroy writes: > The purpose of this serie is to activate CONFIG_THREAD_INFO_IN_TASK which > moves the thread_info into task_struct. > > Moving thread_info into task_struct has the following advantages: > - It protects thread_info from corruption in the case of stack > overflows. > - Its address is harder to determine if stack addresses are > leaked, making a number of attacks more difficult. This is blowing up pretty nicely with CONFIG_KMEMLEAK enabled, haven't had time to dig further: Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0022064 Oops: Kernel access of bad area, sig: 11 [#9] LE SMP NR_CPUS=32 NUMA Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-gcc-7.3.1-00103-gc795acc08338 #268 NIP: c0022064 LR: c00220c0 CTR: c001f5c0 REGS: c1244a50 TRAP: 0380 Not tainted (4.19.0-rc3-gcc-7.3.1-00103-gc795acc08338) MSR: 80001033 CR: 48022244 XER: 2000 CFAR: c00220c4 IRQMASK: 1 GPR00: c00220c0 c1244cd0 c124b200 0001 GPR04: c1201180 0070 c1275ef8 GPR08: 0001 3f90 2b6e6f6d6d6f635f GPR12: c001f5c0 c145 02e2be38 GPR16: 7dc54c70 02d854b8 c0d87f00 GPR20: c0d87ef0 c0d87ee0 c0d87f08 c006c1a8 GPR24: c0d87ec8 7265677368657265 c0062a04 GPR28: 0006 c1201180 NIP [c0022064] show_stack+0xe4/0x2b0 LR [c00220c0] show_stack+0x140/0x2b0 Call Trace: [c1244cd0] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c1244da0] [c002245c] show_regs+0x22c/0x430 [c1244e50] [c002ae8c] __die+0xfc/0x140 [c1244ed0] [c002b954] die+0x74/0xf0 [c1244f10] [c006e0f8] bad_page_fault+0xe8/0x180 [c1244f80] [c0074f48] slb_miss_large_addr+0x68/0x2e0 [c1244fc0] [c0008ce8] large_addr_slb+0x158/0x160 --- interrupt: 380 at show_stack+0xe4/0x2b0 LR = show_stack+0x140/0x2b0 [c12452b0] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c1245380] [c002245c] show_regs+0x22c/0x430 [c1245430] [c002ae8c] __die+0xfc/0x140 [c12454b0] [c002b954] die+0x74/0xf0 [c12454f0] [c006e0f8] bad_page_fault+0xe8/0x180 [c1245560] [c0074f48] slb_miss_large_addr+0x68/0x2e0 [c12455a0] [c0008ce8] large_addr_slb+0x158/0x160 --- interrupt: 380 at show_stack+0xe4/0x2b0 LR = show_stack+0x140/0x2b0 [c1245890] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c1245960] [c002245c] show_regs+0x22c/0x430 [c1245a10] [c002ae8c] __die+0xfc/0x140 [c1245a90] [c002b954] die+0x74/0xf0 [c1245ad0] [c006e0f8] bad_page_fault+0xe8/0x180 [c1245b40] [c0074f48] slb_miss_large_addr+0x68/0x2e0 [c1245b80] [c0008ce8] large_addr_slb+0x158/0x160 --- interrupt: 380 at show_stack+0xe4/0x2b0 LR = show_stack+0x140/0x2b0 [c1245e70] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c1245f40] [c002245c] show_regs+0x22c/0x430 [c1245ff0] [c002ae8c] __die+0xfc/0x140 [c1246070] [c002b954] die+0x74/0xf0 [c12460b0] [c006e0f8] bad_page_fault+0xe8/0x180 [c1246120] [c0074f48] slb_miss_large_addr+0x68/0x2e0 [c1246160] [c0008ce8] large_addr_slb+0x158/0x160 --- interrupt: 380 at show_stack+0xe4/0x2b0 LR = show_stack+0x140/0x2b0 [c1246450] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c1246520] [c002245c] show_regs+0x22c/0x430 [c12465d0] [c002ae8c] __die+0xfc/0x140 [c1246650] [c002b954] die+0x74/0xf0 [c1246690] [c006e0f8] bad_page_fault+0xe8/0x180 [c1246700] [c0074f48] slb_miss_large_addr+0x68/0x2e0 [c1246740] [c0008ce8] large_addr_slb+0x158/0x160 --- interrupt: 380 at show_stack+0xe4/0x2b0 LR = show_stack+0x140/0x2b0 [c1246a30] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c1246b00] [c002245c] show_regs+0x22c/0x430 [c1246bb0] [c002ae8c] __die+0xfc/0x140 [c1246c30] [c002b954] die+0x74/0xf0 [c1246c70] [c006e0f8] bad_page_fault+0xe8/0x180 [c1246ce0] [c0074f48] slb_miss_large_addr+0x68/0x2e0 [c1246d20] [c0008ce8] large_addr_slb+0x158/0x160 --- interrupt: 380 at show_stack+0xe4/0x2b0 LR = show_stack+0x140/0x2b0 [c1247010] [c002217c] show_stack+0x1fc/0x2b0 (unreliable) [c12470e0] [c002245c] show_regs+0x22c/0x430
[PATCH v5 0/9] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
The purpose of this serie is to activate CONFIG_THREAD_INFO_IN_TASK which moves the thread_info into task_struct. Moving thread_info into task_struct has the following advantages: - It protects thread_info from corruption in the case of stack overflows. - Its address is harder to determine if stack addresses are leaked, making a number of attacks more difficult. Changes since v4: - Fixed a build failure on 32bits SMP when include/generated/asm-offsets.h is not already existing, was due to spaces instead of a tab in the Makefile Changes since RFC v3: (based on Nick's review) - Renamed task_size.h to task_size_user64.h to better relate to what it contains. - Handling of the isolation of thread_info cpu field inside CONFIG_SMP #ifdefs moved to a separate patch. - Removed CURRENT_THREAD_INFO macro completely. - Added a guard in asm/smp.h to avoid build failure before _TASK_CPU is defined. - Added a patch at the end to rename 'tp' pointers to 'sp' pointers - Renamed 'tp' into 'sp' pointers in preparation patch when relevant - Fixed a few commit logs - Fixed checkpatch report. Changes since RFC v2: - Removed the modification of names in asm-offsets - Created a rule in arch/powerpc/Makefile to append the offset of current->cpu in CFLAGS - Modified asm/smp.h to use the offset set in CFLAGS - Squashed the renaming of THREAD_INFO to TASK_STACK in the preparation patch - Moved the modification of current_pt_regs in the patch activating CONFIG_THREAD_INFO_IN_TASK Changes since RFC v1: - Removed the first patch which was modifying header inclusion order in timer - Modified some names in asm-offsets to avoid conflicts when including asm-offsets in C files - Modified asm/smp.h to avoid having to include linux/sched.h (using asm-offsets instead) - Moved some changes from the activation patch to the preparation patch. Christophe Leroy (9): book3s/64: avoid circular header inclusion in mmu-hash.h powerpc: Only use task_struct 'cpu' field on SMP powerpc: Prepare for moving thread_info into task_struct powerpc: Activate CONFIG_THREAD_INFO_IN_TASK powerpc: regain entire stack space powerpc: 'current_set' is now a table of task_struct pointers powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU powerpc/64: Remove CURRENT_THREAD_INFO powerpc: clean stack pointers naming arch/powerpc/Kconfig | 1 + arch/powerpc/Makefile | 8 ++- arch/powerpc/include/asm/asm-prototypes.h | 4 +- arch/powerpc/include/asm/book3s/64/mmu-hash.h | 2 +- arch/powerpc/include/asm/exception-64s.h | 4 +- arch/powerpc/include/asm/irq.h | 14 ++--- arch/powerpc/include/asm/livepatch.h | 2 +- arch/powerpc/include/asm/processor.h | 39 + arch/powerpc/include/asm/ptrace.h | 2 +- arch/powerpc/include/asm/reg.h | 2 +- arch/powerpc/include/asm/smp.h | 17 +- arch/powerpc/include/asm/task_size_user64.h| 42 ++ arch/powerpc/include/asm/thread_info.h | 19 --- arch/powerpc/kernel/asm-offsets.c | 10 ++-- arch/powerpc/kernel/entry_32.S | 66 -- arch/powerpc/kernel/entry_64.S | 12 ++-- arch/powerpc/kernel/epapr_hcalls.S | 5 +- arch/powerpc/kernel/exceptions-64e.S | 13 + arch/powerpc/kernel/exceptions-64s.S | 2 +- arch/powerpc/kernel/head_32.S | 14 ++--- arch/powerpc/kernel/head_40x.S | 4 +- arch/powerpc/kernel/head_44x.S | 8 +-- arch/powerpc/kernel/head_64.S | 1 + arch/powerpc/kernel/head_8xx.S | 2 +- arch/powerpc/kernel/head_booke.h | 12 +--- arch/powerpc/kernel/head_fsl_booke.S | 16 +++--- arch/powerpc/kernel/idle_6xx.S | 8 +-- arch/powerpc/kernel/idle_book3e.S | 2 +- arch/powerpc/kernel/idle_e500.S| 8 +-- arch/powerpc/kernel/idle_power4.S | 2 +- arch/powerpc/kernel/irq.c | 77 +- arch/powerpc/kernel/kgdb.c | 28 -- arch/powerpc/kernel/machine_kexec_64.c | 6 +- arch/powerpc/kernel/misc_32.S | 17 +++--- arch/powerpc/kernel/process.c | 15 ++--- arch/powerpc/kernel/setup-common.c | 2 +- arch/powerpc/kernel/setup_32.c | 15 ++--- arch/powerpc/kernel/setup_64.c | 41 -- arch/powerpc/kernel/smp.c | 16 +++--- arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 6 +- arch/powerpc/kvm/book3s_hv_hmi.c | 1 + arch/powerpc/mm/hash_low_32.S | 14 ++--- arch/powerpc/sysdev/6xx-suspend.S | 5 +- arch/powerpc/xmon/xmon.c | 2 +- 44 files changed, 224 insertions(+), 362 deletions(-)
