Currently, x86-64 uses an unusual percpu layout, where the percpu section
is linked at absolute address 0.  The reason behind this is that older GCC
versions placed the stack protector (if enabled) at a fixed offset from the
GS segment base.  Since the GS segement is also used for percpu variables,
this forced the current layout.

GCC since version 8.1 supports a configurable location for the stack
protector value, which allows removal of the restriction on how the percpu
section is linked.  This allows the percpu section to be linked normally,
like other architectures.  In turn, this allows removal of code that was
needed to support the zero-based percpu section.

v4:
- Updated to current tip tree
- Added two new cleanups made possible by the removal of IA-64.
- Small improvements to the objtool conversion code.

Brian Gerst (16):
  x86/stackprotector/32: Remove stack protector test script
  x86/stackprotector/64: Remove stack protector test script
  x86/boot: Disable stack protector for early boot code
  x86/pvh: Use fixed_percpu_data for early boot GSBASE
  x86/relocs: Handle R_X86_64_REX_GOTPCRELX relocations
  objtool: Allow adding relocations to an existing section
  objtool: Convert fixed location stack protector accesses
  x86/stackprotector/64: Convert to normal percpu variable
  x86/percpu/64: Use relative percpu offsets
  x86/percpu/64: Remove fixed_percpu_data
  x86/boot/64: Remove inverse relocations
  x86/percpu/64: Remove INIT_PER_CPU macros
  percpu: Remove PER_CPU_FIRST_SECTION
  percpu: Remove PERCPU_VADDR()
  percpu: Remove __per_cpu_load
  kallsyms: Remove KALLSYMS_ABSOLUTE_PERCPU

 arch/x86/Kconfig                          |  16 +--
 arch/x86/Makefile                         |  21 ++--
 arch/x86/boot/compressed/misc.c           |  14 +--
 arch/x86/entry/entry_64.S                 |   2 +-
 arch/x86/include/asm/desc.h               |   1 -
 arch/x86/include/asm/percpu.h             |  22 ----
 arch/x86/include/asm/processor.h          |  28 +----
 arch/x86/include/asm/stackprotector.h     |  36 +-----
 arch/x86/kernel/Makefile                  |   2 +
 arch/x86/kernel/asm-offsets_64.c          |   6 -
 arch/x86/kernel/cpu/common.c              |   9 +-
 arch/x86/kernel/head64.c                  |   2 +-
 arch/x86/kernel/head_64.S                 |  20 ++-
 arch/x86/kernel/irq_64.c                  |   1 -
 arch/x86/kernel/setup_percpu.c            |  12 +-
 arch/x86/kernel/vmlinux.lds.S             |  35 ------
 arch/x86/platform/pvh/head.S              |  10 +-
 arch/x86/tools/relocs.c                   | 143 ++--------------------
 arch/x86/xen/xen-head.S                   |  10 +-
 include/asm-generic/sections.h            |   2 +-
 include/asm-generic/vmlinux.lds.h         |  43 +------
 include/linux/percpu-defs.h               |  12 --
 init/Kconfig                              |  11 +-
 kernel/kallsyms.c                         |  12 +-
 mm/percpu.c                               |   4 +-
 scripts/Makefile.lib                      |   2 +
 scripts/gcc-x86_32-has-stack-protector.sh |   8 --
 scripts/gcc-x86_64-has-stack-protector.sh |   4 -
 scripts/kallsyms.c                        |  80 +++---------
 scripts/link-vmlinux.sh                   |   4 -
 tools/objtool/arch/x86/decode.c           |  46 +++++++
 tools/objtool/arch/x86/special.c          |  91 ++++++++++++++
 tools/objtool/builtin-check.c             |   9 +-
 tools/objtool/check.c                     |  14 ++-
 tools/objtool/elf.c                       | 133 ++++++++++++++++----
 tools/objtool/include/objtool/arch.h      |   3 +
 tools/objtool/include/objtool/builtin.h   |   2 +
 tools/objtool/include/objtool/elf.h       |  90 +++++++++++---
 38 files changed, 442 insertions(+), 518 deletions(-)
 delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
 delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh


