[Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Avi Kivity]

Change the default to building PIE (position independent executables); instead
of restricting the option to user-only targets, apply it to all targets.

While PIE reduces performance and increases load time, it greatly improves
security, with the potential to reduce a code execution vulnerability to a
self denial of service.

Signed-off-by: Avi Kivity <avi@redhat.com>
---

While we are past the feature freeze, I feel this deserves an exception.  I'd
much rather see "CVE-2012-wxyz QEMU Self denial of service" than
"CVE-2012-wxyz QEMU code execution".  The fact that the option is available
for user targets implies that it is compatible with TCG, and some light testing
agrees.

 configure |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index 6c77fbb..7436361 100755
--- a/configure
+++ b/configure
@@ -172,7 +172,7 @@ aix="no"
 blobs="yes"
 pkgversion=""
 check_utests=""
-user_pie="no"
+pie="yes"
 zero_malloc=""
 trace_backend="nop"
 trace_file="trace"
@@ -701,9 +701,9 @@ for opt do
   ;;
   --disable-guest-base) guest_base="no"
   ;;
-  --enable-user-pie) user_pie="yes"
+  --enable-pie) pie="yes"
   ;;
-  --disable-user-pie) user_pie="no"
+  --disable-pie) pie="no"
   ;;
   --enable-uname-release=*) uname_release="$optarg"
   ;;
@@ -1031,8 +1031,8 @@ echo "  --disable-bsd-user       disable all BSD usermode emulation targets"
 echo "  --enable-guest-base      enable GUEST_BASE support for usermode"
 echo "                           emulation targets"
 echo "  --disable-guest-base     disable GUEST_BASE support"
-echo "  --enable-user-pie        build usermode emulation targets as PIE"
-echo "  --disable-user-pie       do not build usermode emulation targets as PIE"
+echo "  --enable-pie             build Position Independent Executables"
+echo "  --disable-pie            do not build Position Independent Executables"
 echo "  --fmod-lib               path to FMOD library"
 echo "  --fmod-inc               path to FMOD includes"
 echo "  --oss-lib                path to OSS library"
@@ -1099,6 +1099,17 @@ for flag in $gcc_flags; do
     fi
 done
 
+if test "$pie" = "yes" ; then
+  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
+  LDFLAGS="-Wl,-pie $LDFLAGS"
+  cat > $TMPC << EOF
+int main(void) { return 0; }
+EOF
+  if compile_prog "-fPIE -DPIE" "-Wl,-pie -Wl,-z,relro -Wl,-z,now"; then
+    LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
+  fi
+fi
+
 #
 # Solaris specific configure tool chain decisions
 #
@@ -2765,7 +2776,7 @@ echo "Documentation     $docs"
 echo "uname -r          $uname_release"
 echo "NPTL support      $nptl"
 echo "GUEST_BASE        $guest_base"
-echo "PIE user targets  $user_pie"
+echo "PIE               $pie"
 echo "vde support       $vde"
 echo "Linux AIO support $linux_aio"
 echo "ATTR/XATTR support $attr"
@@ -3225,9 +3236,6 @@ for d in libdis libdis-user; do
     symlink $source_path/Makefile.dis $d/Makefile
     echo > $d/config.mak
 done
-if test "$static" = "no" -a "$user_pie" = "yes" ; then
-  echo "QEMU_CFLAGS+=-fpie" > libdis-user/config.mak
-fi
 
 for target in $target_list; do
 target_dir="$target"
@@ -3646,12 +3654,6 @@ if test "$target_softmmu" = "yes" ; then
   esac
 fi
 
-if test "$target_user_only" = "yes" -a "$static" = "no" -a \
-	"$user_pie" = "yes" ; then
-  cflags="-fpie $cflags"
-  ldflags="-pie $ldflags"
-fi
-
 if test "$target_softmmu" = "yes" -a \( \
         "$TARGET_ARCH" = "microblaze" -o \
         "$TARGET_ARCH" = "cris" \) ; then
@@ -3775,9 +3777,6 @@ d=libuser
 mkdir -p $d
 mkdir -p $d/trace
 symlink $source_path/Makefile.user $d/Makefile
-if test "$static" = "no" -a "$user_pie" = "yes" ; then
-  echo "QEMU_CFLAGS+=-fpie" > $d/config.mak
-fi
 
 if test "$docs" = "yes" ; then
   mkdir -p QMP
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Peter Maydell]

On 14 November 2011 14:41, Avi Kivity <avi@redhat.com> wrote:
> +if test "$pie" = "yes" ; then
> +  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> +  LDFLAGS="-Wl,-pie $LDFLAGS"
> +  cat > $TMPC << EOF
> +int main(void) { return 0; }
> +EOF
> +  if compile_prog "-fPIE -DPIE" "-Wl,-pie -Wl,-z,relro -Wl,-z,now"; then
> +    LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
> +  fi
> +fi

It would be good to see some testing that this works on all
our random oddball hosts (MacOSX, for instance, which doesn't
use GNU ld) and architectures we apply this...

-- PMM

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Peter Maydell]

On 14 November 2011 14:59, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 14 November 2011 14:41, Avi Kivity <avi@redhat.com> wrote:
>> +if test "$pie" = "yes" ; then
>> +  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
>> +  LDFLAGS="-Wl,-pie $LDFLAGS"

