Re: [PATCH 0/4] Add support for SafeStack

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

Hello everybody, just pinging since it it's been a few days.

On 5/5/2020 9:56 AM, Philippe Mathieu-Daudé wrote:
> On 5/5/20 3:31 PM, Daniel P. Berrangé wrote:
>> On Tue, May 05, 2020 at 03:15:18PM +0200, Philippe Mathieu-Daudé wrote:
>>> +Alex & Daniel who keep track on CI stuff.
>>>
>>> On 4/29/20 9:44 PM, Daniele Buono wrote:
>>>> LLVM supports SafeStack instrumentation to protect against stack buffer
>>>> overflows, since version 3.7
>>>>
>>>>   From https://clang.llvm.org/docs/SafeStack.html:
>>>> "It works by separating the program stack into two distinct regions: 
>>>> the
>>>> safe stack and the unsafe stack. The safe stack stores return 
>>>> addresses,
>>>> register spills, and local variables that are always accessed in a safe
>>>> way, while the unsafe stack stores everything else. This separation
>>>> ensures that buffer overflows on the unsafe stack cannot be used to
>>>> overwrite anything on the safe stack."
>>>>
>>>> Unfortunately, the use of two stack regions does not cope well with
>>>> QEMU's coroutines. The second stack region is not properly set up with
>>>> both ucontext and sigaltstack, so multiple coroutines end up sharing 
>>>> the
>>>> same memory area for the unsafe stack, causing undefined behaviors at
>>>> runtime (and most iochecks to fail).
>>>>
>>>> This patch series fixes the implementation of the ucontext backend and
>>>> make sure that sigaltstack is never used if the compiler is applying
>>>> the SafeStack instrumentation. It also adds a configure flag to enable
>>>> SafeStack, and enables iotests when SafeStack is used.
>>>>
>>>> This is an RFC mainly because of the low-level use of the SafeStack
>>>> runtime.
>>>> When running swapcontext(), we have to manually set the unsafe stack
>>>> pointer to the new area allocated for the coroutine. LLVM does not 
>>>> allow
>>>> this by using builtin, so we have to use implementation details that 
>>>> may
>>>> change in the future.
>>>> This patch has been tested briefly ( make check on an x86 system ) with
>>>> clang v3.9, v4.0, v5.0, v6.0
>>>> Heavier testing, with make check-acceptance has been performed with
>>>> clang v7.0
>>>
>>> I noticed building using SafeStack is slower, and running with it is 
>>> even
>>> sloooower. It makes sense to have this integrated if we use it 
>>> regularly. Do
>>> you have plan for this? Using public CI doesn't seem reasonable.
>>
>> The runtime behaviour is rather odd, given the docs they provide:
>>
>> "The performance overhead of the SafeStack instrumentation is
>>   less than 0.1% on average across a variety of benchmarks
>>   This is mainly because most small functions do not have any
>>   variables that require the unsafe stack and, hence, do not
>>   need unsafe stack frames to be created. The cost of creating
>>   unsafe stack frames for large functions is amortized by the
>>   cost of executing the function.
>>
>>    In some cases, SafeStack actually improves the performance"
> 
> I'm sorry I was testing this with a single core instead of all of 
> them... Thanks for looking at the doc.
> 
>>
>> Regards,
>> Daniel
>>
> 
>