base-commit: 30052fd948a3b43506c83590eaaada12d1f2dd09
-- 
2.44.0

On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
>
> Currently, x86-64 uses an unusual percpu layout, where the percpu section
> is linked at absolute address 0.  The reason behind this is that older GCC
> versions placed the stack protector (if enabled) at a fixed offset from the
> GS segment base.  Since the GS segement is also used for percpu variables,
> this forced the current layout.
>
> GCC since version 8.1 supports a configurable location for the stack
> protector value, which allows removal of the restriction on how the percpu
> section is linked.  This allows the percpu section to be linked normally,
> like other architectures.  In turn, this allows removal of code that was
> needed to support the zero-based percpu section.

The number of simplifications throughout the code, enabled by this
patch set, is really impressive, and it reflects the number of
workarounds to enable the feature that was originally not designed for
the kernel usage. As noted above, this issue was recognized in the GCC
compiler and the stack protector support was generalized by adding
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 necessary.
Please note that years ago x86_32 simply dropped stack protector
support with earlier compilers and IMO, we should follow this example
also with x86_64, because:

a) There are currently 5 (soon 6) GCC major releases that support
configurable location for stack protector value. GCC 10 is already out
of active maintenance, and GCC 7 is considered an ancient release at
this time. For x86_32, it was advised to drop the support for stack
protector entirely with too old compilers to somehow force users to
upgrade the compiler.

b) Stack protector is not a core feature - the kernel will still boot
without stack protector. So, if someone really has the urge to use
ancient compilers with the bleeding edge kernel, it is still possible
to create a bootable image. I do not think using ancient compilers to
compile bleeding edge kernels makes any sense at all.

c) Maintenance burden - an objtool feature will have to be maintained
until gcc-8.1 is the minimum required compiler version. This feature
will IMO be seldom used and thus prone to bitrot.

d) Discrepancy between x86_32 and x86_64 - either both targets should
use objtool fixups for stack protector, or none at all. As shown by
x86_32 approach in the past, removing stack protector support with
ancient compilers was not problematic at all.

That said, the whole series is heartily Acked-by: Uros Bizjak
<ubizjak@gmail.com>

[1] https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=e1769bdd4cef522ada32aec863feba41116b183a
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81708

Thanks,
Uros.

