All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Add support for Control-Flow Integrity
@ 2020-12-04 23:06 Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 1/5] configure,meson: add option to enable LTO Daniele Buono
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Daniele Buono @ 2020-12-04 23:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniele Buono

This patch adds supports for Control-Flow Integrity checks
on indirect function calls.

Requires the use of clang, and link-time optimizations

Since it's been a month, and some of the patches are being
merged independently, I thought of rebasing, retesting
and sending an updated version. Also, added a documentation
in docs/devel to explain CFI and how to handle CFI-sensitive
code.

Changes in v4:
- Removed patches to avoid clang warnings, since they are
being merged independently and are not really necessary
for CFI
- Added documentation in docs/devel to explain how to
compile with CFI, and how to disable CFI for incompatible
functions

Changes in v3:

- clang 11+ warnings are now handled directly at the source,
instead of disabling specific warnings for the whole code.
Some more work may be needed here to polish the patch, I
would kindly ask for a review from the corresponding
maintainers
- Remove configure-time checks for toolchain compatibility
with LTO.
- the decorator to disable cfi checks on functions has
been renamed and moved to include/qemu/compiler.h
- configure-time checks for cfi support and dependencies
has been moved from configure to meson

Link to v3: https://www.mail-archive.com/qemu-devel@nongnu.org/msg757930.html
Link to v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg753675.html
Link to v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg718786.html

Daniele Buono (5):
  configure,meson: add option to enable LTO
  cfi: Initial support for cfi-icall in QEMU
  check-block: enable iotests with cfi-icall
  configure,meson: support Control-Flow Integrity
  docs: Add CFI Documentation

 accel/tcg/cpu-exec.c                  |  11 +++
 configure                             |  26 +++++
 docs/devel/control-flow-integrity.rst | 137 ++++++++++++++++++++++++++
 include/qemu/compiler.h               |  12 +++
 meson.build                           |  46 +++++++++
 meson_options.txt                     |   4 +
 plugins/core.c                        |  37 +++++++
 plugins/loader.c                      |   7 ++
 tcg/tci.c                             |   7 ++
 tests/check-block.sh                  |  18 ++--
 util/main-loop.c                      |  11 +++
 util/oslib-posix.c                    |  11 +++
 12 files changed, 320 insertions(+), 7 deletions(-)
 create mode 100644 docs/devel/control-flow-integrity.rst

-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/5] configure,meson: add option to enable LTO
  2020-12-04 23:06 [PATCH v4 0/5] Add support for Control-Flow Integrity Daniele Buono
@ 2020-12-04 23:06 ` Daniele Buono
  2021-07-11 10:22   ` Thomas Huth
  2020-12-04 23:06 ` [PATCH v4 2/5] cfi: Initial support for cfi-icall in QEMU Daniele Buono
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Daniele Buono @ 2020-12-04 23:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniele Buono

This patch allows to compile QEMU with link-time optimization (LTO).
Compilation with LTO is handled directly by meson. This patch only
adds the option in configure and forwards the request to meson

Tested with all major versions of clang from 6 to 12

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 configure   | 7 +++++++
 meson.build | 1 +
 2 files changed, 8 insertions(+)

diff --git a/configure b/configure
index 18c26e0389..fee118518b 100755
--- a/configure
+++ b/configure
@@ -242,6 +242,7 @@ host_cc="cc"
 audio_win_int=""
 libs_qga=""
 debug_info="yes"
+lto="false"
 stack_protector=""
 safe_stack=""
 use_containers="yes"
@@ -1167,6 +1168,10 @@ for opt do
   ;;
   --disable-werror) werror="no"
   ;;
+  --enable-lto) lto="true"
+  ;;
+  --disable-lto) lto="false"
+  ;;
   --enable-stack-protector) stack_protector="yes"
   ;;
   --disable-stack-protector) stack_protector="no"
@@ -1751,6 +1756,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   module-upgrades try to load modules from alternate paths for upgrades
   debug-tcg       TCG debugging (default is disabled)
   debug-info      debugging information
+  lto             Enable Link-Time Optimization.
   sparse          sparse checker
   safe-stack      SafeStack Stack Smash Protection. Depends on
                   clang/llvm >= 3.7 and requires coroutine backend ucontext.
@@ -7014,6 +7020,7 @@ NINJA=$ninja $meson setup \
         -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
         -Dvhost_user_blk_server=$vhost_user_blk_server \
+        -Db_lto=$lto \
         $cross_arg \
         "$PWD" "$source_path"
 
diff --git a/meson.build b/meson.build
index e3386196ba..ebd1c690e0 100644
--- a/meson.build
+++ b/meson.build
@@ -2044,6 +2044,7 @@ summary_info += {'gprof enabled':     config_host.has_key('CONFIG_GPROF')}
 summary_info += {'sparse enabled':    sparse.found()}
 summary_info += {'strip binaries':    get_option('strip')}
 summary_info += {'profiler':          config_host.has_key('CONFIG_PROFILER')}
+summary_info += {'link-time optimization (LTO)': get_option('b_lto')}
 summary_info += {'static build':      config_host.has_key('CONFIG_STATIC')}
 if targetos == 'darwin'
   summary_info += {'Cocoa support': config_host.has_key('CONFIG_COCOA')}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 2/5] cfi: Initial support for cfi-icall in QEMU
  2020-12-04 23:06 [PATCH v4 0/5] Add support for Control-Flow Integrity Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 1/5] configure,meson: add option to enable LTO Daniele Buono
@ 2020-12-04 23:06 ` Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 3/5] check-block: enable iotests with cfi-icall Daniele Buono
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Daniele Buono @ 2020-12-04 23:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, Richard Henderson,
	Daniele Buono, Stefan Weil

LLVM/Clang, supports runtime checks for forward-edge Control-Flow
Integrity (CFI).

CFI on indirect function calls (cfi-icall) ensures that, in indirect
function calls, the function called is of the right signature for the
pointer type defined at compile time.

For this check to work, the code must always respect the function
signature when using function pointer, the function must be defined
at compile time, and be compiled with link-time optimization.

This rules out, for example, shared libraries that are dynamically loaded
(given that functions are not known at compile time), and code that is
dynamically generated at run-time.

This patch:

1) Introduces the CONFIG_CFI flag to support cfi in QEMU

2) Introduces a decorator to allow the definition of "sensitive"
functions, where a non-instrumented function may be called at runtime
through a pointer. The decorator will take care of disabling cfi-icall
checks on such functions, when cfi is enabled.

3) Marks functions currently in QEMU that exhibit such behavior,
in particular:
- The function in TCG that calls pre-compiled TBs
- The function in TCI that interprets instructions
- Functions in the plugin infrastructures that jump to callbacks
- Functions in util that directly call a signal handler

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org
---
 accel/tcg/cpu-exec.c    | 11 +++++++++++
 include/qemu/compiler.h | 12 ++++++++++++
 plugins/core.c          | 37 +++++++++++++++++++++++++++++++++++++
 plugins/loader.c        |  7 +++++++
 tcg/tci.c               |  7 +++++++
 util/main-loop.c        | 11 +++++++++++
 util/oslib-posix.c      | 11 +++++++++++
 7 files changed, 96 insertions(+)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 58aea605d8..ffe0e1e3fb 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -26,6 +26,7 @@
 #include "exec/exec-all.h"
 #include "tcg/tcg.h"
 #include "qemu/atomic.h"
+#include "qemu/compiler.h"
 #include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "qemu/rcu.h"
@@ -144,6 +145,16 @@ static void init_delay_params(SyncClocks *sc, const CPUState *cpu)
 #endif /* CONFIG USER ONLY */
 
 /* Execute a TB, and fix up the CPU state afterwards if necessary */
