[PATCH 0/7] configure: Improve PIE and other linkage

[PATCH 0/7] configure: Improve PIE and other linkage

[Richard Henderson]

This begins by dropping the -Ttext-segment stuff, which Fangrui Song
correctly points out does not work with lld.  But it's also obsolete,
so instead of adding support for lld's --image-base, remove it all.

Then, remove some other legacy random addresses that were supposed
to apply to softmmu, but didn't really make any sense, and aren't
used anyway when PIE is used, which is the default with a modern
linux distribution.

Then, clean up some of the configure logic surrounding PIE, and its
current non-application to non-x86.

Finally, add support for static-pie linking.


r~


Richard Henderson (7):
  configure: Drop adjustment of textseg
  tcg: Remove softmmu code_gen_buffer fixed address
  configure: Do not force pie=no for non-x86
  configure: Always detect -no-pie toolchain support
  configure: Unnest detection of -z,relro and -z,now
  configure: Override the os default with --disable-pie
  configure: Support -static-pie if requested

 accel/tcg/translate-all.c |  37 ++----------
 configure                 | 120 ++++++++++++--------------------------
 2 files changed, 41 insertions(+), 116 deletions(-)

-- 
2.20.1

[PATCH 1/7] configure: Drop adjustment of textseg

[Richard Henderson]

This adjustment was random and unnecessary.  The user mode
startup code in probe_guest_base() will choose a value for
guest_base that allows the host qemu binary to not conflict
with the guest binary.

With modern distributions, this isn't even used, as the default
is PIE, which does the same job in a more portable way.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 47 -----------------------------------------------
 1 file changed, 47 deletions(-)

diff --git a/configure b/configure
index 84b413dbfc..255ac432af 100755
--- a/configure
+++ b/configure
@@ -6292,49 +6292,6 @@ if test "$cpu" = "s390x" ; then
   fi
 fi
 
-# Probe for the need for relocating the user-only binary.
-if ( [ "$linux_user" = yes ] || [ "$bsd_user" = yes ] ) && [ "$pie" = no ]; then
-  textseg_addr=
-  case "$cpu" in
-    arm | i386 | ppc* | s390* | sparc* | x86_64 | x32)
-      # ??? Rationale for choosing this address
-      textseg_addr=0x60000000
-      ;;
-    mips)
-      # A 256M aligned address, high in the address space, with enough
-      # room for the code_gen_buffer above it before the stack.
-      textseg_addr=0x60000000
-      ;;
-  esac
-  if [ -n "$textseg_addr" ]; then
-    cat > $TMPC <<EOF
-    int main(void) { return 0; }
-EOF
-    textseg_ldflags="-Wl,-Ttext-segment=$textseg_addr"
-    if ! compile_prog "" "$textseg_ldflags"; then
-      # In case ld does not support -Ttext-segment, edit the default linker
-      # script via sed to set the .text start addr.  This is needed on FreeBSD
-      # at least.
-      if ! $ld --verbose >/dev/null 2>&1; then
-        error_exit \
-            "We need to link the QEMU user mode binaries at a" \
-            "specific text address. Unfortunately your linker" \
-            "doesn't support either the -Ttext-segment option or" \
-            "printing the default linker script with --verbose." \
-            "If you don't want the user mode binaries, pass the" \
-            "--disable-user option to configure."
-      fi
-
-      $ld --verbose | sed \
-        -e '1,/==================================================/d' \
-        -e '/==================================================/,$d' \
-        -e "s/[.] = [0-9a-fx]* [+] SIZEOF_HEADERS/. = $textseg_addr + SIZEOF_HEADERS/" \
-        -e "s/__executable_start = [0-9a-fx]*/__executable_start = $textseg_addr/" > config-host.ld
-      textseg_ldflags="-Wl,-T../config-host.ld"
-    fi
-  fi
-fi
-
 # Check that the C++ compiler exists and works with the C compiler.
 # All the QEMU_CXXFLAGS are based on QEMU_CFLAGS. Keep this at the end to don't miss any other that could be added.
 if has $cxx; then
@@ -7897,10 +7854,6 @@ if test "$gprof" = "yes" ; then
   fi
 fi
 
-if test "$target_linux_user" = "yes" || test "$target_bsd_user" = "yes" ; then
-  ldflags="$ldflags $textseg_ldflags"
-fi
-
 # Newer kernels on s390 check for an S390_PGSTE program header and
 # enable the pgste page table extensions in that case. This makes
 # the vm.allocate_pgste sysctl unnecessary. We enable this program