>
> v4:
> - Updated to current tip tree
> - Added two new cleanups made possible by the removal of IA-64.
> - Small improvements to the objtool conversion code.
>
> Brian Gerst (16):
>   x86/stackprotector/32: Remove stack protector test script
>   x86/stackprotector/64: Remove stack protector test script
>   x86/boot: Disable stack protector for early boot code
>   x86/pvh: Use fixed_percpu_data for early boot GSBASE
>   x86/relocs: Handle R_X86_64_REX_GOTPCRELX relocations
>   objtool: Allow adding relocations to an existing section
>   objtool: Convert fixed location stack protector accesses
>   x86/stackprotector/64: Convert to normal percpu variable
>   x86/percpu/64: Use relative percpu offsets
>   x86/percpu/64: Remove fixed_percpu_data
>   x86/boot/64: Remove inverse relocations
>   x86/percpu/64: Remove INIT_PER_CPU macros
>   percpu: Remove PER_CPU_FIRST_SECTION
>   percpu: Remove PERCPU_VADDR()
>   percpu: Remove __per_cpu_load
>   kallsyms: Remove KALLSYMS_ABSOLUTE_PERCPU
>
>  arch/x86/Kconfig                          |  16 +--
>  arch/x86/Makefile                         |  21 ++--
>  arch/x86/boot/compressed/misc.c           |  14 +--
>  arch/x86/entry/entry_64.S                 |   2 +-
>  arch/x86/include/asm/desc.h               |   1 -
>  arch/x86/include/asm/percpu.h             |  22 ----
>  arch/x86/include/asm/processor.h          |  28 +----
>  arch/x86/include/asm/stackprotector.h     |  36 +-----
>  arch/x86/kernel/Makefile                  |   2 +
>  arch/x86/kernel/asm-offsets_64.c          |   6 -
>  arch/x86/kernel/cpu/common.c              |   9 +-
>  arch/x86/kernel/head64.c                  |   2 +-
>  arch/x86/kernel/head_64.S                 |  20 ++-
>  arch/x86/kernel/irq_64.c                  |   1 -
>  arch/x86/kernel/setup_percpu.c            |  12 +-
>  arch/x86/kernel/vmlinux.lds.S             |  35 ------
>  arch/x86/platform/pvh/head.S              |  10 +-
>  arch/x86/tools/relocs.c                   | 143 ++--------------------
>  arch/x86/xen/xen-head.S                   |  10 +-
>  include/asm-generic/sections.h            |   2 +-
>  include/asm-generic/vmlinux.lds.h         |  43 +------
>  include/linux/percpu-defs.h               |  12 --
>  init/Kconfig                              |  11 +-
>  kernel/kallsyms.c                         |  12 +-
>  mm/percpu.c                               |   4 +-
>  scripts/Makefile.lib                      |   2 +
>  scripts/gcc-x86_32-has-stack-protector.sh |   8 --
>  scripts/gcc-x86_64-has-stack-protector.sh |   4 -
>  scripts/kallsyms.c                        |  80 +++---------
>  scripts/link-vmlinux.sh                   |   4 -
>  tools/objtool/arch/x86/decode.c           |  46 +++++++
>  tools/objtool/arch/x86/special.c          |  91 ++++++++++++++
>  tools/objtool/builtin-check.c             |   9 +-
>  tools/objtool/check.c                     |  14 ++-
>  tools/objtool/elf.c                       | 133 ++++++++++++++++----
>  tools/objtool/include/objtool/arch.h      |   3 +
>  tools/objtool/include/objtool/builtin.h   |   2 +
>  tools/objtool/include/objtool/elf.h       |  90 +++++++++++---
>  38 files changed, 442 insertions(+), 518 deletions(-)
>  delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
>  delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh
>
>
> base-commit: 30052fd948a3b43506c83590eaaada12d1f2dd09
> --
> 2.44.0
>

* Uros Bizjak <ubizjak@gmail.com> wrote:

> On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> >
> > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > is linked at absolute address 0.  The reason behind this is that older GCC
> > versions placed the stack protector (if enabled) at a fixed offset from the
> > GS segment base.  Since the GS segement is also used for percpu variables,
> > this forced the current layout.
> >
> > GCC since version 8.1 supports a configurable location for the stack
> > protector value, which allows removal of the restriction on how the percpu
> > section is linked.  This allows the percpu section to be linked normally,
> > like other architectures.  In turn, this allows removal of code that was
> > needed to support the zero-based percpu section.
> 
> The number of simplifications throughout the code, enabled by this
> patch set, is really impressive, and it reflects the number of
> workarounds to enable the feature that was originally not designed for
> the kernel usage. As noted above, this issue was recognized in the GCC
> compiler and the stack protector support was generalized by adding
> 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 necessary.
> Please note that years ago x86_32 simply dropped stack protector
> support with earlier compilers and IMO, we should follow this example
> 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 distro 
side: when *this* new kernel of ours is picked by a distro, it almost 
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 to 
tweak compiler version requirements.

But please emit a (single) prominent build-time warning if a feature is 
disabled though, even if there are no functional side-effects, such as 
for hardening features.

In general distro kernel developers & maintainers like seeing the 
performance (and other) effects of their compiler version choices, but 
we are not very transparent about this: our fallbacks are way too 
opaque right now.

Thanks,

	Ingo