+/*
+ * Disable CFI checks.
+ * TCG creates binary blobs at runtime, with the transformed code.
+ * A TB is a blob of binary code, created at runtime and called with an
+ * indirect function call. Since such function did not exist at compile time,
+ * the CFI runtime has no way to verify its signature and would fail.
+ * TCG is not considered a security-sensitive part of QEMU so this does not
+ * affect the impact of CFI in environment with high security requirements
+ */
+QEMU_DISABLE_CFI
 static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
 {
     CPUArchState *env = cpu->env_ptr;
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index c76281f354..c87c242063 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -243,4 +243,16 @@ extern void QEMU_NORETURN QEMU_ERROR("code path is reachable")
 #define qemu_build_not_reached()  g_assert_not_reached()
 #endif
 
+#ifdef CONFIG_CFI
+/*
+ * If CFI is enabled, use an attribute to disable cfi-icall on the following
+ * function
+ */
+#define QEMU_DISABLE_CFI __attribute__((no_sanitize("cfi-icall")))
+#else
+/* If CFI is not enabled, use an empty define to not change the behavior */
+#define QEMU_DISABLE_CFI
+#endif
+
+
 #endif /* COMPILER_H */
diff --git a/plugins/core.c b/plugins/core.c
index 51bfc94787..87b823bbc4 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -31,6 +31,7 @@
 #include "tcg/tcg-op.h"
 #include "trace/mem-internal.h" /* mem_info macros */
 #include "plugin.h"
+#include "qemu/compiler.h"
 
 struct qemu_plugin_cb {
     struct qemu_plugin_ctx *ctx;
@@ -90,6 +91,12 @@ void plugin_unregister_cb__locked(struct qemu_plugin_ctx *ctx,
     }
 }
 
+/*
+ * Disable CFI checks.
+ * The callback function has been loaded from an external library so we do not
+ * have type information
+ */
+QEMU_DISABLE_CFI
 static void plugin_vcpu_cb__simple(CPUState *cpu, enum qemu_plugin_event ev)
 {
     struct qemu_plugin_cb *cb, *next;
@@ -111,6 +118,12 @@ static void plugin_vcpu_cb__simple(CPUState *cpu, enum qemu_plugin_event ev)
     }
 }
 
+/*
+ * Disable CFI checks.
+ * The callback function has been loaded from an external library so we do not
+ * have type information
+ */
+QEMU_DISABLE_CFI
 static void plugin_cb__simple(enum qemu_plugin_event ev)
 {
     struct qemu_plugin_cb *cb, *next;
@@ -128,6 +141,12 @@ static void plugin_cb__simple(enum qemu_plugin_event ev)
     }
 }
 
+/*
+ * Disable CFI checks.
+ * The callback function has been loaded from an external library so we do not
+ * have type information
+ */
+QEMU_DISABLE_CFI
 static void plugin_cb__udata(enum qemu_plugin_event ev)
 {
     struct qemu_plugin_cb *cb, *next;
@@ -325,6 +344,12 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
     dyn_cb->f.generic = cb;
 }
 
+/*
+ * Disable CFI checks.
+ * The callback function has been loaded from an external library so we do not
+ * have type information
+ */
+QEMU_DISABLE_CFI
 void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
 {
     struct qemu_plugin_cb *cb, *next;
@@ -339,6 +364,12 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
     }
 }
 
+/*
+ * Disable CFI checks.
+ * The callback function has been loaded from an external library so we do not
+ * have type information
+ */
+QEMU_DISABLE_CFI
 void
 qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
                          uint64_t a3, uint64_t a4, uint64_t a5,
@@ -358,6 +389,12 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
     }
 }
 
+/*
+ * Disable CFI checks.
+ * The callback function has been loaded from an external library so we do not
+ * have type information
+ */
+QEMU_DISABLE_CFI
 void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
 {
     struct qemu_plugin_cb *cb, *next;
diff --git a/plugins/loader.c b/plugins/loader.c
index 8ac5dbc20f..fd491961de 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -32,6 +32,7 @@
 #ifndef CONFIG_USER_ONLY
 #include "hw/boards.h"
 #endif
+#include "qemu/compiler.h"
 
 #include "plugin.h"
 
@@ -150,6 +151,12 @@ static uint64_t xorshift64star(uint64_t x)
     return x * UINT64_C(2685821657736338717);
 }
 
+/*
+ * Disable CFI checks.
+ * The install and version functions have been loaded from an external library
+ * so we do not have type information
+ */
+QEMU_DISABLE_CFI
 static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info)
 {
     qemu_plugin_install_func_t install;
diff --git a/tcg/tci.c b/tcg/tci.c
index 82039fd163..5d97b7c71c 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -31,6 +31,7 @@
 #include "tcg/tcg.h"           /* MAX_OPC_PARAM_IARGS */
 #include "exec/cpu_ldst.h"
 #include "tcg/tcg-op.h"
+#include "qemu/compiler.h"
 
 /* Marker for missing code. */
 #define TODO() \
@@ -475,6 +476,12 @@ static bool tci_compare64(uint64_t u0, uint64_t u1, TCGCond condition)
 #endif
 
 /* Interpret pseudo code in tb. */
+/*
+ * Disable CFI checks.
+ * One possible operation in the pseudo code is a call to binary code.
+ * Therefore, disable CFI checks in the interpreter function
+ */
+QEMU_DISABLE_CFI
 uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
 {
     tcg_target_ulong regs[TCG_TARGET_NB_REGS];
diff --git a/util/main-loop.c b/util/main-loop.c
index 6470f8eae3..6bfc7c46f5 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -33,6 +33,7 @@
 #include "block/aio.h"
 #include "qemu/error-report.h"
 #include "qemu/queue.h"
+#include "qemu/compiler.h"
 
 #ifndef _WIN32
 #include <sys/wait.h>
@@ -44,6 +45,16 @@
  * use signalfd to listen for them.  We rely on whatever the current signal
  * handler is to dispatch the signals when we receive them.
  */
+/*
+ * Disable CFI checks.
+ * We are going to call a signal hander directly. Such handler may or may not
+ * have been defined in our binary, so there's no guarantee that the pointer
+ * used to set the handler is a cfi-valid pointer. Since the handlers are
+ * stored in kernel memory, changing the handler to an attacker-defined
+ * function requires being able to call a sigaction() syscall,
+ * which is not as easy as overwriting a pointer in memory.
+ */
+QEMU_DISABLE_CFI
 static void sigfd_handler(void *opaque)
 {
     int fd = (intptr_t)opaque;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f15234b5c0..f1e2801b11 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -39,6 +39,7 @@
 #include "qemu/thread.h"
 #include <libgen.h>
 #include "qemu/cutils.h"
+#include "qemu/compiler.h"
 
 #ifdef CONFIG_LINUX
 #include <sys/syscall.h>
@@ -773,6 +774,16 @@ void qemu_free_stack(void *stack, size_t sz)
     munmap(stack, sz);
 }
 
+/*
+ * Disable CFI checks.
+ * We are going to call a signal hander directly. Such handler may or may not
+ * have been defined in our binary, so there's no guarantee that the pointer
+ * used to set the handler is a cfi-valid pointer. Since the handlers are
+ * stored in kernel memory, changing the handler to an attacker-defined
+ * function requires being able to call a sigaction() syscall,
+ * which is not as easy as overwriting a pointer in memory.
+ */
+QEMU_DISABLE_CFI
 void sigaction_invoke(struct sigaction *action,
                       struct qemu_signalfd_siginfo *info)
 {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 3/5] check-block: enable iotests with cfi-icall
  2020-12-04 23:06 [PATCH v4 0/5] Add support for Control-Flow Integrity Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 1/5] configure,meson: add option to enable LTO Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 2/5] cfi: Initial support for cfi-icall in QEMU Daniele Buono
@ 2020-12-04 23:06 ` Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 4/5] configure,meson: support Control-Flow Integrity Daniele Buono
  2020-12-04 23:06 ` [PATCH v4 5/5] docs: Add CFI Documentation Daniele Buono
  4 siblings, 0 replies; 13+ messages in thread
