Re: [PATCH] Don't run personality syscall at configure time; don't check it at all

[Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>]

On Thu, May 06, 2021 at 11:43:41PM +0100, Pedro Alves wrote:
> Currently, in order to tell whether support for disabling address
> space randomization on Linux is available, GDB checks if the
> personality syscall works, at configure time.  I.e., it does a run
> test, instead of a compile/link test:
> 
>   AC_RUN_IFELSE([PERSONALITY_TEST],
> 		[have_personality=true],
> 		[have_personality=false],
> 
> In that case we need to fix configure. The fact that personality works on the build machine says nothing about if configure works on the test machine.
> 
> 
> This is a bit bogus, because the machine the build is done on may not
> (and is when you consider distro gdbs) be the machine that eventually
> runs gdb.  It would be better if this were a compile/link test
> instead, and then at runtime, GDB coped with the personality syscall
> failing.  Actually, GDB already copes.
> 
> One environment where this is problematic is building GDB in a Docker
> container -- by default, Docker runs the container with seccomp, with
> a profile that disables the personality syscall.  You can tell Docker
> to use a less restricted seccomp profile, but I think we should just
> fix it in GDB.
> 
> "man 2 personality" says:
> 
>        This system call first appeared in Linux 1.1.20 (and thus first
>        in a stable kernel release with Linux 1.2.0); library support
>        was added in glibc 2.3.
> 
> ...
> 
>        ADDR_NO_RANDOMIZE (since Linux 2.6.12)
>               With this flag set, disable address-space-layout randomization.
> 
> glibc 2.3 was released in 2002.
> Linux 2.6.12 was released in 2005.
> 
> The original patch that added the configure checks was submitted in
> 2008.  The first version of the patch that was submitted to the list
> called personality from common code:
> 
>  https://sourceware.org/pipermail/gdb-patches/2008-June/058204.html
> 
> and then was moved to Linux-specific code:
> 
>  https://sourceware.org/pipermail/gdb-patches/2008-June/058209.html
> 
> Since HAVE_PERSONALITY is only checked in Linux code, and
> ADDR_NO_RANDOMIZE exists for over 15 years, I propose just completely
> removing the configure checks.
> 
> If for some odd reason, some remotely modern system still needs a
> configure check, then we can revert this commit but drop the
> AC_RUN_IFELSE in favor of always doing the AC_LINK_IFELSE
> cross-compile fallback.
> 
> gdb/ChangeLog:
> 
> 	* linux-nat.c (linux_nat_target::supports_disable_randomization):
> 	rEmove references to HAVE_PERSONALITY.
> 	* nat/linux-personality.c: Remove references to HAVE_PERSONALITY.
> 	(maybe_disable_address_space_randomization)
> 	(~maybe_disable_address_space_randomizatio): Remove references to
> 	HAVE_PERSONALITY.
> 	* config.in, configure: Regenerate.
> 
> gdbserver/ChangeLog:
> 
> 	* linux-low.cc: Remove references to HAVE_PERSONALITY.
> 	(linux_process_target::supports_disable_randomization): Remove
> 	reference to HAVE_PERSONALITY.
> 	* config.in, configure: Regenerate.
> 
> gdbsupport/ChangeLog:
> 
> 	* common.m4 (personality test): Remove.
> ---
>  gdb/config.in               |  7 ----
>  gdb/configure               | 74 -------------------------------------
>  gdb/linux-nat.c             |  4 --
>  gdb/nat/linux-personality.c | 12 +-----
>  gdbserver/config.in         |  7 ----
>  gdbserver/configure         | 74 -------------------------------------
>  gdbserver/linux-low.cc      | 11 +-----
>  gdbsupport/common.m4        | 23 ------------
>  8 files changed, 2 insertions(+), 210 deletions(-)
> 
> diff --git a/gdb/config.in b/gdb/config.in
> index 5d10377b736..99c924f9ba0 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -96,10 +96,6 @@
>  /* define if the compiler supports basic C++11 syntax */
>  #undef HAVE_CXX11
>  
> -/* Define to 1 if you have the declaration of `ADDR_NO_RANDOMIZE', and to 0 if
> -   you don't. */
> -#undef HAVE_DECL_ADDR_NO_RANDOMIZE
> -
>  /* Define to 1 if you have the declaration of `asprintf', and to 0 if you
>     don't. */
>  #undef HAVE_DECL_ASPRINTF
> @@ -301,9 +297,6 @@
>  /* Define to 1 if you have the <nlist.h> header file. */
>  #undef HAVE_NLIST_H
>  
> -/* Define if you support the personality syscall. */
> -#undef HAVE_PERSONALITY
> -
>  /* Define to 1 if you have the `pipe' function. */
>  #undef HAVE_PIPE
>  
> diff --git a/gdb/configure b/gdb/configure
> index b47de77bf64..cdc112e10dc 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -13819,80 +13819,6 @@ fi
>  done
>  
>  
> -      ac_fn_c_check_decl "$LINENO" "ADDR_NO_RANDOMIZE" "ac_cv_have_decl_ADDR_NO_RANDOMIZE" "#include <sys/personality.h>
> -"
> -if test "x$ac_cv_have_decl_ADDR_NO_RANDOMIZE" = xyes; then :
> -  ac_have_decl=1
> -else
> -  ac_have_decl=0
> -fi
> -
> -cat >>confdefs.h <<_ACEOF
> -#define HAVE_DECL_ADDR_NO_RANDOMIZE $ac_have_decl
> -_ACEOF
> -
> -
> -  if test "$cross_compiling" = yes; then :
> -  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> -/* end confdefs.h.  */
> -#include <sys/personality.h>
> -int
> -main ()
> -{
> -
> -  #      if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -  #       define ADDR_NO_RANDOMIZE 0x0040000
> -  #      endif
> -	 /* Test the flag could be set and stays set.  */
> -	 personality (personality (0xffffffff) | ADDR_NO_RANDOMIZE);
> -	 if (!(personality (personality (0xffffffff)) & ADDR_NO_RANDOMIZE))
> -	     return 1
> -  ;
> -  return 0;
> -}
> -_ACEOF
> -if ac_fn_c_try_link "$LINENO"; then :
> -  have_personality=true
> -else
> -  have_personality=false
> -fi
> -rm -f core conftest.err conftest.$ac_objext \
> -    conftest$ac_exeext conftest.$ac_ext
> -else
> -  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> -/* end confdefs.h.  */
> -#include <sys/personality.h>
> -int
> -main ()
> -{
> -
> -  #      if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -  #       define ADDR_NO_RANDOMIZE 0x0040000
> -  #      endif
> -	 /* Test the flag could be set and stays set.  */
> -	 personality (personality (0xffffffff) | ADDR_NO_RANDOMIZE);
> -	 if (!(personality (personality (0xffffffff)) & ADDR_NO_RANDOMIZE))
> -	     return 1
> -  ;
> -  return 0;
> -}
> -_ACEOF
> -if ac_fn_c_try_run "$LINENO"; then :
> -  have_personality=true
> -else
> -  have_personality=false
> -fi
> -rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
> -  conftest.$ac_objext conftest.beam conftest.$ac_ext
> -fi
> -
> -  if $have_personality
> -  then
> -
> -$as_echo "#define HAVE_PERSONALITY 1" >>confdefs.h
> -
> -  fi
> -
>    ac_fn_c_check_decl "$LINENO" "strstr" "ac_cv_have_decl_strstr" "$ac_includes_default"
>  if test "x$ac_cv_have_decl_strstr" = xyes; then :
>    ac_have_decl=1
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index c45e335a762..e595c71c3d3 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4022,11 +4022,7 @@ linux_nat_target::supports_multi_process ()
>  bool
>  linux_nat_target::supports_disable_randomization ()
>  {
> -#ifdef HAVE_PERSONALITY
>    return true;
> -#else
> -  return false;
> -#endif
>  }
>  
>  /* SIGCHLD handler that serves two purposes: In non-stop/async mode,
> diff --git a/gdb/nat/linux-personality.c b/gdb/nat/linux-personality.c
> index 87d19218626..9ce345bc6de 100644
> --- a/gdb/nat/linux-personality.c
> +++ b/gdb/nat/linux-personality.c
> @@ -20,12 +20,7 @@
>  #include "gdbsupport/common-defs.h"
>  #include "nat/linux-personality.h"
>  
> -#ifdef HAVE_PERSONALITY
> -# include <sys/personality.h>
> -# if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -#  define ADDR_NO_RANDOMIZE 0x0040000
> -# endif /* ! HAVE_DECL_ADDR_NO_RANDOMIZE */
> -#endif /* HAVE_PERSONALITY */
> +#include <sys/personality.h>
>  
>  /* See comment on nat/linux-personality.h.  */
>  
> @@ -34,7 +29,6 @@ maybe_disable_address_space_randomization (int disable_randomization)
>    : m_personality_set (false),
>      m_personality_orig (0)
>  {
> -#ifdef HAVE_PERSONALITY
>    if (disable_randomization)
>      {
>        errno = 0;
> @@ -49,14 +43,11 @@ maybe_disable_address_space_randomization (int disable_randomization)
>  	warning (_("Error disabling address space randomization: %s"),
>  		 safe_strerror (errno));
>      }
> -#endif /* HAVE_PERSONALITY */
>  }
>  
>  maybe_disable_address_space_randomization::
>  ~maybe_disable_address_space_randomization ()
>  {
> -#ifdef HAVE_PERSONALITY
> -
>    if (m_personality_set)
>      {
>        errno = 0;
> @@ -65,5 +56,4 @@ maybe_disable_address_space_randomization::
>  	warning (_("Error restoring address space randomization: %s"),
>  		 safe_strerror (errno));
>      }
> -#endif /* HAVE_PERSONALITY */
>  }
> diff --git a/gdbserver/config.in b/gdbserver/config.in
> index 99391b4acff..611bfd7aa76 100644
> --- a/gdbserver/config.in
> +++ b/gdbserver/config.in
> @@ -31,10 +31,6 @@
>  /* define if the compiler supports basic C++11 syntax */
>  #undef HAVE_CXX11
>  
> -/* Define to 1 if you have the declaration of `ADDR_NO_RANDOMIZE', and to 0 if
> -   you don't. */
> -#undef HAVE_DECL_ADDR_NO_RANDOMIZE
> -
>  /* Define to 1 if you have the declaration of `asprintf', and to 0 if you
>     don't. */
>  #undef HAVE_DECL_ASPRINTF
> @@ -182,9 +178,6 @@
>  /* Define to 1 if you have the <netinet/tcp.h> header file. */
>  #undef HAVE_NETINET_TCP_H
>  
> -/* Define if you support the personality syscall. */
> -#undef HAVE_PERSONALITY
> -
>  /* Define to 1 if you have the `pipe' function. */
>  #undef HAVE_PIPE
>  
> diff --git a/gdbserver/configure b/gdbserver/configure
> index 032b4ae65bb..aab72c0b8c5 100755
> --- a/gdbserver/configure
> +++ b/gdbserver/configure
> @@ -7130,80 +7130,6 @@ fi
>  done
>  
>  
> -      ac_fn_c_check_decl "$LINENO" "ADDR_NO_RANDOMIZE" "ac_cv_have_decl_ADDR_NO_RANDOMIZE" "#include <sys/personality.h>
> -"
> -if test "x$ac_cv_have_decl_ADDR_NO_RANDOMIZE" = xyes; then :
> -  ac_have_decl=1
> -else
> -  ac_have_decl=0
> -fi
> -
> -cat >>confdefs.h <<_ACEOF
> -#define HAVE_DECL_ADDR_NO_RANDOMIZE $ac_have_decl
> -_ACEOF
> -
> -
> -  if test "$cross_compiling" = yes; then :
> -  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> -/* end confdefs.h.  */
> -#include <sys/personality.h>
> -int
> -main ()
> -{
> -
> -  #      if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -  #       define ADDR_NO_RANDOMIZE 0x0040000
> -  #      endif
> -	 /* Test the flag could be set and stays set.  */
> -	 personality (personality (0xffffffff) | ADDR_NO_RANDOMIZE);
> -	 if (!(personality (personality (0xffffffff)) & ADDR_NO_RANDOMIZE))
> -	     return 1
> -  ;
> -  return 0;
> -}
> -_ACEOF
> -if ac_fn_c_try_link "$LINENO"; then :
> -  have_personality=true
> -else
> -  have_personality=false
> -fi
> -rm -f core conftest.err conftest.$ac_objext \
> -    conftest$ac_exeext conftest.$ac_ext
> -else
> -  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> -/* end confdefs.h.  */
> -#include <sys/personality.h>
> -int
> -main ()
> -{
> -
> -  #      if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -  #       define ADDR_NO_RANDOMIZE 0x0040000
> -  #      endif
> -	 /* Test the flag could be set and stays set.  */
> -	 personality (personality (0xffffffff) | ADDR_NO_RANDOMIZE);
> -	 if (!(personality (personality (0xffffffff)) & ADDR_NO_RANDOMIZE))
> -	     return 1
> -  ;
> -  return 0;
> -}
> -_ACEOF
> -if ac_fn_c_try_run "$LINENO"; then :
> -  have_personality=true
> -else
> -  have_personality=false
> -fi
> -rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
> -  conftest.$ac_objext conftest.beam conftest.$ac_ext
> -fi
> -
> -  if $have_personality
> -  then
> -
> -$as_echo "#define HAVE_PERSONALITY 1" >>confdefs.h
> -
> -  fi
> -
>    ac_fn_c_check_decl "$LINENO" "strstr" "ac_cv_have_decl_strstr" "$ac_includes_default"
>  if test "x$ac_cv_have_decl_strstr" = xyes; then :
>    ac_have_decl=1
> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> index 251a54f5c4a..27556ffb88a 100644
> --- a/gdbserver/linux-low.cc
> +++ b/gdbserver/linux-low.cc
> @@ -60,12 +60,7 @@
>  #endif
>  #include "nat/linux-namespaces.h"
>  
> -#ifdef HAVE_PERSONALITY
> -# include <sys/personality.h>
> -# if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -#  define ADDR_NO_RANDOMIZE 0x0040000
> -# endif
> -#endif
> +#include <sys/personality.h>

Hi,

From what I can see, this include is not needed in this file anymore and
could be removed altogether.  It seems to have been forgotten in a past
refactoring[1].

Lancelot.

[1] https://sourceware.org/pipermail/gdb-patches/2015-January/120503.html

>  
>  #ifndef O_LARGEFILE
>  #define O_LARGEFILE 0
> @@ -6231,11 +6226,7 @@ linux_process_target::core_of_thread (ptid_t ptid)
>  bool
>  linux_process_target::supports_disable_randomization ()
>  {
> -#ifdef HAVE_PERSONALITY
>    return true;
> -#else
> -  return false;
> -#endif
>  }
>  
>  bool
> diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
> index 3ed95dabc85..2e709dbbdbb 100644
> --- a/gdbsupport/common.m4
> +++ b/gdbsupport/common.m4
> @@ -55,29 +55,6 @@ AC_DEFUN([GDB_AC_COMMON], [
>  		  ptrace64 sbrk setns sigaltstack sigprocmask \
>  		  setpgid setpgrp getrusage getauxval])
>  
> -  dnl Check if we can disable the virtual address space randomization.
> -  dnl The functionality of setarch -R.
> -  AC_CHECK_DECLS([ADDR_NO_RANDOMIZE],,, [#include <sys/personality.h>])
> -  define([PERSONALITY_TEST], [AC_LANG_PROGRAM([#include <sys/personality.h>], [
> -  #      if !HAVE_DECL_ADDR_NO_RANDOMIZE
> -  #       define ADDR_NO_RANDOMIZE 0x0040000
> -  #      endif
> -	 /* Test the flag could be set and stays set.  */
> -	 personality (personality (0xffffffff) | ADDR_NO_RANDOMIZE);
> -	 if (!(personality (personality (0xffffffff)) & ADDR_NO_RANDOMIZE))
> -	     return 1])])
> -  AC_RUN_IFELSE([PERSONALITY_TEST],
> -		[have_personality=true],
> -		[have_personality=false],
> -		[AC_LINK_IFELSE([PERSONALITY_TEST],
> -				[have_personality=true],
> -				[have_personality=false])])
> -  if $have_personality
> -  then
> -      AC_DEFINE([HAVE_PERSONALITY], 1,
> -		[Define if you support the personality syscall.])
> -  fi
> -
>    AC_CHECK_DECLS([strstr])
>  
>    # ----------------------- #
> 
> base-commit: ec66d6ea54679d00c1f0d52a5a9fa0cbb8b86d47
> -- 
> 2.26.2
> 

-- 
Lancelot SIX