-- 
2.20.1

Re: [PATCH 1/7] configure: Drop adjustment of textseg

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> This adjustment was random and unnecessary.  The user mode
> startup code in probe_guest_base() will choose a value for
> guest_base that allows the host qemu binary to not conflict
> with the guest binary.
> 
> With modern distributions, this isn't even used, as the default
> is PIE, which does the same job in a more portable way.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  configure | 47 -----------------------------------------------
>  1 file changed, 47 deletions(-)
> 
> diff --git a/configure b/configure
> index 84b413dbfc..255ac432af 100755
> --- a/configure
> +++ b/configure
> @@ -6292,49 +6292,6 @@ if test "$cpu" = "s390x" ; then
>    fi
>  fi
>  
> -# Probe for the need for relocating the user-only binary.
> -if ( [ "$linux_user" = yes ] || [ "$bsd_user" = yes ] ) && [ "$pie" = no ]; then
> -  textseg_addr=
> -  case "$cpu" in
> -    arm | i386 | ppc* | s390* | sparc* | x86_64 | x32)
> -      # ??? Rationale for choosing this address
> -      textseg_addr=0x60000000
> -      ;;
> -    mips)
> -      # A 256M aligned address, high in the address space, with enough
> -      # room for the code_gen_buffer above it before the stack.
> -      textseg_addr=0x60000000
> -      ;;
> -  esac
> -  if [ -n "$textseg_addr" ]; then
> -    cat > $TMPC <<EOF
> -    int main(void) { return 0; }
> -EOF
> -    textseg_ldflags="-Wl,-Ttext-segment=$textseg_addr"
> -    if ! compile_prog "" "$textseg_ldflags"; then
> -      # In case ld does not support -Ttext-segment, edit the default linker
> -      # script via sed to set the .text start addr.  This is needed on FreeBSD
> -      # at least.
> -      if ! $ld --verbose >/dev/null 2>&1; then
> -        error_exit \
> -            "We need to link the QEMU user mode binaries at a" \
> -            "specific text address. Unfortunately your linker" \
> -            "doesn't support either the -Ttext-segment option or" \
> -            "printing the default linker script with --verbose." \
> -            "If you don't want the user mode binaries, pass the" \
> -            "--disable-user option to configure."
> -      fi
> -
> -      $ld --verbose | sed \
> -        -e '1,/==================================================/d' \
> -        -e '/==================================================/,$d' \
> -        -e "s/[.] = [0-9a-fx]* [+] SIZEOF_HEADERS/. = $textseg_addr + SIZEOF_HEADERS/" \
> -        -e "s/__executable_start = [0-9a-fx]*/__executable_start = $textseg_addr/" > config-host.ld
> -      textseg_ldflags="-Wl,-T../config-host.ld"

config-host.ld is mentioned one more time in the main "Makefile" ... I
think you could remove it from there now, too.

With such a hunk added:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 1/7] configure: Drop adjustment of textseg

[Richard Henderson]

On 12/17/19 8:59 PM, Thomas Huth wrote:
> config-host.ld is mentioned one more time in the main "Makefile" ... I
> think you could remove it from there now, too.

Done, thanks.


r~

[PATCH 2/7] tcg: Remove softmmu code_gen_buffer fixed address

[Richard Henderson]

The commentary talks about "in concert with the addresses
assigned in the relevant linker script", except there is no
linker script for softmmu, nor has there been for some time.