From: Daniele Buono @ 2020-12-04 23:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniele Buono

cfi-icall is a form of Control-Flow Integrity for indirect function
calls implemented by llvm. It is enabled with a -fsanitize flag.

iotests are currently disabled when -fsanitize options is used, with the
exception of SafeStack.

This patch implements a generic filtering mechanism to allow iotests
with a set of known-to-be-safe -fsanitize option. Then marks SafeStack
and the new options used for cfi-icall safe for iotests

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 tests/check-block.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index f6b1bda7b9..fb4c1baae9 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -21,14 +21,18 @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
     exit 0
 fi
 
-# Disable tests with any sanitizer except for SafeStack
-CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
-SANITIZE_FLAGS=""
-#Remove all occurrencies of -fsanitize=safe-stack
-for i in ${CFLAGS}; do
-        if [ "${i}" != "-fsanitize=safe-stack" ]; then
-                SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
+# Disable tests with any sanitizer except for specific ones
+SANITIZE_FLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
+ALLOWED_SANITIZE_FLAGS="safe-stack cfi-icall"
+#Remove all occurrencies of allowed Sanitize flags
+for j in ${ALLOWED_SANITIZE_FLAGS}; do
+    TMP_FLAGS=${SANITIZE_FLAGS}
+    SANITIZE_FLAGS=""
+    for i in ${TMP_FLAGS}; do
+        if ! echo ${i} | grep -q "${j}" 2>/dev/null; then
+            SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
         fi
+    done
 done
 if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
     # Have a sanitize flag that is not allowed, stop
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 4/5] configure,meson: support Control-Flow Integrity
  2020-12-04 23:06 [PATCH v4 0/5] Add support for Control-Flow Integrity Daniele Buono
                   ` (2 preceding siblings ...)
  2020-12-04 23:06 ` [PATCH v4 3/5] check-block: enable iotests with cfi-icall Daniele Buono
@ 2020-12-04 23:06 ` Daniele Buono
  2020-12-13  2:55   ` Alexander Bulekov
  2020-12-14 11:22   ` Paolo Bonzini
  2020-12-04 23:06 ` [PATCH v4 5/5] docs: Add CFI Documentation Daniele Buono
  4 siblings, 2 replies; 13+ messages in thread
From: Daniele Buono @ 2020-12-04 23:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniele Buono

This patch adds a flag to enable/disable control flow integrity checks
on indirect function calls.
This feature only allows indirect function calls at runtime to functions
with compatible signatures.

This feature is only provided by LLVM/Clang, and depends on link-time
optimization which is currently supported only with LLVM/Clang >= 6.0

We also add an option to enable a debugging version of cfi, with verbose
output in case of a CFI violation.

CFI on indirect function calls does not support calls to functions in
shared libraries (since they were not known at compile time), and such
calls are forbidden. QEMU relies on dlopen/dlsym when using modules,
so we make modules incompatible with CFI.

All the checks are performed in meson.build. configure is only used to
forward the flags to meson

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 configure         | 21 ++++++++++++++++++++-
 meson.build       | 45 +++++++++++++++++++++++++++++++++++++++++++++
 meson_options.txt |  4 ++++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index fee118518b..c4e5d92167 100755
--- a/configure
+++ b/configure
@@ -400,6 +400,8 @@ coroutine=""
 coroutine_pool=""
 debug_stack_usage="no"
 crypto_afalg="no"
+cfi="disabled"
+cfi_debug="disabled"
 seccomp=""
 glusterfs=""
 glusterfs_xlator_opt="no"
@@ -1180,6 +1182,16 @@ for opt do
   ;;
   --disable-safe-stack) safe_stack="no"
   ;;
+  --enable-cfi)
+      cfi="enabled";
+      lto="true";
+  ;;
+  --disable-cfi) cfi="disabled"
+  ;;
+  --enable-cfi-debug) cfi_debug="enabled"
+  ;;
+  --disable-cfi-debug) cfi_debug="disabled"
+  ;;
   --disable-curses) curses="disabled"
   ;;
   --enable-curses) curses="enabled"
@@ -1760,6 +1772,13 @@ disabled with --disable-FEATURE, default is enabled if available:
   sparse          sparse checker
   safe-stack      SafeStack Stack Smash Protection. Depends on
                   clang/llvm >= 3.7 and requires coroutine backend ucontext.
+  cfi             Enable Control-Flow Integrity for indirect function calls.
+                  In case of a cfi violation, QEMU is terminated with SIGILL
+                  Depends on lto and is incompatible with modules
+                  Automatically enables Link-Time Optimization (lto)
+  cfi-debug       In case of a cfi violation, a message containing the line that
+                  triggered the error is written to stderr. After the error,
+                  QEMU is still terminated with SIGILL
 
   gnutls          GNUTLS cryptography support
   nettle          nettle cryptography support
@@ -7020,7 +7039,7 @@ NINJA=$ninja $meson setup \
         -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
         -Dvhost_user_blk_server=$vhost_user_blk_server \
-        -Db_lto=$lto \
+        -Db_lto=$lto -Dcfi=$cfi -Dcfi_debug=$cfi_debug \
         $cross_arg \
         "$PWD" "$source_path"
 
diff --git a/meson.build b/meson.build
index ebd1c690e0..e1ae6521e0 100644
--- a/meson.build
+++ b/meson.build
@@ -773,6 +773,48 @@ elif get_option('vhost_user_blk_server').disabled() or not have_system
     have_vhost_user_blk_server = false
 endif
 
+if get_option('cfi').enabled()
+  cfi_flags=[]
+  # Check for dependency on LTO
+  if not get_option('b_lto')
+    error('Selected Control-Flow Integrity but LTO is disabled')
+  endif
+  if config_host.has_key('CONFIG_MODULES')
+    error('Selected Control-Flow Integrity is not compatible with modules')
+  endif
+  # Check for cfi flags. CFI requires LTO so we can't use
+  # get_supported_arguments, but need a more complex "compiles" which allows
+  # custom arguments
+  if cc.compiles('int main () { return 0; }', name: '-fsanitize=cfi-icall',
+                 args: ['-flto', '-fsanitize=cfi-icall'] )
+    cfi_flags += '-fsanitize=cfi-icall'
+  else
+    error('-fsanitize=cfi-icall is not supported by the compiler')
+  endif
+  if cc.compiles('int main () { return 0; }',
+                 name: '-fsanitize-cfi-icall-generalize-pointers',
+                 args: ['-flto', '-fsanitize=cfi-icall',
+                        '-fsanitize-cfi-icall-generalize-pointers'] )
+    cfi_flags += '-fsanitize-cfi-icall-generalize-pointers'
+  else
+    error('-fsanitize-cfi-icall-generalize-pointers is not supported by the compiler')
+  endif
+  if get_option('cfi_debug').enabled()
+    if cc.compiles('int main () { return 0; }',
+                   name: '-fno-sanitize-trap=cfi-icall',
+                   args: ['-flto', '-fsanitize=cfi-icall',
+                          '-fno-sanitize-trap=cfi-icall'] )
+      cfi_flags += '-fno-sanitize-trap=cfi-icall'
+    else
+      error('-fno-sanitize-trap=cfi-icall is not supported by the compiler')
+    endif
+  endif
+  add_project_arguments(cfi_flags, native: false, language: ['c', 'cpp',
+                                                             'objc'])
+  add_project_link_arguments(cfi_flags, native: false, language: ['c', 'cpp',
+                                                                  'objc'])
+endif
+
 #################
 # config-host.h #
 #################
@@ -807,6 +849,7 @@ config_host_data.set('CONFIG_KEYUTILS', keyutils.found())
 config_host_data.set('CONFIG_GETTID', has_gettid)
 config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
 config_host_data.set('CONFIG_STATX', has_statx)