On Sat, Mar 23, 2024 at 10:25 PM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Uros Bizjak <ubizjak@gmail.com> wrote:
>
> > On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> > >
> > > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > > is linked at absolute address 0.  The reason behind this is that older GCC
> > > versions placed the stack protector (if enabled) at a fixed offset from the
> > > GS segment base.  Since the GS segement is also used for percpu variables,
> > > this forced the current layout.
> > >
> > > GCC since version 8.1 supports a configurable location for the stack
> > > protector value, which allows removal of the restriction on how the percpu
> > > section is linked.  This allows the percpu section to be linked normally,
> > > like other architectures.  In turn, this allows removal of code that was
> > > needed to support the zero-based percpu section.
> >
> > The number of simplifications throughout the code, enabled by this
> > patch set, is really impressive, and it reflects the number of
> > workarounds to enable the feature that was originally not designed for
> > the kernel usage. As noted above, this issue was recognized in the GCC
> > compiler and the stack protector support was generalized by adding
> > 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 necessary.
> > Please note that years ago x86_32 simply dropped stack protector
> > support with earlier compilers and IMO, we should follow this example
> > 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 distro
> side: when *this* new kernel of ours is picked by a distro, it almost
> 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 to
> tweak compiler version requirements.
>
> But please emit a (single) prominent build-time warning if a feature is
> disabled though, even if there are no functional side-effects, such as
> for hardening features.

Disabled for any reason or only if the compiler lacks support?

Brian Gerst

* Brian Gerst <brgerst@gmail.com> wrote:

> On Sat, Mar 23, 2024 at 10:25 PM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > > On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> > > >
> > > > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > > > is linked at absolute address 0.  The reason behind this is that older GCC
> > > > versions placed the stack protector (if enabled) at a fixed offset from the
> > > > GS segment base.  Since the GS segement is also used for percpu variables,
> > > > this forced the current layout.
> > > >
> > > > GCC since version 8.1 supports a configurable location for the stack
> > > > protector value, which allows removal of the restriction on how the percpu
> > > > section is linked.  This allows the percpu section to be linked normally,
> > > > like other architectures.  In turn, this allows removal of code that was
> > > > needed to support the zero-based percpu section.
> > >
> > > The number of simplifications throughout the code, enabled by this
> > > patch set, is really impressive, and it reflects the number of
> > > workarounds to enable the feature that was originally not designed for
> > > the kernel usage. As noted above, this issue was recognized in the GCC
> > > compiler and the stack protector support was generalized by adding
> > > 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 necessary.
> > > Please note that years ago x86_32 simply dropped stack protector
> > > support with earlier compilers and IMO, we should follow this example
> > > 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 distro
> > side: when *this* new kernel of ours is picked by a distro, it almost
> > 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 to
> > tweak compiler version requirements.
> >
> > But please emit a (single) prominent build-time warning if a feature is
> > disabled though, even if there are no functional side-effects, such 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 due 
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

On Sun, Mar 24, 2024 at 12:05 AM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Brian Gerst <brgerst@gmail.com> wrote:
>
> > On Sat, Mar 23, 2024 at 10:25 PM Ingo Molnar <mingo@kernel.org> wrote:
> > >
> > >
> > > * Uros Bizjak <ubizjak@gmail.com> wrote:
> > >
> > > > On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> > > > >
> > > > > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > > > > is linked at absolute address 0.  The reason behind this is that older GCC
> > > > > versions placed the stack protector (if enabled) at a fixed offset from the
> > > > > GS segment base.  Since the GS segement is also used for percpu variables,
> > > > > this forced the current layout.
> > > > >
> > > > > GCC since version 8.1 supports a configurable location for the stack
> > > > > protector value, which allows removal of the restriction on how the percpu
> > > > > section is linked.  This allows the percpu section to be linked normally,
> > > > > like other architectures.  In turn, this allows removal of code that was
> > > > > needed to support the zero-based percpu section.
> > > >
> > > > The number of simplifications throughout the code, enabled by this
> > > > patch set, is really impressive, and it reflects the number of
> > > > workarounds to enable the feature that was originally not designed for
> > > > the kernel usage. As noted above, this issue was recognized in the GCC
> > > > compiler and the stack protector support was generalized by adding
> > > > 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 necessary.
> > > > Please note that years ago x86_32 simply dropped stack protector
> > > > support with earlier compilers and IMO, we should follow this example
> > > > 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 distro
> > > side: when *this* new kernel of ours is picked by a distro, it almost
> > > 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 to
> > > tweak compiler version requirements.
> > >
> > > But please emit a (single) prominent build-time warning if a feature is
> > > disabled though, even if there are no functional side-effects, such 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 due
> 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 compiler"
       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.

