On Thu, Jul 02, 2020 at 08:50:08AM -0400, Daniele Buono wrote: > > > On 7/2/2020 5:52 AM, Daniel P. Berrangé wrote: > > On Thu, Jul 02, 2020 at 01:49:48AM -0400, Daniele Buono wrote: > > > This patch adds a flag to enable/disable control flow integrity checks > > > on indirect function calls. This feature is only provided by LLVM/Clang > > > v3.9 or higher, and only allows indirect function calls to functions > > > with compatible signatures. > > > > > > 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. > > > > > > We introduce a blacklist file, to disable CFI checks in a limited number > > > of TCG functions. > > > > > > The feature relies on link-time optimization (lto), which requires the > > > use of the gold linker, and the LLVM versions of ar, ranlib and nm. > > > This patch take care of checking that all the compiler toolchain > > > dependencies are met. > > > > > > Signed-off-by: Daniele Buono <dbu...@linux.vnet.ibm.com> > > > --- > > > cfi-blacklist.txt | 27 +++++++ > > > configure | 177 ++++++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 204 insertions(+) > > > create mode 100644 cfi-blacklist.txt > > > > > > diff --git a/cfi-blacklist.txt b/cfi-blacklist.txt > > > new file mode 100644 > > > index 0000000000..bf804431a5 > > > --- /dev/null > > > +++ b/cfi-blacklist.txt > > > @@ -0,0 +1,27 @@ > > > +# List of functions that should not be compiled with Control-Flow > > > Integrity > > > + > > > +[cfi-icall] > > > +# TCG creates binary blobs at runtime, with the transformed code. > > > +# When it's time to execute it, the code is called with an indirect > > > function > > > +# call. Since such function did not exist at compile time, the runtime > > > has no > > > +# way to verify its signature. Disable CFI checks in the function that > > > calls > > > +# the binary blob > > > +fun:cpu_tb_exec > > > + > > > +# TCI (Tiny Compiler Interpreter) is an interpreter for TCG pseudo code. > > > +# One possible operation in the pseudo code is a call to binary code. > > > +# Therefore, disable CFI checks in the interpreter function > > > +fun:tcg_qemu_tb_exec > > > + > > > +# TCG Plugins Callback Functions. The mechanism rely on opening external > > > +# shared libraries at runtime and get pointers to functions in such > > > libraries > > > +# Since these pointers are external to the QEMU binary, the runtime > > > cannot > > > +# verify their signature. Disable CFI Checks in all the functions that > > > use > > > +# such pointers. > > > +fun:plugin_vcpu_cb__simple > > > +fun:plugin_cb__simple > > > +fun:plugin_cb__udata > > > +fun:qemu_plugin_tb_trans_cb > > > +fun:qemu_plugin_vcpu_syscall > > > +fun:qemu_plugin_vcpu_syscall_ret > > > +fun:plugin_load > > > > The need to maintain this list of functions makes me feel very > > uneasy. > > > > How can we have any confidence that this list of functions is > > accurate ? How will maintainers ensure that they correctly update > > it as they are writing/changing code, and how will they test the > > result ? > > > > It feels like it has the same general maint problem as the original > > seccomp code we used, where we were never confident we had added > > the right exceptions to let QEMU run without crashing when users > > tickled some feature we forgot about. > > > > > > Regards, > > Daniel > > > > I agree with you that keeping that list updated is a daunting task. In my > opinion, however, it is not as difficult as a seccomp filter, for the > following reasons: > > 1) Seccomp covers everything that runs in your process, including shared > libraries that you have no control over. CFI covers only the code in the > QEMU binary. So at least we don't have to guess what other libraries used by > QEMU will or won't do during execution. > > 2) With seccomp you have to filter behavior that, while admissible, should > not happen in your code. CFI can be seen as a run-time type checking system; > if the signature of the function is wrong, that is a coding error... in > theory. In practice, there is a corner-case because the type checking > doesn't know the signature of code loaded or written at run-time, and that > is why you have to use a CFI filter.
Can you elaborate on this function signature rules here ? Is this referring to scenarios where you cast between 2 different function prototypes ? I'm wondering if this applies to the way GLib's main loop source callbacks are registered. eg the g_source_set_callback method takes a callback with a signature of "GSourceFunc" which is gboolean (*)(void *) but the way GSources are implemented means that each implementation gets to define its own custom callback signature. So for example, in QIOChannel we use int (*)(struct QIOChannel *, enum <anonymous>, void *) Thus, we always have an intentional bad function cast when invoking the g_source_set_callback method. GCC is able to warn about these if we add -Wcast-function-type, but we don't do that because these bad casts are intentional. eg io/channel.c: In function ‘qio_channel_add_watch_full’: io/channel.c:315:35: error: cast between incompatible function types from ‘QIOChannelFunc’ {aka ‘int (*)(struct QIOChannel *, enum <anonymous>, void *)’} to ‘gboolean (*)(void *)’ {aka ‘int (*)(void *)’} [-Werror=cast-function-type] 315 | g_source_set_callback(source, (GSourceFunc)func, user_data, notify); | ^ io/channel.c: In function ‘qio_channel_wait’: io/channel.c:507:27: error: cast between incompatible function types from ‘gboolean (*)(QIOChannel *, GIOCondition, void *)’ {aka ‘int (*)(struct QIOChannel *, enum <anonymous>, void *)’} to ‘gboolean (*)(void *)’ {aka ‘int (*)(void *)’} [-Werror=cast-function-type] 507 | (GSourceFunc)qio_channel_wait_complete, | ^ Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|