+config_host_data.set('CONFIG_CFI', get_option('cfi').enabled())
 config_host_data.set('QEMU_VERSION', '"@0@"'.format(meson.project_version()))
 config_host_data.set('QEMU_VERSION_MAJOR', meson.project_version().split('.')[0])
 config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1])
@@ -2159,6 +2202,8 @@ if targetos == 'windows'
   summary_info += {'QGA MSI support':   config_host.has_key('CONFIG_QGA_MSI')}
 endif
 summary_info += {'seccomp support':   config_host.has_key('CONFIG_SECCOMP')}
+summary_info += {'cfi support':       get_option('cfi').enabled()}
+summary_info += {'cfi debug support': get_option('cfi_debug').enabled()}
 summary_info += {'coroutine backend': config_host['CONFIG_COROUTINE_BACKEND']}
 summary_info += {'coroutine pool':    config_host['CONFIG_COROUTINE_POOL'] == '1'}
 summary_info += {'debug stack usage': config_host.has_key('CONFIG_DEBUG_STACK_USAGE')}
diff --git a/meson_options.txt b/meson_options.txt
index f6f64785fe..8d5729e450 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -35,6 +35,10 @@ option('xen_pci_passthrough', type: 'feature', value: 'auto',
        description: 'Xen PCI passthrough support')
 option('tcg', type: 'feature', value: 'auto',
        description: 'TCG support')
+option('cfi', type: 'feature', value: 'auto',
+       description: 'Control-Flow Integrity (CFI)')
+option('cfi_debug', type: 'feature', value: 'auto',
+       description: 'Verbose errors in case of CFI violation')
 
 option('cocoa', type : 'feature', value : 'auto',
        description: 'Cocoa user interface (macOS only)')
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 5/5] docs: Add CFI Documentation
  2020-12-04 23:06 [PATCH v4 0/5] Add support for Control-Flow Integrity Daniele Buono
                   ` (3 preceding siblings ...)
  2020-12-04 23:06 ` [PATCH v4 4/5] configure,meson: support Control-Flow Integrity Daniele Buono
@ 2020-12-04 23:06 ` Daniele Buono
  2020-12-13  3:04   ` Alexander Bulekov
  2020-12-14 11:33   ` Paolo Bonzini
  4 siblings, 2 replies; 13+ messages in thread
From: Daniele Buono @ 2020-12-04 23:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniele Buono

Document how to compile with CFI and how to maintain CFI-safe code

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 docs/devel/control-flow-integrity.rst | 137 ++++++++++++++++++++++++++
 1 file changed, 137 insertions(+)
 create mode 100644 docs/devel/control-flow-integrity.rst

diff --git a/docs/devel/control-flow-integrity.rst b/docs/devel/control-flow-integrity.rst
new file mode 100644
index 0000000000..ec54d16a42
--- /dev/null
+++ b/docs/devel/control-flow-integrity.rst
@@ -0,0 +1,137 @@
+============================
+Control-Flow Integrity (CFI)
+============================
+
+This document describes the current control-flow integrity (CFI) mechanism in
+QEMU. How it can be enabled, its benefits and deficiencies, and how it affects
+new and existing code in QEMU
+
+Basics
+------
+
+CFI is a hardening technique that focusing on guaranteeing that indirect
+function calls have not been altered by an attacker.
+The type used in QEMU is a forward-edge control-flow integrity that ensures
+function calls performed through function pointers, always call a "compatible"
+function. A compatible function is a function with the same signature of the
+function pointer declared in the source code.
+
+This type of CFI is entirely compiler-based and relies on the compiler knowing
+the signature of every function and every function pointer used in the code.
+As of now, the only compiler that provides support for CFI is Clang.
+
+CFI is best used on production binaries, to protect against unknown attack
+vectors.
+
+In case of a CFI violation (i.e. call to a non-compatible function) QEMU will
+terminate abruptly, to stop the possible attack.
+
+Building with CFI
+-----------------
+
+NOTE: CFI requires the use of link-time optimization. Therefore, when CFI is
+selected, LTO will be automatically enabled.
+
+To build with CFI, the minimum requirement is Clang 6+. If you
+are planning to also enable fuzzing, then Clang 11+ is needed (more on this
+later).
+
+Given the use of LTO, a version of AR that supports LLVM IR is required.
+The easies way of doing this is by selecting the AR provided by LLVM::
+
+ AR=llvm-ar-9 CC=clang-9 CXX=lang++-9 /path/to/configure --enable-cfi
+
+CFI is enabled on every binary produced.
+
+If desired, an additional flag to increase the verbosity of the output in case
+of a CFI violation is offered (``--enable-debug-cfi``).
+
+Using QEMU built with CFI
+-------------------------
+
+A binary with CFI will work exactly like a standard binary. In case of a CFI
+violation, the binary will terminate with an illegal instruction signal.
+
+Incompatible code with CFI
+--------------------------
+
+As mentioned above, CFI is entirely compiler-based and therefore relies on
+compile-time knowledge of the code. This means that, while generally supported
+for most code, some specific use pattern can break CFI compatibility, and
+create false-positives. The two main patterns that can cause issues are:
+
+* Just-in-time compiled code: since such code is created at runtime, the jump
+  to the buffer containing JIT code will fail.
+
+* Libraries loaded dynamically, e.g. with dlopen/dlsym, since the library was
+  not known at compile time.
+
+Current areas of QEMU that are not entirely compatible with CFI are:
+
+1. TCG, since the idea of TCG is to pre-compile groups of instructions at
+   runtime to speed-up interpretation, quite similarly to a JIT compiler
+
+2. TCI, where the interpreter has to interpret the generic *call* operation
+
+3. Plugins, since a plugin is implemented as an external library
+
+4. Modules, since they are implemented as an external library
+
+5. Directly calling signal handlers from the QEMU source code, since the
+   signal handler may have been provided by an external library or even plugged
+   at runtime.
+
+Disabling CFI for a specific function
+-------------------------------------
+
+If you are working on function that is performing a call using an
+incompatible way, as described before, you can selectively disable CFI checks
+for such function by using the decorator ``QEMU_DISABLE_CFI`` at function
+definition, and add an explanation on why the function is not compatible
+with CFI. An example of the use of ``QEMU_DISABLE_CFI`` is provided here::
+
+	/*
+	 * Disable CFI checks.
+	 * TCG creates binary blobs at runtime, with the transformed code.
+	 * A TB is a blob of binary code, created at runtime and called with an
+	 * indirect function call. Since such function did not exist at compile time,
+	 * the CFI runtime has no way to verify its signature and would fail.
+	 * TCG is not considered a security-sensitive part of QEMU so this does not
+	 * affect the impact of CFI in environment with high security requirements
+	 */
+	QEMU_DISABLE_CFI
+	static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
+
+NOTE: CFI needs to be disabled at the **caller** function, (i.e. a compatible
+cfi function that calls a non-compatible one), since the check is performed
+when the function call is performed.
+
+CFI and fuzzing
+---------------
+
+There is generally no advantage of using CFI and fuzzing together, because
+they target different environments (production for CFI, debug for fuzzing).
+
+CFI could be used in conjunction with fuzzing to identify a broader set of
+bugs that may not end immediately in a segmentation fault or triggering
+an assertion. However, other sanitizers such as address and ub sanitizers
+can identify such bugs in a more precise way than CFI.
+
+There is, however, an interesting use case in using CFI in conjunction with
+fuzzing, that is to make sure that CFI is not triggering any false positive
+in remote-but-possible parts of the code.
+
+CFI can be enabled with fuzzing, but with some caveats:
+1. Fuzzing relies on the linker performing function wrapping at link-time.
+The standard BFD linker does not support function wrapping when LTO is
+also enabled. The workaround is to use LLVM's lld linker.
+2. Fuzzing also relies on a custom linker script, which is only supported by
+lld with version 11+.
+
+In other words, to compile with fuzzing and CFI, clang 11+ is required, and
+lld needs to be used as a linker::
+
+ AR=llvm-ar-11 CC=clang-11 CXX=lang++-11 /path/to/configure --enable-cfi \
+                           -enable-fuzzing --extra-ldflags="-fuse-ld=lld"
+
+and then, compile the fuzzers as usual.
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 4/5] configure,meson: support Control-Flow Integrity
  2020-12-04 23:06 ` [PATCH v4 4/5] configure,meson: support Control-Flow Integrity Daniele Buono
