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

[Carlos O'Donell]

On 1/11/24 10:58, H.J. Lu wrote:

Looking forward to a v2 so we can resolve this for 2.39.

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

Agreed.

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

Does this also work for SELinux deny_execmem?

What does `getsebool -a | grep deny_execmem` say?

What does your patched glibc do with `semanage boolean --modify deny_execmem --on`?

WARNING: Do not attempt deny_execmen on anything but a VM you can spare having crash
to just a terminal.

> When it is set, systemd service stops working if PLT rewrite is enabled.
> Check if mprotect works before rewriting PLT.  This fixes BZ #31230.
> ---
>  .../unix/sysv/linux/x86_64/dl-plt-rewrite.h   | 43 +++++++++++++++++++
>  sysdeps/x86/cpu-features.c                    |  4 +-
>  sysdeps/x86_64/dl-plt-rewrite.h               | 25 +++++++++++
>  3 files changed, 71 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..6401b7b2f2
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86_64/dl-plt-rewrite.h
> @@ -0,0 +1,43 @@
> +/* PLT rewrite help function.  Linux/x86-64 version.

s/help/helper/g

> +   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);

Is it specifically required that you go from RW to RWE?

Could you start with just an RWE page and go to RE?

> +
> +  INTERNAL_SYSCALL_CALL (munmap, plt, 4096);
> +
> +  return status;
> +}
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 46bdaffbc2..6aaa750e20 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -28,10 +28,12 @@ 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)
> +  if (valp->numval != 0 && dl_plt_rewrite_supported ())

Suggest comment here:

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

The && short-circuit means we won't test the page support unless the tunable
is non-zero. This is important because we don't want logging to SELinux or
systemd for the rewrite test if the feature isn't being used.

>      {
>        /* 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..7eaae8f457
> --- /dev/null
> +++ b/sysdeps/x86_64/dl-plt-rewrite.h
> @@ -0,0 +1,25 @@
> +/* PLT rewrite help function.  x86-64 version.

s/help/helper/g

> +   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;
> +}

-- 
Cheers,
Carlos.