Brian Gerst

* Brian Gerst <brgerst@gmail.com> wrote:

> On Sun, Mar 24, 2024 at 12:05 AM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Brian Gerst <brgerst@gmail.com> wrote:
> >
> > > On Sat, Mar 23, 2024 at 10:25 PM Ingo Molnar <mingo@kernel.org> wrote:
> > > >
> > > >
> > > > * Uros Bizjak <ubizjak@gmail.com> wrote:
> > > >
> > > > > On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> > > > > >
> > > > > > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > > > > > is linked at absolute address 0.  The reason behind this is that older GCC
> > > > > > versions placed the stack protector (if enabled) at a fixed offset from the
> > > > > > GS segment base.  Since the GS segement is also used for percpu variables,
> > > > > > this forced the current layout.
> > > > > >
> > > > > > GCC since version 8.1 supports a configurable location for the stack
> > > > > > protector value, which allows removal of the restriction on how the percpu
> > > > > > section is linked.  This allows the percpu section to be linked normally,
> > > > > > like other architectures.  In turn, this allows removal of code that was
> > > > > > needed to support the zero-based percpu section.
> > > > >
> > > > > The number of simplifications throughout the code, enabled by this
> > > > > patch set, is really impressive, and it reflects the number of
> > > > > workarounds to enable the feature that was originally not designed for
> > > > > the kernel usage. As noted above, this issue was recognized in the GCC
> > > > > compiler and the stack protector support was generalized by adding
> > > > > 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 necessary.
> > > > > Please note that years ago x86_32 simply dropped stack protector
> > > > > support with earlier compilers and IMO, we should follow this example
> > > > > 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 distro
> > > > side: when *this* new kernel of ours is picked by a distro, it almost
> > > > 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 to
> > > > tweak compiler version requirements.
> > > >
> > > > But please emit a (single) prominent build-time warning if a feature is
> > > > disabled though, even if there are no functional side-effects, such 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 due
> > 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 compiler"
>        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 ]  <========
...                                    zlib: [ on  ]
...                                    lzma: [ on  ]
...                               get_cpuid: [ on  ]
...                                     bpf: [ on  ]
...                                  libaio: [ on  ]
...                                 libzstd: [ on  ]

... or something like that.

Thanks,

	Ingo

On Sun, Mar 24, 2024 at 6:53 AM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Brian Gerst <brgerst@gmail.com> wrote:
>
> > On Sun, Mar 24, 2024 at 12:05 AM Ingo Molnar <mingo@kernel.org> wrote:
> > >
> > >
> > > * Brian Gerst <brgerst@gmail.com> wrote:
> > >
> > > > On Sat, Mar 23, 2024 at 10:25 PM Ingo Molnar <mingo@kernel.org> wrote:
> > > > >
> > > > >
> > > > > * Uros Bizjak <ubizjak@gmail.com> wrote:
> > > > >
> > > > > > On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> > > > > > >
> > > > > > > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > > > > > > is linked at absolute address 0.  The reason behind this is that older GCC
> > > > > > > versions placed the stack protector (if enabled) at a fixed offset from the
> > > > > > > GS segment base.  Since the GS segement is also used for percpu variables,
> > > > > > > this forced the current layout.
> > > > > > >
> > > > > > > GCC since version 8.1 supports a configurable location for the stack
> > > > > > > protector value, which allows removal of the restriction on how the percpu
> > > > > > > section is linked.  This allows the percpu section to be linked normally,
> > > > > > > like other architectures.  In turn, this allows removal of code that was
> > > > > > > needed to support the zero-based percpu section.
> > > > > >
> > > > > > The number of simplifications throughout the code, enabled by this
> > > > > > patch set, is really impressive, and it reflects the number of
> > > > > > workarounds to enable the feature that was originally not designed for
> > > > > > the kernel usage. As noted above, this issue was recognized in the GCC
> > > > > > compiler and the stack protector support was generalized by adding
> > > > > > 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 necessary.
> > > > > > Please note that years ago x86_32 simply dropped stack protector
> > > > > > support with earlier compilers and IMO, we should follow this example
> > > > > > 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 distro
> > > > > side: when *this* new kernel of ours is picked by a distro, it almost
> > > > > 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 to
> > > > > tweak compiler version requirements.
> > > > >
> > > > > But please emit a (single) prominent build-time warning if a feature is
> > > > > disabled though, even if there are no functional side-effects, such 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 due
> > > 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 compiler"
> >        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 ]  <========
> ...                                    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