@ 2020-12-13  2:55   ` Alexander Bulekov
  2020-12-14 11:22     ` Paolo Bonzini
  2020-12-14 11:22   ` Paolo Bonzini
  1 sibling, 1 reply; 13+ messages in thread
From: Alexander Bulekov @ 2020-12-13  2:55 UTC (permalink / raw)
  To: Daniele Buono; +Cc: Paolo Bonzini, qemu-devel

On 201204 1806, Daniele Buono wrote:
> This patch adds a flag to enable/disable control flow integrity checks
> on indirect function calls.
> This feature only allows indirect function calls at runtime to functions
> with compatible signatures.
> 
> This feature is only provided by LLVM/Clang, and depends on link-time
> optimization which is currently supported only with LLVM/Clang >= 6.0
> 
> We also add an option to enable a debugging version of cfi, with verbose
> output in case of a CFI violation.
> 
> CFI on indirect function calls does not support calls to functions in
> shared libraries (since they were not known at compile time), and such
> calls are forbidden. QEMU relies on dlopen/dlsym when using modules,
> so we make modules incompatible with CFI.
> 
> All the checks are performed in meson.build. configure is only used to
> forward the flags to meson
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> ---
>  configure         | 21 ++++++++++++++++++++-
>  meson.build       | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  meson_options.txt |  4 ++++
>  3 files changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index fee118518b..c4e5d92167 100755
> --- a/configure
> +++ b/configure
> @@ -400,6 +400,8 @@ coroutine=""
>  coroutine_pool=""
>  debug_stack_usage="no"
>  crypto_afalg="no"
> +cfi="disabled"
> +cfi_debug="disabled"
>  seccomp=""
>  glusterfs=""
>  glusterfs_xlator_opt="no"
> @@ -1180,6 +1182,16 @@ for opt do
>    ;;
>    --disable-safe-stack) safe_stack="no"
>    ;;
> +  --enable-cfi)
> +      cfi="enabled";
> +      lto="true";
> +  ;;
> +  --disable-cfi) cfi="disabled"
> +  ;;
> +  --enable-cfi-debug) cfi_debug="enabled"
> +  ;;
> +  --disable-cfi-debug) cfi_debug="disabled"
> +  ;;
>    --disable-curses) curses="disabled"
>    ;;
>    --enable-curses) curses="enabled"
> @@ -1760,6 +1772,13 @@ disabled with --disable-FEATURE, default is enabled if available:
>    sparse          sparse checker
>    safe-stack      SafeStack Stack Smash Protection. Depends on
>                    clang/llvm >= 3.7 and requires coroutine backend ucontext.
> +  cfi             Enable Control-Flow Integrity for indirect function calls.
> +                  In case of a cfi violation, QEMU is terminated with SIGILL
> +                  Depends on lto and is incompatible with modules
> +                  Automatically enables Link-Time Optimization (lto)
> +  cfi-debug       In case of a cfi violation, a message containing the line that
> +                  triggered the error is written to stderr. After the error,
> +                  QEMU is still terminated with SIGILL
>  
>    gnutls          GNUTLS cryptography support
>    nettle          nettle cryptography support
> @@ -7020,7 +7039,7 @@ NINJA=$ninja $meson setup \
>          -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
>          -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
>          -Dvhost_user_blk_server=$vhost_user_blk_server \
> -        -Db_lto=$lto \
> +        -Db_lto=$lto -Dcfi=$cfi -Dcfi_debug=$cfi_debug \
>          $cross_arg \
>          "$PWD" "$source_path"
>  
> diff --git a/meson.build b/meson.build
> index ebd1c690e0..e1ae6521e0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -773,6 +773,48 @@ elif get_option('vhost_user_blk_server').disabled() or not have_system
>      have_vhost_user_blk_server = false
>  endif
>  
> +if get_option('cfi').enabled()
> +  cfi_flags=[]
> +  # Check for dependency on LTO
> +  if not get_option('b_lto')
> +    error('Selected Control-Flow Integrity but LTO is disabled')
> +  endif
> +  if config_host.has_key('CONFIG_MODULES')
> +    error('Selected Control-Flow Integrity is not compatible with modules')
> +  endif
> +  # Check for cfi flags. CFI requires LTO so we can't use
> +  # get_supported_arguments, but need a more complex "compiles" which allows
> +  # custom arguments
> +  if cc.compiles('int main () { return 0; }', name: '-fsanitize=cfi-icall',
> +                 args: ['-flto', '-fsanitize=cfi-icall'] )
> +    cfi_flags += '-fsanitize=cfi-icall'
> +  else
> +    error('-fsanitize=cfi-icall is not supported by the compiler')
> +  endif
> +  if cc.compiles('int main () { return 0; }',
> +                 name: '-fsanitize-cfi-icall-generalize-pointers',
> +                 args: ['-flto', '-fsanitize=cfi-icall',
> +                        '-fsanitize-cfi-icall-generalize-pointers'] )
> +    cfi_flags += '-fsanitize-cfi-icall-generalize-pointers'
> +  else
> +    error('-fsanitize-cfi-icall-generalize-pointers is not supported by the compiler')
> +  endif
> +  if get_option('cfi_debug').enabled()
> +    if cc.compiles('int main () { return 0; }',
> +                   name: '-fno-sanitize-trap=cfi-icall',
> +                   args: ['-flto', '-fsanitize=cfi-icall',
> +                          '-fno-sanitize-trap=cfi-icall'] )
> +      cfi_flags += '-fno-sanitize-trap=cfi-icall'
> +    else
> +      error('-fno-sanitize-trap=cfi-icall is not supported by the compiler')
> +    endif
> +  endif
> +  add_project_arguments(cfi_flags, native: false, language: ['c', 'cpp',
> +                                                             'objc'])
> +  add_project_link_arguments(cfi_flags, native: false, language: ['c', 'cpp',
> +                                                                  'objc'])
> +endif

Hi Daniele,
I think it would be nice to have a separate block for get_option('d_lto').
Unless I missed something, right now --enable-lto --disable-cfi builds
don't actually use lto.
Thanks
-Alex

> +
>  #################
>  # config-host.h #
>  #################
> @@ -807,6 +849,7 @@ config_host_data.set('CONFIG_KEYUTILS', keyutils.found())
>  config_host_data.set('CONFIG_GETTID', has_gettid)
>  config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
>  config_host_data.set('CONFIG_STATX', has_statx)
> +config_host_data.set('CONFIG_CFI', get_option('cfi').enabled())
>  config_host_data.set('QEMU_VERSION', '"@0@"'.format(meson.project_version()))
>  config_host_data.set('QEMU_VERSION_MAJOR', meson.project_version().split('.')[0])
>  config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1])
> @@ -2159,6 +2202,8 @@ if targetos == 'windows'
>    summary_info += {'QGA MSI support':   config_host.has_key('CONFIG_QGA_MSI')}
>  endif
>  summary_info += {'seccomp support':   config_host.has_key('CONFIG_SECCOMP')}
> +summary_info += {'cfi support':       get_option('cfi').enabled()}
> +summary_info += {'cfi debug support': get_option('cfi_debug').enabled()}
>  summary_info += {'coroutine backend': config_host['CONFIG_COROUTINE_BACKEND']}
>  summary_info += {'coroutine pool':    config_host['CONFIG_COROUTINE_POOL'] == '1'}
>  summary_info += {'debug stack usage': config_host.has_key('CONFIG_DEBUG_STACK_USAGE')}
> diff --git a/meson_options.txt b/meson_options.txt
> index f6f64785fe..8d5729e450 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -35,6 +35,10 @@ option('xen_pci_passthrough', type: 'feature', value: 'auto',
>         description: 'Xen PCI passthrough support')
>  option('tcg', type: 'feature', value: 'auto',
>         description: 'TCG support')
> +option('cfi', type: 'feature', value: 'auto',
> +       description: 'Control-Flow Integrity (CFI)')
> +option('cfi_debug', type: 'feature', value: 'auto',
> +       description: 'Verbose errors in case of CFI violation')
>  
>  option('cocoa', type : 'feature', value : 'auto',
>         description: 'Cocoa user interface (macOS only)')
> -- 
> 2.17.1
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 5/5] docs: Add CFI Documentation
  2020-12-04 23:06 ` [PATCH v4 5/5] docs: Add CFI Documentation Daniele Buono
