Received-SPF: pass (google.com: domain of linux-kernel+bounces-38415-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240122090851.851120-7-ardb+git@google.com> <20240122090851.851120-11-ardb+git@google.com>
 <CAMzpN2jcWxCy=H-1uvS7kN8gVohee2_cMwyC0SbSEwEoedo3WQ@mail.gmail.com> <20240122224417.GC141255@dev-fedora.aadp>
In-Reply-To: <20240122224417.GC141255@dev-fedora.aadp>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Thu, 25 Jan 2024 11:43:18 +0100
Message-ID: <CAMj1kXG+97DPc9Ws-5=QrrHmf+KyvfqgoDFwLCV2Afy3ZJQM2Q@mail.gmail.com>
Subject: Re: [RFC PATCH 4/5] x86/head64: Replace pointer fixups with PIE codegen
To: Nathan Chancellor <nathan@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>, Ard Biesheuvel <ardb+git@google.com>, linux-kernel@vger.kernel.org, 
	Kevin Loughlin <kevinloughlin@google.com>, Tom Lendacky <thomas.lendacky@amd.com>, 
	Dionna Glaze <dionnaglaze@google.com>, Thomas Gleixner <tglx@linutronix.de>, 
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	Dave Hansen <dave.hansen@linux.intel.com>, Andy Lutomirski <luto@kernel.org>, 
	Arnd Bergmann <arnd@arndb.de>, Martin KaFai Lau <martin.lau@linux.dev>, 
	Nick Desaulniers <ndesaulniers@google.com>, Justin Stitt <justinstitt@google.com>, 
	linux-arch@vger.kernel.org, bpf@vger.kernel.org, llvm@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, 22 Jan 2024 at 23:44, Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Mon, Jan 22, 2024 at 02:34:46PM -0500, Brian Gerst wrote:
> > On Mon, Jan 22, 2024 at 4:14=E2=80=AFAM Ard Biesheuvel <ardb+git@google=
com> wrote:
> > >
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Some of the C code in head64.c may be called from a different virtual
> > > address than it was linked at. Currently, we deal with this by using
> > > ordinary, position dependent codegen, and fixing up all symbol
> > > references on the fly. This is fragile and tricky to maintain. It is
> > > also unnecessary: we can use position independent codegen (with hidde=
n
> > > visibility) to ensure that all compiler generated symbol references a=
re
> > > RIP-relative, removing the need for fixups entirely.
> > >
> > > It does mean we need explicit references to kernel virtual addresses =
to
> > > be generated by hand, so generate those using a movabs instruction in
> > > inline asm in the handful places where we actually need this.
> > >
> > > While at it, move these routines to .inittext where they belong.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  arch/x86/Makefile                 |  11 ++
> > >  arch/x86/boot/compressed/Makefile |   2 +-
> > >  arch/x86/include/asm/init.h       |   2 -
> > >  arch/x86/include/asm/setup.h      |   2 +-
> > >  arch/x86/kernel/Makefile          |   4 +
> > >  arch/x86/kernel/head64.c          | 117 +++++++-------------
> > >  6 files changed, 60 insertions(+), 78 deletions(-)
> > >
> > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > > index 1a068de12a56..bed0850d91b0 100644
> > > --- a/arch/x86/Makefile
> > > +++ b/arch/x86/Makefile
> > > @@ -168,6 +168,17 @@ else
> > >          KBUILD_CFLAGS +=3D -mcmodel=3Dkernel
> > >          KBUILD_RUSTFLAGS +=3D -Cno-redzone=3Dy
> > >          KBUILD_RUSTFLAGS +=3D -Ccode-model=3Dkernel
> > > +
> > > +       PIE_CFLAGS :=3D -fpie -mcmodel=3Dsmall \
> > > +                     -include $(srctree)/include/linux/hidden.h
> > > +
> > > +       ifeq ($(CONFIG_STACKPROTECTOR),y)
> > > +               ifeq ($(CONFIG_SMP),y)
> > > +                       PIE_CFLAGS +=3D -mstack-protector-guard-reg=
=3Dgs
> > > +               endif
> >
> > This compiler flag requires GCC 8.1 or later.  When I posted a patch
> > series[1] to convert the stack protector to a normal percpu variable
> > instead of the fixed offset, there was pushback over requiring GCC 8.1
> > to keep stack protector support.  I added code to objtool to convert
> > code from older compilers, but there hasn't been any feedback since.
> > Similar conversion code would be needed in objtool for this unless the
> > decision is made to require GCC 8.1 for stack protector support going
> > forward.
> >
> > Brian Gerst
> >
> > [1] https://lore.kernel.org/lkml/20231115173708.108316-1-brgerst@gmail.=
com/
>
> I was going to comment on this as well, as that flag was only supported
> in clang 12.0.0 and newer. It should not be too big of a deal for us
> though, as I was already planning on bumping the minimum supported
> version of clang for building the kernel to 13.0.1 (but there may be
> breakage reports if this series lands before that):
>

Thanks for pointing this out.

Given that building the entire kernel with fPIC is neither necessary
nor sufficient, I am going to abandon this approach.

If we apply fPIC to only a handful of compilation units containing
code that runs from the 1:1 mapping, it is not unreasonable to simply
disable the stack protector altogether for those pieces too. This
works around the older GCC issue.