(Do not confuse the user-only linker script editing that was
removed in the previous patch, because user-only does not
use this code_gen_buffer allocation method.)

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/translate-all.c | 37 +++++--------------------------------
 1 file changed, 5 insertions(+), 32 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9f48da9472..88468a1c08 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1032,47 +1032,20 @@ static inline void *alloc_code_gen_buffer(void)
 {
     int prot = PROT_WRITE | PROT_READ | PROT_EXEC;
     int flags = MAP_PRIVATE | MAP_ANONYMOUS;
-    uintptr_t start = 0;
     size_t size = tcg_ctx->code_gen_buffer_size;
     void *buf;
 
-    /* Constrain the position of the buffer based on the host cpu.
-       Note that these addresses are chosen in concert with the
-       addresses assigned in the relevant linker script file.  */
-# if defined(__PIE__) || defined(__PIC__)
-    /* Don't bother setting a preferred location if we're building
-       a position-independent executable.  We're more likely to get
-       an address near the main executable if we let the kernel
-       choose the address.  */
-# elif defined(__x86_64__) && defined(MAP_32BIT)
-    /* Force the memory down into low memory with the executable.
-       Leave the choice of exact location with the kernel.  */
-    flags |= MAP_32BIT;
-    /* Cannot expect to map more than 800MB in low memory.  */
-    if (size > 800u * 1024 * 1024) {
-        tcg_ctx->code_gen_buffer_size = size = 800u * 1024 * 1024;
-    }
-# elif defined(__sparc__)
-    start = 0x40000000ul;
-# elif defined(__s390x__)
-    start = 0x90000000ul;
-# elif defined(__mips__)
-#  if _MIPS_SIM == _ABI64
-    start = 0x128000000ul;
-#  else
-    start = 0x08000000ul;
-#  endif
-# endif
-
-    buf = mmap((void *)start, size, prot, flags, -1, 0);
+    buf = mmap(NULL, size, prot, flags, -1, 0);
     if (buf == MAP_FAILED) {
         return NULL;
     }
 
 #ifdef __mips__
     if (cross_256mb(buf, size)) {
-        /* Try again, with the original still mapped, to avoid re-acquiring
-           that 256mb crossing.  This time don't specify an address.  */
+        /*
+         * Try again, with the original still mapped, to avoid re-acquiring
+         * the same 256mb crossing.
+         */
         size_t size2;
         void *buf2 = mmap(NULL, size, prot, flags, -1, 0);
         switch ((int)(buf2 != MAP_FAILED)) {
-- 
2.20.1

Re: [PATCH 2/7] tcg: Remove softmmu code_gen_buffer fixed address

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> The commentary talks about "in concert with the addresses
> assigned in the relevant linker script", except there is no
> linker script for softmmu, nor has there been for some time.
> 
> (Do not confuse the user-only linker script editing that was
> removed in the previous patch, because user-only does not
> use this code_gen_buffer allocation method.)
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/translate-all.c | 37 +++++--------------------------------
>  1 file changed, 5 insertions(+), 32 deletions(-)
> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 9f48da9472..88468a1c08 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1032,47 +1032,20 @@ static inline void *alloc_code_gen_buffer(void)
>  {
>      int prot = PROT_WRITE | PROT_READ | PROT_EXEC;
>      int flags = MAP_PRIVATE | MAP_ANONYMOUS;
> -    uintptr_t start = 0;
>      size_t size = tcg_ctx->code_gen_buffer_size;
>      void *buf;
>  
> -    /* Constrain the position of the buffer based on the host cpu.
> -       Note that these addresses are chosen in concert with the
> -       addresses assigned in the relevant linker script file.  */
> -# if defined(__PIE__) || defined(__PIC__)
> -    /* Don't bother setting a preferred location if we're building
> -       a position-independent executable.  We're more likely to get
> -       an address near the main executable if we let the kernel
> -       choose the address.  */
> -# elif defined(__x86_64__) && defined(MAP_32BIT)
> -    /* Force the memory down into low memory with the executable.
> -       Leave the choice of exact location with the kernel.  */
> -    flags |= MAP_32BIT;
> -    /* Cannot expect to map more than 800MB in low memory.  */
> -    if (size > 800u * 1024 * 1024) {
> -        tcg_ctx->code_gen_buffer_size = size = 800u * 1024 * 1024;
> -    }
> -# elif defined(__sparc__)
> -    start = 0x40000000ul;
> -# elif defined(__s390x__)
> -    start = 0x90000000ul;
> -# elif defined(__mips__)
> -#  if _MIPS_SIM == _ABI64
> -    start = 0x128000000ul;
> -#  else
> -    start = 0x08000000ul;
> -#  endif
> -# endif
> -
> -    buf = mmap((void *)start, size, prot, flags, -1, 0);
> +    buf = mmap(NULL, size, prot, flags, -1, 0);
>      if (buf == MAP_FAILED) {
>          return NULL;
>      }
>  
>  #ifdef __mips__
>      if (cross_256mb(buf, size)) {
> -        /* Try again, with the original still mapped, to avoid re-acquiring
> -           that 256mb crossing.  This time don't specify an address.  */
> +        /*
> +         * Try again, with the original still mapped, to avoid re-acquiring
> +         * the same 256mb crossing.
> +         */
>          size_t size2;
>          void *buf2 = mmap(NULL, size, prot, flags, -1, 0);
>          switch ((int)(buf2 != MAP_FAILED)) {
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 3/7] configure: Do not force pie=no for non-x86

[Richard Henderson]

PIE is supported on many other hosts besides x86.

The default for non-x86 is now the same as x86: pie is used
if supported, and may be forced via --enable/--disable-pie.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/configure b/configure
index 255ac432af..2fb4457d7c 100755
--- a/configure
+++ b/configure
@@ -2012,16 +2012,6 @@ if ! compile_prog "-Werror" "" ; then
 	"Thread-Local Storage (TLS). Please upgrade to a version that does."
 fi
 
-if test "$pie" = ""; then
-  case "$cpu-$targetos" in
-    i386-Linux|x86_64-Linux|x32-Linux|i386-OpenBSD|x86_64-OpenBSD)
-      ;;
-    *)
-      pie="no"
-      ;;
-  esac
-fi
-
 if test "$pie" != "no" ; then
   cat > $TMPC << EOF
 
-- 
2.20.1

Re: [PATCH 3/7] configure: Do not force pie=no for non-x86

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> PIE is supported on many other hosts besides x86.
> 
> The default for non-x86 is now the same as x86: pie is used
> if supported, and may be forced via --enable/--disable-pie.

The original commit that introduce this code (40d6444e91c) said:

 "Non-x86 are not changed, as they require TCG changes"

... are these "TCG changes" in place nowadays? Did you check on non-x86
systems? If so, please mention this in the commit message.

 Thomas

Re: [PATCH 3/7] configure: Do not force pie=no for non-x86

[Richard Henderson]

On 12/17/19 9:05 PM, Thomas Huth wrote:
> On 18/12/2019 04.19, Richard Henderson wrote:
>> PIE is supported on many other hosts besides x86.
>>
>> The default for non-x86 is now the same as x86: pie is used
>> if supported, and may be forced via --enable/--disable-pie.
> 
> The original commit that introduce this code (40d6444e91c) said:
> 
>  "Non-x86 are not changed, as they require TCG changes"
> 
> ... are these "TCG changes" in place nowadays? Did you check on non-x86
> systems? If so, please mention this in the commit message.

I have no idea what those "tcg changes" would be.  I have checked aarch64
(bionic) and ppc64le (centos 7).  I'll update the commit message.


r~

[PATCH 4/7] configure: Always detect -no-pie toolchain support

[Richard Henderson]

The CFLAGS_NOPIE and LDFLAGS_NOPIE variables are used
in pc-bios/optionrom/Makefile, which has nothing to do
with the PIE setting of the main qemu executables.

This overrides any operating system default to build
all executables as PIE, which is important for ROMs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 2fb4457d7c..972ce7396f 100755
--- a/configure
+++ b/configure
@@ -2012,26 +2012,24 @@ if ! compile_prog "-Werror" "" ; then
 	"Thread-Local Storage (TLS). Please upgrade to a version that does."
 fi
 
-if test "$pie" != "no" ; then
-  cat > $TMPC << EOF
+cat > $TMPC << EOF
 
 #ifdef __linux__
 #  define THREAD __thread
 #else
 #  define THREAD
 #endif
-
 static THREAD int tls_var;
-
 int main(void) { return tls_var; }
-
 EOF
-  # check we support --no-pie first...
-  if compile_prog "-Werror -fno-pie" "-no-pie"; then
-    CFLAGS_NOPIE="-fno-pie"
-    LDFLAGS_NOPIE="-nopie"
-  fi
 
+# Check we support --no-pie first; we will need this for building ROMs.
+if compile_prog "-Werror -fno-pie" "-no-pie"; then
+  CFLAGS_NOPIE="-fno-pie"
+  LDFLAGS_NOPIE="-no-pie"
+fi
+
+if test "$pie" != "no" ; then
   if compile_prog "-fPIE -DPIE" "-pie"; then
     QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
     LDFLAGS="-pie $LDFLAGS"
-- 
2.20.1

Re: [PATCH 4/7] configure: Always detect -no-pie toolchain support

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> The CFLAGS_NOPIE and LDFLAGS_NOPIE variables are used
> in pc-bios/optionrom/Makefile, which has nothing to do
> with the PIE setting of the main qemu executables.
> 
> This overrides any operating system default to build
> all executables as PIE, which is important for ROMs.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  configure | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 4/7] configure: Always detect -no-pie toolchain support

[Philippe Mathieu-Daudé]

On 12/18/19 4:19 AM, Richard Henderson wrote:
> The CFLAGS_NOPIE and LDFLAGS_NOPIE variables are used
> in pc-bios/optionrom/Makefile, which has nothing to do
> with the PIE setting of the main qemu executables.
> 
> This overrides any operating system default to build
> all executables as PIE, which is important for ROMs.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   configure | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/configure b/configure
> index 2fb4457d7c..972ce7396f 100755
> --- a/configure
> +++ b/configure
> @@ -2012,26 +2012,24 @@ if ! compile_prog "-Werror" "" ; then
>   	"Thread-Local Storage (TLS). Please upgrade to a version that does."
>   fi
>   
> -if test "$pie" != "no" ; then
> -  cat > $TMPC << EOF
> +cat > $TMPC << EOF
>   
>   #ifdef __linux__
>   #  define THREAD __thread
>   #else
>   #  define THREAD
>   #endif
> -
>   static THREAD int tls_var;
> -
>   int main(void) { return tls_var; }
> -
>   EOF
> -  # check we support --no-pie first...
> -  if compile_prog "-Werror -fno-pie" "-no-pie"; then
> -    CFLAGS_NOPIE="-fno-pie"
> -    LDFLAGS_NOPIE="-nopie"
> -  fi
>   
> +# Check we support --no-pie first; we will need this for building ROMs.
> +if compile_prog "-Werror -fno-pie" "-no-pie"; then
> +  CFLAGS_NOPIE="-fno-pie"
> +  LDFLAGS_NOPIE="-no-pie"
> +fi
> +
> +if test "$pie" != "no" ; then
>     if compile_prog "-fPIE -DPIE" "-pie"; then
>       QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
>       LDFLAGS="-pie $LDFLAGS"
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 5/7] configure: Unnest detection of -z,relro and -z,now

[Richard Henderson]

There is nothing about these options that is related to PIE.
Nor is there anything that specifically ties them to each other.
Use them unconditionally.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 972ce7396f..f8981eec15 100755
--- a/configure
+++ b/configure
@@ -2034,9 +2034,6 @@ if test "$pie" != "no" ; then
     QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
     LDFLAGS="-pie $LDFLAGS"
     pie="yes"
-    if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then
-      LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
-    fi
   else
     if test "$pie" = "yes"; then
       error_exit "PIE not available due to missing toolchain support"
@@ -2047,6 +2044,16 @@ if test "$pie" != "no" ; then
   fi
 fi
 
+# Detect support for DT_BIND_NOW.
+if compile_prog "" "-Wl,-z,now" ; then
+  LDFLAGS="-Wl,-z,now $LDFLAGS"
+fi
+
+# Detect support for PT_GNU_RELRO.
+if compile_prog "" "-Wl,-z,relro" ; then
+  LDFLAGS="-Wl,-z,relro $LDFLAGS"
+fi
+
 ##########################################
 # __sync_fetch_and_and requires at least -march=i486. Many toolchains
 # use i686 as default anyway, but for those that don't, an explicit
-- 
2.20.1

Re: [PATCH 5/7] configure: Unnest detection of -z,relro and -z,now

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> There is nothing about these options that is related to PIE.
> Nor is there anything that specifically ties them to each other.
> Use them unconditionally.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  configure | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 972ce7396f..f8981eec15 100755
> --- a/configure
> +++ b/configure
> @@ -2034,9 +2034,6 @@ if test "$pie" != "no" ; then
>      QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
>      LDFLAGS="-pie $LDFLAGS"
>      pie="yes"
> -    if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then
> -      LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
> -    fi
>    else
>      if test "$pie" = "yes"; then
>        error_exit "PIE not available due to missing toolchain support"
> @@ -2047,6 +2044,16 @@ if test "$pie" != "no" ; then
>    fi
>  fi
>  
> +# Detect support for DT_BIND_NOW.
> +if compile_prog "" "-Wl,-z,now" ; then
> +  LDFLAGS="-Wl,-z,now $LDFLAGS"
> +fi
> +
> +# Detect support for PT_GNU_RELRO.
> +if compile_prog "" "-Wl,-z,relro" ; then
> +  LDFLAGS="-Wl,-z,relro $LDFLAGS"
> +fi

Looking at
https://mudongliang.github.io/2016/07/11/relro-a-not-so-well-known-memory-corruption-mitigation-technique.html
the idea of specifying these two options together was likely to get
"Full RELRO" instead of only "Partial RELRO".
Thus, does it make sense to have "-Wl,-z,now" without "-Wl,-z,relro" in
QEMU? Or should this rather check whether both are possible, then use
both, otherwise just try to use "relro" alone?

 Thomas

Re: [PATCH 5/7] configure: Unnest detection of -z,relro and -z,now

[Richard Henderson]

On 12/17/19 10:11 PM, Thomas Huth wrote:
>> +# Detect support for DT_BIND_NOW.
>> +if compile_prog "" "-Wl,-z,now" ; then
>> +  LDFLAGS="-Wl,-z,now $LDFLAGS"
>> +fi
>> +
>> +# Detect support for PT_GNU_RELRO.
>> +if compile_prog "" "-Wl,-z,relro" ; then
>> +  LDFLAGS="-Wl,-z,relro $LDFLAGS"
>> +fi
> 
> Looking at
> https://mudongliang.github.io/2016/07/11/relro-a-not-so-well-known-memory-corruption-mitigation-technique.html
> the idea of specifying these two options together was likely to get
> "Full RELRO" instead of only "Partial RELRO".

Sure.

> Thus, does it make sense to have "-Wl,-z,now" without "-Wl,-z,relro" in
> QEMU? Or should this rather check whether both are possible, then use
> both, otherwise just try to use "relro" alone?

Honestly, I expect them both to be supported in any binutils.

I split the two tests just because they didn't seem to be logically connected.
 But I had forgotten about, or perhaps never heard, the terms "full" and
"partial" relro.

I can put them back together with an appropriate comment it you like.  One less
thing to run during configure...


r~

Re: [PATCH 5/7] configure: Unnest detection of -z,relro and -z,now

[Thomas Huth]

On 18/12/2019 17.58, Richard Henderson wrote:
> On 12/17/19 10:11 PM, Thomas Huth wrote:
>>> +# Detect support for DT_BIND_NOW.
>>> +if compile_prog "" "-Wl,-z,now" ; then
>>> +  LDFLAGS="-Wl,-z,now $LDFLAGS"
>>> +fi
>>> +
>>> +# Detect support for PT_GNU_RELRO.
>>> +if compile_prog "" "-Wl,-z,relro" ; then
>>> +  LDFLAGS="-Wl,-z,relro $LDFLAGS"
>>> +fi
>>
>> Looking at
>> https://mudongliang.github.io/2016/07/11/relro-a-not-so-well-known-memory-corruption-mitigation-technique.html
>> the idea of specifying these two options together was likely to get
>> "Full RELRO" instead of only "Partial RELRO".
> 
> Sure.
> 
>> Thus, does it make sense to have "-Wl,-z,now" without "-Wl,-z,relro" in
>> QEMU? Or should this rather check whether both are possible, then use
>> both, otherwise just try to use "relro" alone?
> 
> Honestly, I expect them both to be supported in any binutils.
> 
> I split the two tests just because they didn't seem to be logically connected.
>  But I had forgotten about, or perhaps never heard, the terms "full" and
> "partial" relro.
> 
> I can put them back together with an appropriate comment it you like.  One less
> thing to run during configure...

Yes, please.

 Thanks,
  Thomas

[PATCH 6/7] configure: Override the os default with --disable-pie

[Richard Henderson]

Some distributions, e.g. Ubuntu 19.10, enable PIE by default.
If for some reason one wishes to build a non-pie binary, we
must provide additional options to override.

At the same time, reorg the code to an elif chain.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/configure b/configure
index f8981eec15..1645a58b3a 100755
--- a/configure
+++ b/configure
@@ -2029,19 +2029,18 @@ if compile_prog "-Werror -fno-pie" "-no-pie"; then
   LDFLAGS_NOPIE="-no-pie"
 fi
 
-if test "$pie" != "no" ; then
-  if compile_prog "-fPIE -DPIE" "-pie"; then
-    QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
-    LDFLAGS="-pie $LDFLAGS"
-    pie="yes"
-  else
-    if test "$pie" = "yes"; then
-      error_exit "PIE not available due to missing toolchain support"
-    else
-      echo "Disabling PIE due to missing toolchain support"
-      pie="no"
-    fi
-  fi
+if test "$pie" = "no"; then
+  QEMU_CFLAGS="$CFLAGS_NOPIE $QEMU_CFLAGS"
+  LDFLAGS="$LDFLAGS_NOPIE $LDFLAGS"
+elif compile_prog "-fPIE -DPIE" "-pie"; then
+  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
+  LDFLAGS="-pie $LDFLAGS"
+  pie="yes"
+elif test "$pie" = "yes"; then
+  error_exit "PIE not available due to missing toolchain support"
+else
+  echo "Disabling PIE due to missing toolchain support"
+  pie="no"
 fi
 
 # Detect support for DT_BIND_NOW.
-- 
2.20.1

Re: [PATCH 6/7] configure: Override the os default with --disable-pie

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> Some distributions, e.g. Ubuntu 19.10, enable PIE by default.
> If for some reason one wishes to build a non-pie binary, we
> must provide additional options to override.
> 
> At the same time, reorg the code to an elif chain.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  configure | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/configure b/configure
> index f8981eec15..1645a58b3a 100755
> --- a/configure
> +++ b/configure
> @@ -2029,19 +2029,18 @@ if compile_prog "-Werror -fno-pie" "-no-pie"; then
>    LDFLAGS_NOPIE="-no-pie"
>  fi
>  
> -if test "$pie" != "no" ; then
> -  if compile_prog "-fPIE -DPIE" "-pie"; then
> -    QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> -    LDFLAGS="-pie $LDFLAGS"
> -    pie="yes"
> -  else
> -    if test "$pie" = "yes"; then
> -      error_exit "PIE not available due to missing toolchain support"
> -    else
> -      echo "Disabling PIE due to missing toolchain support"
> -      pie="no"
> -    fi
> -  fi
> +if test "$pie" = "no"; then
> +  QEMU_CFLAGS="$CFLAGS_NOPIE $QEMU_CFLAGS"
> +  LDFLAGS="$LDFLAGS_NOPIE $LDFLAGS"
> +elif compile_prog "-fPIE -DPIE" "-pie"; then
> +  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> +  LDFLAGS="-pie $LDFLAGS"
> +  pie="yes"
> +elif test "$pie" = "yes"; then
> +  error_exit "PIE not available due to missing toolchain support"
> +else
> +  echo "Disabling PIE due to missing toolchain support"
> +  pie="no"
>  fi
>  
>  # Detect support for DT_BIND_NOW.
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 6/7] configure: Override the os default with --disable-pie

