Received-SPF: pass (google.com: domain of linux-kernel+bounces-112668-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: <20240322165233.71698-1-brgerst@gmail.com> <CAFULd4bCufzKjaUyOcJ5MfsPBcVTj1zQiP3+FFCGo6SbxTpK2A@mail.gmail.com>
 <Zf+PIYP4TyF6ZRVy@gmail.com> <CAMzpN2htOit94c-M+zHqEcLcGPOU2zTS6wM-r7xWwd9Ku8h3-Q@mail.gmail.com>
 <Zf+mjy49dG5ly9ka@gmail.com> <CAMzpN2go9mmyWRb9vsg7O1aAtSKrW=HqcZYmddkq7eZQQHuM1Q@mail.gmail.com>
 <ZgAGEcmrWZyDrO50@gmail.com>
In-Reply-To: <ZgAGEcmrWZyDrO50@gmail.com>
From: Brian Gerst <brgerst@gmail.com>
Date: Sun, 24 Mar 2024 08:34:01 -0400
Message-ID: <CAMzpN2j1B99FSXVQ=S5a3G+XQf2Cq5rtx=fR77VHW8RDn7WKAQ@mail.gmail.com>
Subject: Re: [PATCH v4 00/16] x86-64: Stack protector and percpu improvements
To: Ingo Molnar <mingo@kernel.org>
Cc: Uros Bizjak <ubizjak@gmail.com>, linux-kernel@vger.kernel.org, x86@kernel.org, 
	Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, "H . Peter Anvin" <hpa@zytor.com>, 
	David.Laight@aculab.com, Linus Torvalds <torvalds@linux-foundation.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, Mar 24, 2024 at 6:53=E2=80=AFAM Ingo Molnar <mingo@kernel.org> wrot=
e:
>
>
> * Brian Gerst <brgerst@gmail.com> wrote:
>
> > On Sun, Mar 24, 2024 at 12:05=E2=80=AFAM Ingo Molnar <mingo@kernel.org>=
 wrote:
> > >
> > >
> > > * Brian Gerst <brgerst@gmail.com> wrote:
> > >
> > > > On Sat, Mar 23, 2024 at 10:25=E2=80=AFPM Ingo Molnar <mingo@kernel.=
org> wrote:
> > > > >
> > > > >
> > > > > * Uros Bizjak <ubizjak@gmail.com> wrote:
> > > > >
> > > > > > On Fri, Mar 22, 2024 at 5:52=E2=80=AFPM Brian Gerst <brgerst@gm=
ail.com> wrote:
> > > > > > >
> > > > > > > Currently, x86-64 uses an unusual percpu layout, where the pe=
rcpu section
> > > > > > > is linked at absolute address 0.  The reason behind this is t=
hat older GCC
> > > > > > > versions placed the stack protector (if enabled) at a fixed o=
ffset from the
> > > > > > > GS segment base.  Since the GS segement is also used for perc=
pu variables,
> > > > > > > this forced the current layout.
> > > > > > >
> > > > > > > GCC since version 8.1 supports a configurable location for th=
e stack
> > > > > > > protector value, which allows removal of the restriction on h=
ow the percpu
> > > > > > > section is linked.  This allows the percpu section to be link=
ed normally,
> > > > > > > like other architectures.  In turn, this allows removal of co=
de that was
> > > > > > > needed to support the zero-based percpu section.
> > > > > >
> > > > > > The number of simplifications throughout the code, enabled by t=
his
> > > > > > patch set, is really impressive, and it reflects the number of
> > > > > > workarounds to enable the feature that was originally not desig=
ned for
> > > > > > the kernel usage. As noted above, this issue was recognized in =
the GCC
> > > > > > compiler and the stack protector support was generalized by add=
ing
> > > > > > configurable location for the stack protector value [1,2].
> > > > > >
> > > > > > The improved stack protector support was implemented in gcc-8.1=
,
> > > > > > released on May 2, 2018, when linux 4.17 was in development. In=
 light
> > > > > > of this fact, and 5 (soon 6) GCC major releases later, I'd like=
 to ask
> > > > > > if the objtool support to fixup earlier compilers is really nec=
essary.
> > > > > > Please note that years ago x86_32 simply dropped stack protecto=
r
> > > > > > support with earlier compilers and IMO, we should follow this e=
xample
> > > > > > also with x86_64, because:
> > > > >
> > > > > Ack on raising the minimum version requirement for x86-64
> > > > > stackprotector to 8.1 or so - this causes no real pain on the dis=
tro
> > > > > side: when *this* new kernel of ours is picked by a distro, it al=
most
> > > > > always goes hand in hand with a compiler version upgrade.
> > > > >
> > > > > We should be careful with fixes marked for -stable backport, but =
other
> > > > > than that, new improvements like Brian's series are a fair game t=
o
> > > > > tweak compiler version requirements.
> > > > >
> > > > > But please emit a (single) prominent build-time warning if a feat=
ure is
> > > > > disabled though, even if there are no functional side-effects, su=
ch as
> > > > > for hardening features.
> > > >
> > > > Disabled for any reason or only if the compiler lacks support?
> > >
> > > Only if the user desires to have it enabled, but it's not possible du=
e
> > > to compiler (or other build environment) reasons. Ie. if something
> > > unexpected happens from the user's perspective.
> > >
> > > The .config option is preserved even if the compiler doesn't support
> > > it, right?
> > >
> > > I suspect this should also cover features that get select-ed by a
> > > feature that the user enables. (Not sure about architecture level
> > > select-ed options.)
> > >
> > > Thanks,
> > >
> > >         Ingo
> >
> > I could add something like:
> >
> > comment "Stack protector is not supported by the architecture or compil=
er"
> >        depends on !HAVE_STACKPROTECTOR
> >
> > But, "make oldconfig" will still silently disable stack protector if
> > the compiler doesn't support the new options.  It does put the
> > comment into the .config file though, so that may be enough.
>
> So I was thinking more along the lines of emitting an actual warning to
> the build log, every time the compiler check is executed and fails to
> detect [certain] essential or good-to-have compiler features.
>
> A bit like the red '[ OFF ]' build lines during the perf build:
>
> Auto-detecting system features:
>
> ...                                   dwarf: [ on  ]
> ...                      dwarf_getlocations: [ on  ]
> ...                                   glibc: [ on  ]
> ...                                  libbfd: [ on  ]
> ...                          libbfd-buildid: [ on  ]
> ...                                  libcap: [ on  ]
> ...                                  libelf: [ on  ]
> ...                                 libnuma: [ on  ]
> ...                  numa_num_possible_cpus: [ on  ]
> ...                                 libperl: [ on  ]
> ...                               libpython: [ on  ]
> ...                               libcrypto: [ on  ]
> ...                               libunwind: [ on  ]
> ...                      libdw-dwarf-unwind: [ on  ]
> ...                             libcapstone: [ OFF ]  <=3D=3D=3D=3D=3D=3D=
=3D=3D
> ...                                    zlib: [ on  ]
> ...                                    lzma: [ on  ]
> ...                               get_cpuid: [ on  ]
> ...                                     bpf: [ on  ]
> ...                                  libaio: [ on  ]
> ...                                 libzstd: [ on  ]
>
> ... or something like that.
>
> Thanks,
>
>         Ingo

That list comes from the perf tool itself
(tools/perf/builtin-version.c), not the kernel config or build system.
Something like that could be added to the main kernel build.  But it
should be a separate patch series as it will likely need a lot of
design iteration.

Brian Gerst