| Message ID | 20200429194420.21147-1-dbuono@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for SafeStack | expand |
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 Clang10: Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Daniele Buono (4): > coroutine: support SafeStack in ucontext backend > coroutine: Add check for SafeStack in sigalstack > configure: add flag to enable SafeStack > check-block: Enable iotests with SafeStack > > configure | 29 +++++++++++++++++++++++++++++ > include/qemu/coroutine_int.h | 6 ++++++ > tests/check-block.sh | 12 +++++++++++- > util/coroutine-sigaltstack.c | 4 ++++ > util/coroutine-ucontext.c | 25 +++++++++++++++++++++++++ > 5 files changed, 75 insertions(+), 1 deletion(-) >
+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. > > Daniele Buono (4): > coroutine: support SafeStack in ucontext backend > coroutine: Add check for SafeStack in sigalstack > configure: add flag to enable SafeStack > check-block: Enable iotests with SafeStack > > configure | 29 +++++++++++++++++++++++++++++ > include/qemu/coroutine_int.h | 6 ++++++ > tests/check-block.sh | 12 +++++++++++- > util/coroutine-sigaltstack.c | 4 ++++ > util/coroutine-ucontext.c | 25 +++++++++++++++++++++++++ > 5 files changed, 75 insertions(+), 1 deletion(-) >
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" Regards, Daniel
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 >
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 >> > >
On Wed, May 13, 2020 at 10:48:04AM -0400, Daniele Buono wrote:
> Hello everybody, just pinging since it it's been a few days.
Hi Daniele,
Sorry I'm late to the party. This looks useful, the patches are not
invasive and provide the option of enabling extra security.
I have left comments on specific patches.
Stefan