* Brian Gerst <brgerst@gmail.com> wrote:

> > > 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 ]  <========
> > ...                                    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.

Yeah, I know, I wrote the initial version. ;-)

( See upstream commits b6aa9979416e~1..4cc9117a35b2 )

> 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.

Doesn't have to be complicated really, but obviously not a requirement for 
this series.

Thanks,

	Ingo

On Sat, Mar 23, 2024 at 7:39 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Fri, Mar 22, 2024 at 5:52 PM Brian Gerst <brgerst@gmail.com> wrote:
> >
> > Currently, x86-64 uses an unusual percpu layout, where the percpu section
> > is linked at absolute address 0.  The reason behind this is that older GCC
> > versions placed the stack protector (if enabled) at a fixed offset from the
> > GS segment base.  Since the GS segement is also used for percpu variables,
> > this forced the current layout.
> >
> > GCC since version 8.1 supports a configurable location for the stack
> > protector value, which allows removal of the restriction on how the percpu
> > section is linked.  This allows the percpu section to be linked normally,
> > like other architectures.  In turn, this allows removal of code that was
> > needed to support the zero-based percpu section.
>
> The number of simplifications throughout the code, enabled by this
> patch set, is really impressive, and it reflects the number of
> workarounds to enable the feature that was originally not designed for
> the kernel usage. As noted above, this issue was recognized in the GCC
> compiler and the stack protector support was generalized by adding
> 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 necessary.
> Please note that years ago x86_32 simply dropped stack protector
> support with earlier compilers and IMO, we should follow this example
> also with x86_64, because:
>
> a) There are currently 5 (soon 6) GCC major releases that support
> configurable location for stack protector value. GCC 10 is already out
> of active maintenance, and GCC 7 is considered an ancient release at
> this time. For x86_32, it was advised to drop the support for stack
> protector entirely with too old compilers to somehow force users to
> upgrade the compiler.

At least one developer claimed to be using an older compiler.  I asked
for more details on why, but got no response.

> b) Stack protector is not a core feature - the kernel will still boot
> without stack protector. So, if someone really has the urge to use
> ancient compilers with the bleeding edge kernel, it is still possible
> to create a bootable image. I do not think using ancient compilers to
> compile bleeding edge kernels makes any sense at all.

One small issue is that Kconfig would silently disable istackprotector
if the compiler doesn't support the new options.  That said, the
number of people that this would affect is very small, as just about
any modern distribution ships a compiler newer than 8.1.

I'm all in favor of only supporting compilers that are supported upstream.

> c) Maintenance burden - an objtool feature will have to be maintained
> until gcc-8.1 is the minimum required compiler version. This feature
> will IMO be seldom used and thus prone to bitrot.

That's the reason I added a config option to allow testing objtool
even if the compiler has support.

> d) Discrepancy between x86_32 and x86_64 - either both targets should
> use objtool fixups for stack protector, or none at all. As shown by
> x86_32 approach in the past, removing stack protector support with
> ancient compilers was not problematic at all.

Objtool doesn't support x86-32, mostly because working with REL type
relocations is a pain.

> That said, the whole series is heartily Acked-by: Uros Bizjak
> <ubizjak@gmail.com>
>
> [1] https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=e1769bdd4cef522ada32aec863feba41116b183a
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81708
>
> Thanks,
> Uros.