@ 2020-12-13  3:04   ` Alexander Bulekov
  2020-12-14 11:33   ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Alexander Bulekov @ 2020-12-13  3:04 UTC (permalink / raw)
  To: Daniele Buono; +Cc: Paolo Bonzini, qemu-devel

On 201204 1806, Daniele Buono wrote:
> Document how to compile with CFI and how to maintain CFI-safe code
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>

Reviewed-by: Alexander Bulekov <alxndr@bu.edu>

Thanks

> ---
>  docs/devel/control-flow-integrity.rst | 137 ++++++++++++++++++++++++++
>  1 file changed, 137 insertions(+)
>  create mode 100644 docs/devel/control-flow-integrity.rst
> 
> diff --git a/docs/devel/control-flow-integrity.rst b/docs/devel/control-flow-integrity.rst
> new file mode 100644
> index 0000000000..ec54d16a42
> --- /dev/null
> +++ b/docs/devel/control-flow-integrity.rst
> @@ -0,0 +1,137 @@
> +============================
> +Control-Flow Integrity (CFI)
> +============================
> +
> +This document describes the current control-flow integrity (CFI) mechanism in
> +QEMU. How it can be enabled, its benefits and deficiencies, and how it affects
> +new and existing code in QEMU
> +
> +Basics
> +------
> +
> +CFI is a hardening technique that focusing on guaranteeing that indirect
> +function calls have not been altered by an attacker.
> +The type used in QEMU is a forward-edge control-flow integrity that ensures
> +function calls performed through function pointers, always call a "compatible"
> +function. A compatible function is a function with the same signature of the
> +function pointer declared in the source code.
> +
> +This type of CFI is entirely compiler-based and relies on the compiler knowing
> +the signature of every function and every function pointer used in the code.
> +As of now, the only compiler that provides support for CFI is Clang.
> +
> +CFI is best used on production binaries, to protect against unknown attack
> +vectors.
> +
> +In case of a CFI violation (i.e. call to a non-compatible function) QEMU will
> +terminate abruptly, to stop the possible attack.
> +
> +Building with CFI
> +-----------------
> +
> +NOTE: CFI requires the use of link-time optimization. Therefore, when CFI is
> +selected, LTO will be automatically enabled.
> +
> +To build with CFI, the minimum requirement is Clang 6+. If you
> +are planning to also enable fuzzing, then Clang 11+ is needed (more on this
> +later).
> +
> +Given the use of LTO, a version of AR that supports LLVM IR is required.
> +The easies way of doing this is by selecting the AR provided by LLVM::
> +
> + AR=llvm-ar-9 CC=clang-9 CXX=lang++-9 /path/to/configure --enable-cfi
> +
> +CFI is enabled on every binary produced.
> +
> +If desired, an additional flag to increase the verbosity of the output in case
> +of a CFI violation is offered (``--enable-debug-cfi``).
> +
> +Using QEMU built with CFI
> +-------------------------
> +
> +A binary with CFI will work exactly like a standard binary. In case of a CFI
> +violation, the binary will terminate with an illegal instruction signal.
> +
> +Incompatible code with CFI
> +--------------------------
> +
> +As mentioned above, CFI is entirely compiler-based and therefore relies on
> +compile-time knowledge of the code. This means that, while generally supported
> +for most code, some specific use pattern can break CFI compatibility, and
> +create false-positives. The two main patterns that can cause issues are:
> +
> +* Just-in-time compiled code: since such code is created at runtime, the jump
> +  to the buffer containing JIT code will fail.
> +
> +* Libraries loaded dynamically, e.g. with dlopen/dlsym, since the library was
> +  not known at compile time.
> +
> +Current areas of QEMU that are not entirely compatible with CFI are:
> +
> +1. TCG, since the idea of TCG is to pre-compile groups of instructions at
> +   runtime to speed-up interpretation, quite similarly to a JIT compiler
> +
> +2. TCI, where the interpreter has to interpret the generic *call* operation
> +
> +3. Plugins, since a plugin is implemented as an external library
> +
> +4. Modules, since they are implemented as an external library
> +
> +5. Directly calling signal handlers from the QEMU source code, since the
> +   signal handler may have been provided by an external library or even plugged
> +   at runtime.
> +
> +Disabling CFI for a specific function
> +-------------------------------------
> +
> +If you are working on function that is performing a call using an
> +incompatible way, as described before, you can selectively disable CFI checks
> +for such function by using the decorator ``QEMU_DISABLE_CFI`` at function
> +definition, and add an explanation on why the function is not compatible
> +with CFI. An example of the use of ``QEMU_DISABLE_CFI`` is provided here::
> +
> +	/*
> +	 * Disable CFI checks.
> +	 * TCG creates binary blobs at runtime, with the transformed code.
> +	 * A TB is a blob of binary code, created at runtime and called with an
> +	 * indirect function call. Since such function did not exist at compile time,
> +	 * the CFI runtime has no way to verify its signature and would fail.
> +	 * TCG is not considered a security-sensitive part of QEMU so this does not
> +	 * affect the impact of CFI in environment with high security requirements
> +	 */
> +	QEMU_DISABLE_CFI
> +	static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
> +
> +NOTE: CFI needs to be disabled at the **caller** function, (i.e. a compatible
> +cfi function that calls a non-compatible one), since the check is performed
> +when the function call is performed.
> +
> +CFI and fuzzing
> +---------------
> +
> +There is generally no advantage of using CFI and fuzzing together, because
> +they target different environments (production for CFI, debug for fuzzing).
> +
> +CFI could be used in conjunction with fuzzing to identify a broader set of
> +bugs that may not end immediately in a segmentation fault or triggering
> +an assertion. However, other sanitizers such as address and ub sanitizers
> +can identify such bugs in a more precise way than CFI.
> +
> +There is, however, an interesting use case in using CFI in conjunction with
> +fuzzing, that is to make sure that CFI is not triggering any false positive
> +in remote-but-possible parts of the code.
> +
> +CFI can be enabled with fuzzing, but with some caveats:
> +1. Fuzzing relies on the linker performing function wrapping at link-time.
> +The standard BFD linker does not support function wrapping when LTO is
> +also enabled. The workaround is to use LLVM's lld linker.
> +2. Fuzzing also relies on a custom linker script, which is only supported by
> +lld with version 11+.
> +
> +In other words, to compile with fuzzing and CFI, clang 11+ is required, and
> +lld needs to be used as a linker::
> +
> + AR=llvm-ar-11 CC=clang-11 CXX=lang++-11 /path/to/configure --enable-cfi \
> +                           -enable-fuzzing --extra-ldflags="-fuse-ld=lld"
> +
> +and then, compile the fuzzers as usual.
> -- 
> 2.17.1
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 4/5] configure,meson: support Control-Flow Integrity
  2020-12-04 23:06 ` [PATCH v4 4/5] configure,meson: support Control-Flow Integrity Daniele Buono
  2020-12-13  2:55   ` Alexander Bulekov
@ 2020-12-14 11:22   ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2020-12-14 11:22 UTC (permalink / raw)
  To: Daniele Buono, qemu-devel

On 05/12/20 00:06, Daniele Buono wrote:
> +option('cfi', type: 'feature', value: 'auto',
> +       description: 'Control-Flow Integrity (CFI)')
> +option('cfi_debug', type: 'feature', value: 'auto',
> +       description: 'Verbose errors in case of CFI violation')

I'm changing these to value: 'disabled' to match what configure does 
already.

Paolo



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 4/5] configure,meson: support Control-Flow Integrity
  2020-12-13  2:55   ` Alexander Bulekov
