Re: [PATCH 2/2] configure: add support for Control-Flow Integrity

[Daniele Buono <dbuono@linux.vnet.ibm.com>]

On 7/2/2020 5:52 AM, Daniel P. Berrangé wrote:
> 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 ?

Hi Daniel,

I gave it some thought and studied more of clang's options. It is 
possible to disable cfi on specific functions by using an __attribute__ 
keyword, instead of providing a list in an external file.
In terms of maintaining, this is much better since we are removing the 
need to update the list. I would suggest defining a macro, 
__disable_cfi__, that can be prepended to a function. The macro would 
expand to nothing if cfi is disabled, or to the proper attribute if it 
is enabled. Here's example code snippet

/* Disable CFI checks.
  * The callback function has been loaded from an external library so we 
do not
  * have type information */
__disable_cfi__
void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
{
...
}

This would take care of renaming and removal of functions that need cfi.
It would also probably be beneficial to the developers since they can 
see immediately that the function they are working with needs to have 
CFI checks disabled, and why.

If you think this is a better approach, I'll submit v2 with this 
approach instead of the external function list.


For new code, however, the best thing is proper education and testing.
I'll work on a document for docs/devel to detail what it is and how to 
make code cfi-safe.
A good approach should be to test code changes with CFI enabled. CFI is, 
after all, a sanitizer and therefore it makes sense to use it also 
during development, together with ASan, UBSan and the likes. 
Unfortunately, since it works only with clang, I don't think this can 
ever be a hard requirement.

Daniele