[Philippe Mathieu-Daudé]

On 12/18/19 4:19 AM, Richard Henderson wrote:
> Some distributions, e.g. Ubuntu 19.10, enable PIE by default.
> If for some reason one wishes to build a non-pie binary, we
> must provide additional options to override.
> 
> At the same time, reorg the code to an elif chain.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   configure | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/configure b/configure
> index f8981eec15..1645a58b3a 100755
> --- a/configure
> +++ b/configure
> @@ -2029,19 +2029,18 @@ if compile_prog "-Werror -fno-pie" "-no-pie"; then
>     LDFLAGS_NOPIE="-no-pie"
>   fi
>   
> -if test "$pie" != "no" ; then
> -  if compile_prog "-fPIE -DPIE" "-pie"; then
> -    QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> -    LDFLAGS="-pie $LDFLAGS"
> -    pie="yes"
> -  else
> -    if test "$pie" = "yes"; then
> -      error_exit "PIE not available due to missing toolchain support"
> -    else
> -      echo "Disabling PIE due to missing toolchain support"
> -      pie="no"
> -    fi
> -  fi
> +if test "$pie" = "no"; then
> +  QEMU_CFLAGS="$CFLAGS_NOPIE $QEMU_CFLAGS"
> +  LDFLAGS="$LDFLAGS_NOPIE $LDFLAGS"
> +elif compile_prog "-fPIE -DPIE" "-pie"; then
> +  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> +  LDFLAGS="-pie $LDFLAGS"
> +  pie="yes"
> +elif test "$pie" = "yes"; then
> +  error_exit "PIE not available due to missing toolchain support"
> +else
> +  echo "Disabling PIE due to missing toolchain support"
> +  pie="no"
>   fi
>   
>   # Detect support for DT_BIND_NOW.
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 7/7] configure: Support -static-pie if requested

