| Message ID | 1321281701-4192-1-git-send-email-avi@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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)
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).
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
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.
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
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.
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
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(-)