This patch adds a flag to enable the SafeStack instrumentation provided
by LLVM.
The checks make sure that the compiler supports the flags, and that we
are using the proper coroutine implementation (coroutine-ucontext).
While SafeStack is supported only on Linux, NetBSD, FreeBSD and macOS,
we are not checking for the O.S. since this is already done by LLVM.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 configure | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/configure b/configure
index 23b5e93752..f37e4ae0bd 100755
--- a/configure
+++ b/configure
@@ -302,6 +302,7 @@ audio_win_int=""
 libs_qga=""
 debug_info="yes"
 stack_protector=""
+safe_stack="no"
 use_containers="yes"
 gdb_bin=$(command -v "gdb")
 
@@ -1275,6 +1276,8 @@ for opt do
   ;;
   --disable-stack-protector) stack_protector="no"
   ;;
+  --enable-safe-stack) safe_stack="yes"
+  ;;
   --disable-curses) curses="no"
   ;;
   --enable-curses) curses="yes"
@@ -1774,6 +1777,8 @@ Advanced options (experts only):
   --with-coroutine=BACKEND coroutine backend. Supported options:
                            ucontext, sigaltstack, windows
   --enable-gcov            enable test coverage analysis with gcov
+  --enable-safe-stack      enable the SafeStack stack protection. Depends on
+                           clang/llvm >= 3.7 and coroutine backend ucontext.
   --gcov=GCOV              use specified gcov [$gcov_tool]
   --disable-blobs          disable installing provided firmware blobs
   --with-vss-sdk=SDK-path  enable Windows VSS support in QEMU Guest Agent
@@ -5501,6 +5506,29 @@ if test "$debug_stack_usage" = "yes"; then
   fi
 fi
 
+##################################################
+# Check if SafeStack is enabled and supported
+
+if test "$safe_stack" = "yes"; then
+  cat > $TMPC << EOF
+int main(int argc, char *argv[])
+{
+    return 0;
+}
+EOF
+  flag="-fsanitize=safe-stack"
+  # Check that safe-stack is supported.
+  if compile_prog "-Werror $flag" ""; then
+    # Flag needed both at compilation and at linking
+    QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+    QEMU_LDFLAGS="$QEMU_LDFLAGS $flag"
+  else
+    error_exit "SafeStack not supported by your compiler"
+  fi
+  if test "$coroutine" != "ucontext"; then
+    error_exit "SafeStack is only supported by the coroutine backend ucontext"
+  fi
+fi
 
 ##########################################
 # check if we have open_by_handle_at
@@ -6595,6 +6623,7 @@ echo "sparse enabled    $sparse"
 echo "strip binaries    $strip_opt"
 echo "profiler          $profiler"
 echo "static build      $static"
+echo "safe stack        $safe_stack"
 if test "$darwin" = "yes" ; then
     echo "Cocoa support     $cocoa"
 fi
-- 
2.26.2

On Wed, Apr 29, 2020 at 03:44:19PM -0400, Daniele Buono wrote:
> This patch adds a flag to enable the SafeStack instrumentation provided
> by LLVM.
> The checks make sure that the compiler supports the flags, and that we
> are using the proper coroutine implementation (coroutine-ucontext).
> While SafeStack is supported only on Linux, NetBSD, FreeBSD and macOS,
> we are not checking for the O.S. since this is already done by LLVM.
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> ---
>  configure | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)

Great, this can become Patch 1 and it can set CONFIG_SAFESTACK as
mentioned in my earlier reply.

> diff --git a/configure b/configure
> index 23b5e93752..f37e4ae0bd 100755
> --- a/configure
> +++ b/configure
> @@ -302,6 +302,7 @@ audio_win_int=""
>  libs_qga=""
>  debug_info="yes"
>  stack_protector=""
> +safe_stack="no"

The comment above this says:

  # Always add --enable-foo and --disable-foo command line args.

Please add --disable-safe-stack.

Hi Stefan,

I would feel more confident by adding another check in configure to make
sure that the user didn't enable SafeStack manually through other means,
like manually setting the option through extra_cflags.
What do you think?

On 5/21/2020 5:52 AM, Stefan Hajnoczi wrote:
> On Wed, Apr 29, 2020 at 03:44:19PM -0400, Daniele Buono wrote:
>> This patch adds a flag to enable the SafeStack instrumentation provided
>> by LLVM.
>> The checks make sure that the compiler supports the flags, and that we
>> are using the proper coroutine implementation (coroutine-ucontext).
>> While SafeStack is supported only on Linux, NetBSD, FreeBSD and macOS,
>> we are not checking for the O.S. since this is already done by LLVM.
>>
>> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
>> ---
>>   configure | 29 +++++++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
> 
> Great, this can become Patch 1 and it can set CONFIG_SAFESTACK as
> mentioned in my earlier reply.
> 
>> diff --git a/configure b/configure
>> index 23b5e93752..f37e4ae0bd 100755
>> --- a/configure
>> +++ b/configure
>> @@ -302,6 +302,7 @@ audio_win_int=""
>>   libs_qga=""
>>   debug_info="yes"
>>   stack_protector=""
>> +safe_stack="no"
> 
> The comment above this says:
> 
>    # Always add --enable-foo and --disable-foo command line args.
> 
> Please add --disable-safe-stack.
> 

On Fri, May 22, 2020 at 11:24:46AM -0400, Daniele Buono wrote:
> I would feel more confident by adding another check in configure to make
> sure that the user didn't enable SafeStack manually through other means,
> like manually setting the option through extra_cflags.
> What do you think?

Sure, a compile_prog call could check if SafeStack is enable when it
shouldn't be.

This can be done together with a --disable option.

Stefan

Hi Stefan, a quick clarification on configure:

right now, in configure, there's
* "Advanced Options (experts only)"
which usually don't have both enable and disable for each option, and
* "Optional features, enabled with --enable-FEATURE and
disabled with --disable-FEATURE, default is enabled if available:"

How do you think SafeStack should be classified?
* If we do it as Advanced option, we should probably force it disabled
unless --enable-safe-stack is provided. In this case
--disable-safe-stack is not really necessary.
* If we do it as optional feature, I have two ways to handle the default:
1. enable/disable based on the compilation flags given to configure
2. enable every time the provided compiler supports it

On 5/27/2020 7:12 AM, Stefan Hajnoczi wrote:
> On Fri, May 22, 2020 at 11:24:46AM -0400, Daniele Buono wrote:
>> I would feel more confident by adding another check in configure to make
>> sure that the user didn't enable SafeStack manually through other means,
>> like manually setting the option through extra_cflags.
>> What do you think?
> 
> Sure, a compile_prog call could check if SafeStack is enable when it
> shouldn't be.
> 
> This can be done together with a --disable option.
> 
> Stefan
>