> It would be good to see some testing that this works on all
> our random oddball hosts (MacOSX, for instance, which doesn't
> use GNU ld) and architectures we apply this...

*before we apply this...

-- PMM (rats)

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Avi Kivity]

On 11/14/2011 05:00 PM, Peter Maydell wrote:
> On 14 November 2011 14:59, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 14 November 2011 14:41, Avi Kivity <avi@redhat.com> wrote:
> >> +if test "$pie" = "yes" ; then
> >> +  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> >> +  LDFLAGS="-Wl,-pie $LDFLAGS"
>
> > It would be good to see some testing that this works on all
> > our random oddball hosts (MacOSX, for instance, which doesn't
> > use GNU ld) and architectures we apply this...
>
> *before we apply this...

We're unlikely to see testing before we apply the patch, and for oddball
archs, even afterwards.  What we can do the qualify it on a build test
(and assume that if it builds, it runs, which I think is a safe assumption).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Peter Maydell]

On 14 November 2011 15:06, Avi Kivity <avi@redhat.com> wrote:
> On 11/14/2011 05:00 PM, Peter Maydell wrote:
>> On 14 November 2011 14:59, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > It would be good to see some testing that this works on all
>> > our random oddball hosts (MacOSX, for instance, which doesn't
>> > use GNU ld) and architectures we apply this...
>>
>> *before we apply this...
>
> We're unlikely to see testing before we apply the patch, and for oddball
> archs, even afterwards.

Yes, but if you put this change in just before release you
get much less testing than if you do it with several months
before release. This kind of change makes me nervous.

> What we can do the qualify it on a build test
> (and assume that if it builds, it runs, which I think is a safe assumption).

One of the failure cases I was thinking of is that if PIE means
the platform's loader puts things in a different bit of the
address space this might break TCG's assumptions about maximum
distances between the codegen buffer and host C code. (That's
a bug in TCG really but it would still be near-to-release
breakage.)

But yes, a build test would be a good start. (My money's
on it failing to build on MacOSX.)

PS: what's the -DPIE needed for?

-- PMM

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Avi Kivity]

On 11/14/2011 05:15 PM, Peter Maydell wrote:
> >
> > We're unlikely to see testing before we apply the patch, and for oddball
> > archs, even afterwards.
>
> Yes, but if you put this change in just before release you
> get much less testing than if you do it with several months
> before release. This kind of change makes me nervous.
>
> > What we can do the qualify it on a build test
> > (and assume that if it builds, it runs, which I think is a safe assumption).
>
> One of the failure cases I was thinking of is that if PIE means
> the platform's loader puts things in a different bit of the
> address space this might break TCG's assumptions about maximum
> distances between the codegen buffer and host C code. (That's
> a bug in TCG really but it would still be near-to-release
> breakage.)

Is this assumption tested at runtime?  If so, we can have the failure
message mumble something about building with --disable-pie.

> But yes, a build test would be a good start. (My money's
> on it failing to build on MacOSX.)

I'll post a v3 with an additional test.

> PS: what's the -DPIE needed for?

Copied from cookbook.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Peter Maydell]

On 14 November 2011 15:18, Avi Kivity <avi@redhat.com> wrote:
> On 11/14/2011 05:15 PM, Peter Maydell wrote:
>> One of the failure cases I was thinking of is that if PIE means
>> the platform's loader puts things in a different bit of the
>> address space this might break TCG's assumptions about maximum
>> distances between the codegen buffer and host C code. (That's
>> a bug in TCG really but it would still be near-to-release
>> breakage.)

And indeed testing this on an ARM host running i386 TCG system
mode, applying this patch causes qemu to fail at startup with
a tcg abort due to an out of range jump.

I've already said that ARM is going to be broken for 1.0 so
that's not inherently a problem but it does indicate that
we definitely need to test the other TCG target systems
(and not just a "does it compile" test) if we want to put
this change in.

-- PMM

Re: [Qemu-devel] [PATCH 1.0] configure: build position independent executables across the board, by default

[Avi Kivity]

On 11/14/2011 08:45 PM, Peter Maydell wrote:
> On 14 November 2011 15:18, Avi Kivity <avi@redhat.com> wrote:
> > On 11/14/2011 05:15 PM, Peter Maydell wrote:
> >> One of the failure cases I was thinking of is that if PIE means
> >> the platform's loader puts things in a different bit of the
> >> address space this might break TCG's assumptions about maximum
> >> distances between the codegen buffer and host C code. (That's
> >> a bug in TCG really but it would still be near-to-release
> >> breakage.)
>
> And indeed testing this on an ARM host running i386 TCG system
> mode, applying this patch causes qemu to fail at startup with
> a tcg abort due to an out of range jump.
>
> I've already said that ARM is going to be broken for 1.0 so
> that's not inherently a problem but it does indicate that
> we definitely need to test the other TCG target systems
> (and not just a "does it compile" test) if we want to put
> this change in.

I'll change the default to 'yes' for x86 hosts and 'no' for anything
else.  tcg host maintainers can then update after testing/modifications.

-- 
error compiling committee.c: too many arguments to function