Re: [PATCH v2 0/6] Add support for Control-Flow Integrity

[Daniele Buono]

On 10/23/2020 4:33 PM, Eric Blake wrote:
> On 10/23/20 3:06 PM, Daniele Buono wrote:
> > v2: Several months (and structural changes in QEMU) have passed since v1.
> > While the spirit of the patch is similar, the implementation is changed
> > in multiple points, and should address most if not all the comments
> > received in v1.
>
> > 5) Most of the logic to enable CFI goes in the configure, since it's
> > just a matter of checking for dependencies and incompatible options.
> > However, I had to disable CFI checks for a few TCG functions.
> > This can only be done through a blacklist file. I added a file in the
> > root of QEMU, called cfi-blacklist.txt for such purpose. I am open to
> > suggestions on where the file should go, and I am willing to become the
> > maintainer of it, if deemed necessary.
>
> In the meantime, we have commits like:
>
> commit b199c682f1f0aaee22b2170a5fb885250057eec2
> Author: Philippe Mathieu-Daudé <philmd@redhat.com>
> Date:   Thu Sep 10 09:01:31 2020 +0200
>
>      target/i386/kvm: Rename host_tsx_blacklisted() as host_tsx_broken()
>
>      In order to use inclusive terminology, rename host_tsx_blacklisted()
>      as host_tsx_broken().
>
> which may help you in coming up with a more appropriate name for the new
> file.
>
> >   MAINTAINERS                   |   5 +
> >   accel/tcg/cpu-exec.c          |   9 ++
> >   configure                     | 214 ++++++++++++++++++++++++++++++++++
> >   include/qemu/sanitizers.h     |  22 ++++
> >   meson.build                   |   3 +
> >   plugins/core.c                |  25 ++++
> >   plugins/loader.c              |   5 +
> >   tcg/tci.c                     |   5 +
> >   tests/check-block.sh          |  18 +--
> >   tests/qtest/fuzz/fork_fuzz.ld |  12 +-
> >   util/main-loop.c              |   9 ++
> >   util/oslib-posix.c            |   9 ++
> >   12 files changed, 328 insertions(+), 8 deletions(-)
> >   create mode 100644 include/qemu/sanitizers.h
>
> although I don't see a new file by that name here, so perhaps the v1
> overview is now stale?
Correct, the v1 overview is stale on that regard. V2 is not using a
"broken" file anymore. CFI is now disabled by using an attribute
directly on the code.

From the v2 overview:
* Instead of disabling CFI in specific functions by using a filter file,
  disable cfi by using a new decorator to be prefixed to the function
  definition.

Beside the removal of a non-inclusive term, I believe this is a better
way to track functions, since it is directly inside the code so everyone
working on those functions will see it immediately. It's safer with
regards of function naming changes and, hopefully, this will make
maintaining cfi easier.