[Richard Henderson]

Recent toolchains support static and pie at the same time.

As with normal dynamic builds, allow --static to default to PIE
if supported by the toolchain.  Allow --enable/--disable-pie to
override the default.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 1645a58b3a..c03491018a 100755
--- a/configure
+++ b/configure
@@ -1023,7 +1023,6 @@ for opt do
   ;;
   --static)
     static="yes"
-    LDFLAGS="-static $LDFLAGS"
     QEMU_PKG_CONFIG_FLAGS="--static $QEMU_PKG_CONFIG_FLAGS"
   ;;
   --mandir=*) mandir="$optarg"
@@ -1994,11 +1993,6 @@ if test "$static" = "yes" ; then
   if test "$modules" = "yes" ; then
     error_exit "static and modules are mutually incompatible"
   fi
-  if test "$pie" = "yes" ; then
-    error_exit "static and pie are mutually incompatible"
-  else
-    pie="no"
-  fi
 fi
 
 # Unconditional check for compiler __thread support
@@ -2032,6 +2026,17 @@ fi
 if test "$pie" = "no"; then
   QEMU_CFLAGS="$CFLAGS_NOPIE $QEMU_CFLAGS"
   LDFLAGS="$LDFLAGS_NOPIE $LDFLAGS"
+elif test "$static" = "yes"; then
+  if compile_prog "-fPIE -DPIE" "-static-pie"; then
+    QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
+    LDFLAGS="-static-pie $LDFLAGS"
+    pie="yes"
+  elif test "$pie" = "yes"; then
+    error_exit "-static-pie not available due to missing toolchain support"
+  else
+    LDFLAGS="-static $LDFLAGS"
+    pie="no"
+  fi
 elif compile_prog "-fPIE -DPIE" "-pie"; then
   QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
   LDFLAGS="-pie $LDFLAGS"