@ 2020-12-14 11:22     ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2020-12-14 11:22 UTC (permalink / raw)
  To: Alexander Bulekov, Daniele Buono; +Cc: qemu-devel

On 13/12/20 03:55, Alexander Bulekov wrote:
> Hi Daniele,
> I think it would be nice to have a separate block for get_option('d_lto').
> Unless I missed something, right now --enable-lto --disable-cfi builds
> don't actually use lto.

Meson handles b_lto internally, it should work.

Paolo



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 5/5] docs: Add CFI Documentation
  2020-12-04 23:06 ` [PATCH v4 5/5] docs: Add CFI Documentation Daniele Buono
  2020-12-13  3:04   ` Alexander Bulekov
@ 2020-12-14 11:33   ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2020-12-14 11:33 UTC (permalink / raw)
  To: Daniele Buono, qemu-devel

On 05/12/20 00:06, Daniele Buono wrote:
> Document how to compile with CFI and how to maintain CFI-safe code
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> ---
>   docs/devel/control-flow-integrity.rst | 137 ++++++++++++++++++++++++++
>   1 file changed, 137 insertions(+)
>   create mode 100644 docs/devel/control-flow-integrity.rst

You would have to add the file to docs/devel/index.rst as well.  I'll do it.

Paolo

> diff --git a/docs/devel/control-flow-integrity.rst b/docs/devel/control-flow-integrity.rst
> new file mode 100644
> index 0000000000..ec54d16a42
> --- /dev/null
> +++ b/docs/devel/control-flow-integrity.rst
> @@ -0,0 +1,137 @@
> +============================
> +Control-Flow Integrity (CFI)
> +============================
> +
> +This document describes the current control-flow integrity (CFI) mechanism in
> +QEMU. How it can be enabled, its benefits and deficiencies, and how it affects
> +new and existing code in QEMU
> +
> +Basics
> +------
> +
> +CFI is a hardening technique that focusing on guaranteeing that indirect
> +function calls have not been altered by an attacker.
> +The type used in QEMU is a forward-edge control-flow integrity that ensures
> +function calls performed through function pointers, always call a "compatible"
> +function. A compatible function is a function with the same signature of the
> +function pointer declared in the source code.
> +
> +This type of CFI is entirely compiler-based and relies on the compiler knowing
> +the signature of every function and every function pointer used in the code.
> +As of now, the only compiler that provides support for CFI is Clang.
> +
> +CFI is best used on production binaries, to protect against unknown attack
> +vectors.
> +
> +In case of a CFI violation (i.e. call to a non-compatible function) QEMU will
> +terminate abruptly, to stop the possible attack.
> +
> +Building with CFI
> +-----------------
> +
> +NOTE: CFI requires the use of link-time optimization. Therefore, when CFI is
> +selected, LTO will be automatically enabled.
> +
> +To build with CFI, the minimum requirement is Clang 6+. If you
> +are planning to also enable fuzzing, then Clang 11+ is needed (more on this
> +later).
> +
> +Given the use of LTO, a version of AR that supports LLVM IR is required.
> +The easies way of doing this is by selecting the AR provided by LLVM::
> +
> + AR=llvm-ar-9 CC=clang-9 CXX=lang++-9 /path/to/configure --enable-cfi
> +
> +CFI is enabled on every binary produced.
> +
> +If desired, an additional flag to increase the verbosity of the output in case
> +of a CFI violation is offered (``--enable-debug-cfi``).
> +
> +Using QEMU built with CFI
> +-------------------------
> +
> +A binary with CFI will work exactly like a standard binary. In case of a CFI
> +violation, the binary will terminate with an illegal instruction signal.
> +
> +Incompatible code with CFI
> +--------------------------
> +
> +As mentioned above, CFI is entirely compiler-based and therefore relies on
> +compile-time knowledge of the code. This means that, while generally supported
> +for most code, some specific use pattern can break CFI compatibility, and
> +create false-positives. The two main patterns that can cause issues are:
> +
> +* Just-in-time compiled code: since such code is created at runtime, the jump
> +  to the buffer containing JIT code will fail.
> +
> +* Libraries loaded dynamically, e.g. with dlopen/dlsym, since the library was
> +  not known at compile time.
> +
> +Current areas of QEMU that are not entirely compatible with CFI are:
> +
> +1. TCG, since the idea of TCG is to pre-compile groups of instructions at
> +   runtime to speed-up interpretation, quite similarly to a JIT compiler
> +
> +2. TCI, where the interpreter has to interpret the generic *call* operation
> +
> +3. Plugins, since a plugin is implemented as an external library
> +
> +4. Modules, since they are implemented as an external library
> +
> +5. Directly calling signal handlers from the QEMU source code, since the
> +   signal handler may have been provided by an external library or even plugged
> +   at runtime.
> +
> +Disabling CFI for a specific function
> +-------------------------------------
> +
> +If you are working on function that is performing a call using an
> +incompatible way, as described before, you can selectively disable CFI checks
> +for such function by using the decorator ``QEMU_DISABLE_CFI`` at function
> +definition, and add an explanation on why the function is not compatible
> +with CFI. An example of the use of ``QEMU_DISABLE_CFI`` is provided here::
> +
> +	/*
> +	 * Disable CFI checks.
> +	 * TCG creates binary blobs at runtime, with the transformed code.
> +	 * A TB is a blob of binary code, created at runtime and called with an
> +	 * indirect function call. Since such function did not exist at compile time,
> +	 * the CFI runtime has no way to verify its signature and would fail.
> +	 * TCG is not considered a security-sensitive part of QEMU so this does not
> +	 * affect the impact of CFI in environment with high security requirements
> +	 */
> +	QEMU_DISABLE_CFI
> +	static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
> +
> +NOTE: CFI needs to be disabled at the **caller** function, (i.e. a compatible
> +cfi function that calls a non-compatible one), since the check is performed
> +when the function call is performed.
> +
> +CFI and fuzzing
> +---------------
> +
> +There is generally no advantage of using CFI and fuzzing together, because
> +they target different environments (production for CFI, debug for fuzzing).
> +
> +CFI could be used in conjunction with fuzzing to identify a broader set of
> +bugs that may not end immediately in a segmentation fault or triggering
> +an assertion. However, other sanitizers such as address and ub sanitizers
> +can identify such bugs in a more precise way than CFI.
> +
> +There is, however, an interesting use case in using CFI in conjunction with
> +fuzzing, that is to make sure that CFI is not triggering any false positive
> +in remote-but-possible parts of the code.
> +
> +CFI can be enabled with fuzzing, but with some caveats:
> +1. Fuzzing relies on the linker performing function wrapping at link-time.
> +The standard BFD linker does not support function wrapping when LTO is
> +also enabled. The workaround is to use LLVM's lld linker.
> +2. Fuzzing also relies on a custom linker script, which is only supported by
> +lld with version 11+.
> +
> +In other words, to compile with fuzzing and CFI, clang 11+ is required, and
> +lld needs to be used as a linker::
> +
> + AR=llvm-ar-11 CC=clang-11 CXX=lang++-11 /path/to/configure --enable-cfi \
> +                           -enable-fuzzing --extra-ldflags="-fuse-ld=lld"
> +
> +and then, compile the fuzzers as usual.
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/5] configure,meson: add option to enable LTO
  2020-12-04 23:06 ` [PATCH v4 1/5] configure,meson: add option to enable LTO Daniele Buono
@ 2021-07-11 10:22   ` Thomas Huth
  2021-07-15 15:46     ` Thomas Huth
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Huth @ 2021-07-11 10:22 UTC (permalink / raw)
  To: Daniele Buono, qemu-devel; +Cc: Paolo Bonzini

On 05/12/2020 00.06, Daniele Buono wrote:
> This patch allows to compile QEMU with link-time optimization (LTO).
> Compilation with LTO is handled directly by meson. This patch only
> adds the option in configure and forwards the request to meson
> 
> Tested with all major versions of clang from 6 to 12
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> ---
>   configure   | 7 +++++++
>   meson.build | 1 +
>   2 files changed, 8 insertions(+)
> 
> diff --git a/configure b/configure
> index 18c26e0389..fee118518b 100755
> --- a/configure
> +++ b/configure
> @@ -242,6 +242,7 @@ host_cc="cc"
>   audio_win_int=""
>   libs_qga=""
>   debug_info="yes"
> +lto="false"
>   stack_protector=""
>   safe_stack=""
>   use_containers="yes"
> @@ -1167,6 +1168,10 @@ for opt do
>     ;;
>     --disable-werror) werror="no"
>     ;;
> +  --enable-lto) lto="true"
> +  ;;
> +  --disable-lto) lto="false"
> +  ;;
>     --enable-stack-protector) stack_protector="yes"
>     ;;
>     --disable-stack-protector) stack_protector="no"
> @@ -1751,6 +1756,7 @@ disabled with --disable-FEATURE, default is enabled if available:
>     module-upgrades try to load modules from alternate paths for upgrades
>     debug-tcg       TCG debugging (default is disabled)
>     debug-info      debugging information
> +  lto             Enable Link-Time Optimization.
>     sparse          sparse checker
>     safe-stack      SafeStack Stack Smash Protection. Depends on
>                     clang/llvm >= 3.7 and requires coroutine backend ucontext.
> @@ -7014,6 +7020,7 @@ NINJA=$ninja $meson setup \
>           -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
>           -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
>           -Dvhost_user_blk_server=$vhost_user_blk_server \
> +        -Db_lto=$lto \
>           $cross_arg \
>           "$PWD" "$source_path"
>   
> diff --git a/meson.build b/meson.build
> index e3386196ba..ebd1c690e0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2044,6 +2044,7 @@ summary_info += {'gprof enabled':     config_host.has_key('CONFIG_GPROF')}
>   summary_info += {'sparse enabled':    sparse.found()}
>   summary_info += {'strip binaries':    get_option('strip')}
>   summary_info += {'profiler':          config_host.has_key('CONFIG_PROFILER')}
> +summary_info += {'link-time optimization (LTO)': get_option('b_lto')}
>   summary_info += {'static build':      config_host.has_key('CONFIG_STATIC')}
>   if targetos == 'darwin'
>     summary_info += {'Cocoa support': config_host.has_key('CONFIG_COCOA')}
> 

I just came across this --enable-lto option ... but looking at the 
implementation here, it seems only to emit a line in the summary_info, 
without adding any compiler flags? Was this patch incomplete? Or do I miss 
something?

  Thomas



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/5] configure,meson: add option to enable LTO
  2021-07-11 10:22   ` Thomas Huth
