Re: [RFC PATCH v3 3/7] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-10-03 05:30:34
Also in:
lkml
On Mon, 1 Oct 2018 12:30:23 +0000 (UTC) Christophe Leroy [off-list ref] wrote:
This patch activates 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 has the following consequences: - thread_info is now located at the top of task_struct.
"top"... I got confused for a minute thinking high address and wondering how you can change CURRENT_THREAD_INFO just to point to current :)
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 07d9dce7eda6..4e98989b5512 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile@@ -422,3 +422,9 @@ checkbin: CLEAN_FILES += $(TOUT) +ifdef CONFIG_SMP +prepare: task_cpu_prepare + +task_cpu_prepare: prepare0 + $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TI_CPU") print $$3;}' include/generated/asm-offsets.h)) +endifdiff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index 447cbd1bee99..3a7e5561630b 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h@@ -120,7 +120,7 @@ extern int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data); #define current_pt_regs() \ - ((struct pt_regs *)((unsigned long)current_thread_info() + THREAD_SIZE) - 1) + ((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1) /* * We use the least-significant bit of the trap field to indicate * whether we have saved the full set of registers, or only adiff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index 95b66a0c639b..df519b7322e5 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h@@ -83,7 +83,13 @@ int is_cpu_dead(unsigned int cpu); /* 32-bit */ extern int smp_hw_index[]; -#define raw_smp_processor_id() (current_thread_info()->cpu) +/* + * This is particularly ugly: it appears we can't actually get the definition + * of task_struct here, but we need access to the CPU this task is running on. + * Instead of using task_struct we're using _TASK_CPU which is extracted from + * asm-offsets.h by kbuild to get the current processor ID. + */ +#define raw_smp_processor_id() (*(unsigned int*)((void*)current + _TASK_CPU))
This is clever but yes ugly. Can't you include asm-offsets.h? riscv seems to. I'm not 100% sure on kgdb and kexec stuff but I think it seems okay. Looks like a pretty nice cleanup too aside from the features it brings, thanks for working on it. Reviewed-by: Nicholas Piggin <npiggin@gmail.com>