Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1601925251.git.luto@kernel.org> <4b3b4fbf8e9806840135e95cef142a0adefc3455.1601925251.git.luto@kernel.org>
In-Reply-To: <4b3b4fbf8e9806840135e95cef142a0adefc3455.1601925251.git.luto@kernel.org>
From:   Brian Gerst <brgerst@gmail.com>
Date:   Tue, 6 Oct 2020 13:14:32 -0400
Message-ID: <CAMzpN2iTceq90wMU7O5jDDq4KH_AWF-AbajNowp_xvkGWxuwCw@mail.gmail.com>
Subject: Re: [PATCH 1/2] x86/stackprotector/32: Make the canary into a regular
 percpu variable
To:     Andy Lutomirski <luto@kernel.org>
Cc:     "the arch/x86 maintainers" <x86@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "Christopherson, Sean J" <sean.j.christopherson@intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Oct 5, 2020 at 3:31 PM Andy Lutomirski <luto@kernel.org> 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
> 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.

This doesn't consider !SMP builds, where per-cpu variable are just
normal variables, and the FS segment is disabled.  Unfortunately,
-mstack-protector-guard-reg=ds is not accepted.  The simplest fix
would be to define  __KERNEL_PERCPU when either SMP or STACKPROTECTOR
are enabled.

--
Brian Gerst