[v2] x86-64: Check if mprotect works before rewriting PLT

  Systemd execution environment configuration may prohibit changing a memory
mapping to become executable:

MemoryDenyWriteExecute=
Takes a boolean argument. If set, attempts to create memory mappings
that are writable and executable at the same time, or to change existing
memory mappings to become executable, or mapping shared memory segments
as executable, are prohibited.

When it is set, systemd service stops working if PLT rewrite is enabled.
Check if mprotect works before rewriting PLT.  This fixes BZ #31230.
This also works with SELinux when deny_execmem is on.
---
 .../unix/sysv/linux/x86_64/dl-plt-rewrite.h   | 43 +++++++++++++++++++
 sysdeps/x86/cpu-features.c                    |  8 +++-
 sysdeps/x86_64/dl-plt-rewrite.h               | 25 +++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/dl-plt-rewrite.h
 create mode 100644 sysdeps/x86_64/dl-plt-rewrite.h
  

On 1/12/24 13:19, H.J. Lu wrote:
> Systemd execution environment configuration may prohibit changing a memory
> mapping to become executable:
> 
> MemoryDenyWriteExecute=
> Takes a boolean argument. If set, attempts to create memory mappings
> that are writable and executable at the same time, or to change existing
> memory mappings to become executable, or mapping shared memory segments
> as executable, are prohibited.
> 
> When it is set, systemd service stops working if PLT rewrite is enabled.
> Check if mprotect works before rewriting PLT.  This fixes BZ #31230.
> This also works with SELinux when deny_execmem is on.

LGTM. Good commit message and comments. Thank you!

Please commit this for 2.39.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  .../unix/sysv/linux/x86_64/dl-plt-rewrite.h   | 43 +++++++++++++++++++
>  sysdeps/x86/cpu-features.c                    |  8 +++-
>  sysdeps/x86_64/dl-plt-rewrite.h               | 25 +++++++++++
>  3 files changed, 75 insertions(+), 1 deletion(-)
>  create mode 100644 sysdeps/unix/sysv/linux/x86_64/dl-plt-rewrite.h
>  create mode 100644 sysdeps/x86_64/dl-plt-rewrite.h
> 
> diff --git a/sysdeps/unix/sysv/linux/x86_64/dl-plt-rewrite.h b/sysdeps/unix/sysv/linux/x86_64/dl-plt-rewrite.h
> new file mode 100644
> index 0000000000..ad637df930
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86_64/dl-plt-rewrite.h
> @@ -0,0 +1,43 @@
> +/* PLT rewrite helper function.  Linux/x86-64 version.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdbool.h>
> +#include <sys/mman.h>
> +
> +static __always_inline bool
> +dl_plt_rewrite_supported (void)
> +{
> +  /* PLT rewrite is enabled.  Check if mprotect works.  */
> +  void *plt = (void *) INTERNAL_SYSCALL_CALL (mmap, NULL, 4096,
> +					      PROT_READ | PROT_WRITE,
> +					      MAP_PRIVATE | MAP_ANONYMOUS,
> +					      -1, 0);
> +  if (__glibc_unlikely (plt == MAP_FAILED))
> +    return false;
> +
> +  /* Touch the PROT_READ | PROT_WRITE page.  */
> +  *(int32_t *) plt = 1;
> +
> +  /* If the updated PROT_READ | PROT_WRITE page can be changed to
> +     PROT_EXEC | PROT_READ, rewrite PLT.  */
> +  bool status = (INTERNAL_SYSCALL_CALL (mprotect, plt, 4096,
> +					PROT_EXEC | PROT_READ) == 0);
> +
> +  INTERNAL_SYSCALL_CALL (munmap, plt, 4096);
> +
> +  return status;
> +}
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 46bdaffbc2..25e6622a79 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -28,10 +28,16 @@ extern void TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *)
>    attribute_hidden;
>  
>  #if defined SHARED && defined __x86_64__
> +# include <dl-plt-rewrite.h>
> +
>  static void
>  TUNABLE_CALLBACK (set_plt_rewrite) (tunable_val_t *valp)
>  {
> -  if (valp->numval != 0)
> +  /* We must be careful about where we put the call to
> +     dl_plt_rewrite_supported() since it may generate
> +     spurious SELinux log entries.  It should only be
> +     attempted if the user requested a PLT rewrite.  */

OK.

> +  if (valp->numval != 0 && dl_plt_rewrite_supported ())
>      {
>        /* Use JMPABS only on APX processors.  */
>        const struct cpu_features *cpu_features = __get_cpu_features ();
> diff --git a/sysdeps/x86_64/dl-plt-rewrite.h b/sysdeps/x86_64/dl-plt-rewrite.h
> new file mode 100644
> index 0000000000..cab6fe75ea
> --- /dev/null
> +++ b/sysdeps/x86_64/dl-plt-rewrite.h
> @@ -0,0 +1,25 @@
> +/* PLT rewrite helper function.  x86-64 version.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdbool.h>
> +#include <sys/mman.h>
> +
> +static __always_inline bool
> +dl_plt_rewrite_supported (void)
> +{
> +  return true;
> +}
  

On 12/01/24 15:19, H.J. Lu wrote:
> Systemd execution environment configuration may prohibit changing a memory
> mapping to become executable:
> 
> MemoryDenyWriteExecute=
> Takes a boolean argument. If set, attempts to create memory mappings
> that are writable and executable at the same time, or to change existing
> memory mappings to become executable, or mapping shared memory segments
> as executable, are prohibited.
> 
> When it is set, systemd service stops working if PLT rewrite is enabled.
> Check if mprotect works before rewriting PLT.  This fixes BZ #31230.
> This also works with SELinux when deny_execmem is on.

On musl channel Rich has raised some points for this optimization that
made me curious.  His main points are this should not be faster than 
-fno-plt, so the main advantage is for old binaries or environments
where PLT is required (either for audit or any other instrumentation).

Since this new tunable requires more resources (either for the probing, 
plus the setup itself, and the extra VMA for the new PLT rewrite), with
recent  Linux security modules that would most likely to prevent it in 
a lot of deployments; the question is how really useful this would be 
and whether this is really more like an experiment to show a new x86 
feature.
  

On Tue, Jan 16, 2024 at 9:36 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 12/01/24 15:19, H.J. Lu wrote:
> > Systemd execution environment configuration may prohibit changing a memory
> > mapping to become executable:
> >
> > MemoryDenyWriteExecute=
> > Takes a boolean argument. If set, attempts to create memory mappings
> > that are writable and executable at the same time, or to change existing
> > memory mappings to become executable, or mapping shared memory segments
> > as executable, are prohibited.
> >
> > When it is set, systemd service stops working if PLT rewrite is enabled.
> > Check if mprotect works before rewriting PLT.  This fixes BZ #31230.
> > This also works with SELinux when deny_execmem is on.
>
> On musl channel Rich has raised some points for this optimization that
> made me curious.  His main points are this should not be faster than
> -fno-plt, so the main advantage is for old binaries or environments

That is true for most cases.   But in some cases, direct call + direct
jump is faster than indirect call.

> where PLT is required (either for audit or any other instrumentation).

This is also true.

> Since this new tunable requires more resources (either for the probing,
> plus the setup itself, and the extra VMA for the new PLT rewrite), with
> recent  Linux security modules that would most likely to prevent it in
> a lot of deployments; the question is how really useful this would be

This also applies to all JITs.

> and whether this is really more like an experiment to show a new x86
> feature.

This feature has a minimal performance impact for most people.  But
it is very useful for cases where PLT performance is critical.