On Mon, Oct 05, 2020 at 12:30:03PM -0700, Andy Lutomirski wrote:
> On 32-bit kernels, the stackprotector canary is quite nasty -- it is
> stored at %gs:(20), which is nasty because 32-bit kernels use %fs for
> percpu storage. It's even nastier because it means that whether %gs
> contains userspace state or kernel state while running kernel code
> sepends on whether stackprotector is enabled (this is
depends
> CONFIG_X86_32_LAZY_GS), and this setting radically changes the way
> that segment selectors work. Supporting both variants is a
> maintenance and testing mess.
>
> Merely rearranging so that percpu and the stack canary
> share the same segment would be messy as the 32-bit percpu address
> layout isn't currently compatible with putting a variable at a fixed
> offset.
>
> Fortunately, GCC 8.1 added options that allow the stack canary to be
> accessed as %fs:stack_canary, effectively turning it into an ordinary
> percpu variable. This lets us get rid of all of the code to manage
> the stack canary GDT descriptor and the CONFIG_X86_32_LAZY_GS mess.
>
> This patch forcibly disables stackprotector on older compilers that
> don't support the new options and makes the stack canary into a
> percpu variable.
It'd be helpful to explicitly state that the so called "lazy GS" approach is
now always used for i386.
> Signed-off-by: Andy Lutomirski <l...@kernel.org>
> ---
...
> diff --git a/arch/x86/include/asm/suspend_32.h
> b/arch/x86/include/asm/suspend_32.h
> index fdbd9d7b7bca..eb872363ca82 100644
> --- a/arch/x86/include/asm/suspend_32.h
> +++ b/arch/x86/include/asm/suspend_32.h
> @@ -16,9 +16,7 @@ struct saved_context {
> * On x86_32, all segment registers, with the possible exception of
Is this still a "possible" exception, or is it now always an exception?
> * gs, are saved at kernel entry in pt_regs.
> */
> -#ifdef CONFIG_X86_32_LAZY_GS
> u16 gs;
> -#endif
> unsigned long cr0, cr2, cr3, cr4;
> u64 misc_enable;
> bool misc_enable_saved;
...
> diff --git a/arch/x86/kernel/tls.c b/arch/x86/kernel/tls.c
> index 64a496a0687f..3c883e064242 100644
> --- a/arch/x86/kernel/tls.c
> +++ b/arch/x86/kernel/tls.c
> @@ -164,17 +164,11 @@ int do_set_thread_area(struct task_struct *p, int idx,
> savesegment(fs, sel);
> if (sel == modified_sel)
> loadsegment(fs, sel);
> -
> - savesegment(gs, sel);
> - if (sel == modified_sel)
> - load_gs_index(sel);
> #endif
>
> -#ifdef CONFIG_X86_32_LAZY_GS
> savesegment(gs, sel);
> if (sel == modified_sel)
> - loadsegment(gs, sel);
> -#endif
> + load_gs_index(sel);
Side topic, the "index" part of this is super confusing. I had to reread
this entire patch after discovering load_gs_index is loadsegment on i386.
Maybe also worth a shout out in the changelog?
> } else {
> #ifdef CONFIG_X86_64
> if (p->thread.fsindex == modified_sel)
