Re: [PATCH v14 07/12] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK

[Mark Rutland]

On Thu, Jan 24, 2019 at 04:19:43PM +0000, Christophe Leroy 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 beginning of task_struct.
> - The 'cpu' field is now in task_struct, and only exists when
> CONFIG_SMP is active.
> - thread_info doesn't have anymore the 'task' field.
> 
> This patch:
> - Removes all recopy of thread_info struct when the stack changes.
> - Changes the CURRENT_THREAD_INFO() macro to point to current.
> - Selects CONFIG_THREAD_INFO_IN_TASK.
> - Modifies raw_smp_processor_id() to get ->cpu from current without
> including linux/sched.h to avoid circular inclusion and without
> including asm/asm-offsets.h to avoid symbol names duplication
> between ASM constants and C constants.
> - Modifies klp_init_thread_info() to take a task_struct pointer
> argument.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxx>
> Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx>

[...]

> +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))
> +endif

[...]

> -#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.
> + *
> + * This also needs to be safeguarded when building asm-offsets.s because at
> + * that time _TASK_CPU is not defined yet. It could have been guarded by
> + * _TASK_CPU itself, but we want the build to fail if _TASK_CPU is missing
> + * when building something else than asm-offsets.s
> + */
> +#ifdef GENERATING_ASM_OFFSETS
> +#define raw_smp_processor_id()		(0)
> +#else
> +#define raw_smp_processor_id()		(*(unsigned int *)((void *)current + _TASK_CPU))
> +#endif
>  #define hard_smp_processor_id() 	(smp_hw_index[smp_processor_id()])

On arm64 we have the per-cpu offset in a CPU register (TPIDR_EL1), so we
can do:

DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number);

#define raw_smp_processor_id() (*raw_cpu_ptr(&cpu_number))

... but I guess that's not possible on PPC for some reason?

I think I asked that before, but I couldn't find the thread.

Otherwise, this all looks sound to me, but I don't know much about PPC.

Thanks,
Mark.