@ 2021-07-15 15:46     ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2021-07-15 15:46 UTC (permalink / raw)
  To: Daniele Buono, qemu-devel; +Cc: Paolo Bonzini

On 11/07/2021 12.22, Thomas Huth wrote:
> On 05/12/2020 00.06, Daniele Buono wrote:
>> This patch allows to compile QEMU with link-time optimization (LTO).
>> Compilation with LTO is handled directly by meson. This patch only
>> adds the option in configure and forwards the request to meson
>>
>> Tested with all major versions of clang from 6 to 12
>>
>> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
>> ---
>>   configure   | 7 +++++++
>>   meson.build | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 18c26e0389..fee118518b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -242,6 +242,7 @@ host_cc="cc"
>>   audio_win_int=""
>>   libs_qga=""
>>   debug_info="yes"
>> +lto="false"
>>   stack_protector=""
>>   safe_stack=""
>>   use_containers="yes"
>> @@ -1167,6 +1168,10 @@ for opt do
>>     ;;
>>     --disable-werror) werror="no"
>>     ;;
>> +  --enable-lto) lto="true"
>> +  ;;
>> +  --disable-lto) lto="false"
>> +  ;;
>>     --enable-stack-protector) stack_protector="yes"
>>     ;;
>>     --disable-stack-protector) stack_protector="no"
>> @@ -1751,6 +1756,7 @@ disabled with --disable-FEATURE, default is enabled 
>> if available:
>>     module-upgrades try to load modules from alternate paths for upgrades
>>     debug-tcg       TCG debugging (default is disabled)
>>     debug-info      debugging information
>> +  lto             Enable Link-Time Optimization.
>>     sparse          sparse checker
>>     safe-stack      SafeStack Stack Smash Protection. Depends on
>>                     clang/llvm >= 3.7 and requires coroutine backend 
>> ucontext.
>> @@ -7014,6 +7020,7 @@ NINJA=$ninja $meson setup \
>>           -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
>>           -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
>>           -Dvhost_user_blk_server=$vhost_user_blk_server \
>> +        -Db_lto=$lto \
>>           $cross_arg \
>>           "$PWD" "$source_path"
>> diff --git a/meson.build b/meson.build
>> index e3386196ba..ebd1c690e0 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2044,6 +2044,7 @@ summary_info += {'gprof enabled':     
>> config_host.has_key('CONFIG_GPROF')}
>>   summary_info += {'sparse enabled':    sparse.found()}
>>   summary_info += {'strip binaries':    get_option('strip')}
>>   summary_info += {'profiler':          
>> config_host.has_key('CONFIG_PROFILER')}
>> +summary_info += {'link-time optimization (LTO)': get_option('b_lto')}
>>   summary_info += {'static build':      config_host.has_key('CONFIG_STATIC')}
>>   if targetos == 'darwin'
>>     summary_info += {'Cocoa support': config_host.has_key('CONFIG_COCOA')}
>>
> 
> I just came across this --enable-lto option ... but looking at the 
> implementation here, it seems only to emit a line in the summary_info, 
> without adding any compiler flags? Was this patch incomplete? Or do I miss 
> something?

Never mind, I now learnt that b_lto is apparently an option that is directly 
understood by meson already :-)

  Thomas



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-07-15 15:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04 23:06 [PATCH v4 0/5] Add support for Control-Flow Integrity Daniele Buono
2020-12-04 23:06 ` [PATCH v4 1/5] configure,meson: add option to enable LTO Daniele Buono
2021-07-11 10:22   ` Thomas Huth
2021-07-15 15:46     ` Thomas Huth
2020-12-04 23:06 ` [PATCH v4 2/5] cfi: Initial support for cfi-icall in QEMU Daniele Buono
2020-12-04 23:06 ` [PATCH v4 3/5] check-block: enable iotests with cfi-icall Daniele Buono
2020-12-04 23:06 ` [PATCH v4 4/5] configure,meson: support Control-Flow Integrity Daniele Buono
2020-12-13  2:55   ` Alexander Bulekov
2020-12-14 11:22     ` Paolo Bonzini
2020-12-14 11:22   ` Paolo Bonzini
2020-12-04 23:06 ` [PATCH v4 5/5] docs: Add CFI Documentation Daniele Buono
2020-12-13  3:04   ` Alexander Bulekov
2020-12-14 11:33   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.