-- 
2.20.1

Re: [PATCH 7/7] configure: Support -static-pie if requested

[Thomas Huth]

On 18/12/2019 04.19, Richard Henderson wrote:
> Recent toolchains support static and pie at the same time.
> 
> As with normal dynamic builds, allow --static to default to PIE
> if supported by the toolchain.  Allow --enable/--disable-pie to
> override the default.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  configure | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 0/7] configure: Improve PIE and other linkage

[Fangrui Song]

On 2019-12-17, Richard Henderson wrote:
>This begins by dropping the -Ttext-segment stuff, which Fangrui Song
>correctly points out does not work with lld.  But it's also obsolete,
>so instead of adding support for lld's --image-base, remove it all.
>
>Then, remove some other legacy random addresses that were supposed
>to apply to softmmu, but didn't really make any sense, and aren't
>used anyway when PIE is used, which is the default with a modern
>linux distribution.
>
>Then, clean up some of the configure logic surrounding PIE, and its
>current non-application to non-x86.
>
>Finally, add support for static-pie linking.
>
>
>r~
>
>
>Richard Henderson (7):
>  configure: Drop adjustment of textseg
>  tcg: Remove softmmu code_gen_buffer fixed address
>  configure: Do not force pie=no for non-x86
>  configure: Always detect -no-pie toolchain support
>  configure: Unnest detection of -z,relro and -z,now
>  configure: Override the os default with --disable-pie
>  configure: Support -static-pie if requested
>
> accel/tcg/translate-all.c |  37 ++----------
> configure                 | 120 ++++++++++++--------------------------
> 2 files changed, 41 insertions(+), 116 deletions(-)
>
>-- 
>2.20.1

Thank you for the patch set. I hope this will make that lld qemu user
happy.

How will this patch set affect statically linked user mode binaries?
(qemu-user-static packages on Debian, CentOS, ...)

Re: [PATCH 0/7] configure: Improve PIE and other linkage

[Richard Henderson]

On 12/17/19 9:28 PM, Fangrui Song wrote:
> How will this patch set affect statically linked user mode binaries?
> (qemu-user-static packages on Debian, CentOS, ...)

The statically linked user mode binaries will still build, and should still be
able to run any guest binary that could be run before.

If the distribution is new enough to support -static-pie, then that will be
automatically used.

r~