On Wed, 3 Oct 2018 07:47:05 +0200 Christophe LEROY <christophe.le...@c-s.fr> wrote:
> Le 03/10/2018 à 07:30, Nicholas Piggin a écrit : > > On Mon, 1 Oct 2018 12:30:23 +0000 (UTC) > > Christophe Leroy <christophe.le...@c-s.fr> 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 :) > > Would 'beginning' be less confusing ? Yes, good idea. > >> @@ -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. > > riscv has a clean asm-offsets.h . Our's defines constant with the same > name as those defined in other headers which are included in C files. So > including asm-offsets in C files does create conflicts like: > > ./include/generated/asm-offsets.h:71:0: warning: "TASK_SIZE" redefined > #define TASK_SIZE -2147483648 /* TASK_SIZE */ > ./arch/powerpc/include/asm/processor.h:95:0: note: this is the location > of the previous definition > #define TASK_SIZE (CONFIG_TASK_SIZE) > > ./include/generated/asm-offsets.h:98:0: warning: "NSEC_PER_SEC" redefined > #define NSEC_PER_SEC 1000000000 /* NSEC_PER_SEC */ > ./include/linux/time64.h:36:0: note: this is the location of the > previous definition > #define NSEC_PER_SEC 1000000000L > > ./arch/powerpc/include/asm/nohash/32/pgtable.h:34:0: warning: > "PGD_TABLE_SIZE" redefined > #define PGD_TABLE_SIZE (sizeof(pgd_t) << PGD_INDEX_SIZE) > ./include/generated/asm-offsets.h:101:0: note: this is the location of > the previous definition > #define PGD_TABLE_SIZE 256 /* PGD_TABLE_SIZE */ > > ... Okay. > > In v2, I had a patch to fix those redundancies > (https://patchwork.ozlabs.org/patch/974363/) but I found it unconvenient. Because of merge conflicts, or you did not like the new names? Thanks, Nick