Brian Gerst

On Sat, 23 Mar 2024 at 06:23, Brian Gerst <brgerst@gmail.com> wrote:
>
> One small issue is that Kconfig would silently disable istackprotector
> if the compiler doesn't support the new options.  That said, the
> number of people that this would affect is very small, as just about
> any modern distribution ships a compiler newer than 8.1.

Yes, let's make the rule be that you can still compile the kernel with
gcc-5.1+, but you can't get stackprotector support unless you have
gcc-8.1+.

I'd hate to add the objtool support for an old compiler - this is a
hardening feature, not a core feature, and anybody who insists on old
compilers just won't get it.

And we have other cases like this where various debug features depend
on the gcc version, eg

  config CC_HAS_WORKING_NOSANITIZE_ADDRESS
          def_bool !CC_IS_GCC || GCC_VERSION >= 80300

so we could easily do the same for stack protector support.

And we might as well also do the semi-yearly compiler version review.
We raised the minimum to 4.9 almost four years ago, and then the jump
to 5.1 was first for arm64 due to a serious gcc code generation bug
and then globally in Sept 2021.

So it's probably time to think about that anyway,

That said, we don't actually have all that many gcc version checks
around any more, so I think the jump to 5.1 got rid of the worst of
the horrors. Most of the GCC_VERSION checks are either in gcc-plugins
(which we should just remove, imnsho - not the version checks, the
plugins entirely), or for various random minor details (warnign
enablement and the asm goto workaround).

So there doesn't seem to be a major reason to up the versioning, since
the stack protector thing can just be disabled for older versions.

But maybe even enterprise distros have upgraded anyway, and we should
be proactive.

Cc'ing Arnd, who has historically been one of the people pushing this.
He may no longer care because we haven't had huge issues.

               Linus

On Sat, 23 Mar 2024 at 09:16, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And we might as well also do the semi-yearly compiler version review.
> We raised the minimum to 4.9 almost four years ago, and then the jump
> to 5.1 was first for arm64 due to a serious gcc code generation bug
> and then globally in Sept 2021.

Looking at RHEL, I find a page that claims

  RHEL9 : gcc 11.x in app stream
  RHEL8 : gcc 8.x or gcc 9.x in app stream.
  RHEL7 : gcc 4.8.x

so RHEL7 is already immaterial from a kernel compiler standpoint, and
so it looks like at least as far as RHEL is concerned, we could just
jump to gcc 8.1 as a minimum.

RHEL also has a "Developer Toolset" that allows you to pick a compiler
upgrade, so it's not *quite* as black-and-white as that, but it does
seem like we could at some point just pick gcc-8 as a new minimum with
very little pain on that front.

The SLES situation seems somewhat similar, with SLES12 being 4.8.x and
SLES15 being 7.3. But again with a "Development Tools Module" setup.
So that *might* argue for 7.3.

I can't make sense of Debian releases. There's "stable" (bookworm)
that comes with gcc-12.2, but there's oldstable, oldoldstable, and
various "archived" releases still under LTS. I can't even begin to
guess what may be relevant.

I don't think we care that deeply on the kernel side, other than a
"maybe we should be a bit more proactive about raising gcc version
requirements". I don't think we have any huge issues right now with
old gcc versions.

               Linus

From: Linus Torvalds
> Sent: 23 March 2024 17:07
> 
> On Sat, 23 Mar 2024 at 09:16, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > And we might as well also do the semi-yearly compiler version review.
> > We raised the minimum to 4.9 almost four years ago, and then the jump
> > to 5.1 was first for arm64 due to a serious gcc code generation bug
> > and then globally in Sept 2021.
> 
> Looking at RHEL, I find a page that claims
> 
>   RHEL9 : gcc 11.x in app stream
>   RHEL8 : gcc 8.x or gcc 9.x in app stream.
>   RHEL7 : gcc 4.8.x

I'm running Ubuntu 18.04 LTS (and still getting updates) and that
has gcc 7.3.0.
Clearly it might be time to update/reinstall that system :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)