[PATCH 0/3] Add stack protector

Volodymyr Babchuk posted 3 patches 2 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20241122210719.2572072-1-volodymyr._5Fbabchuk@epam.com
There is a newer version of this series
Config.mk                            |  2 +-
stubdom/Makefile                     |  2 ++
tools/firmware/Rules.mk              |  2 ++
tools/tests/x86_emulator/testcase.mk |  2 ++
xen/Makefile                         |  6 ++++++
xen/arch/arm/Kconfig                 |  1 +
xen/arch/arm/setup.c                 |  3 +++
xen/arch/riscv/Kconfig               |  1 +
xen/arch/riscv/setup.c               |  3 +++
xen/common/Kconfig                   | 13 ++++++++++++
xen/common/Makefile                  |  1 +
xen/common/stack_protector.c         | 16 +++++++++++++++
xen/include/xen/stack_protector.h    | 30 ++++++++++++++++++++++++++++
13 files changed, 81 insertions(+), 1 deletion(-)
create mode 100644 xen/common/stack_protector.c
create mode 100644 xen/include/xen/stack_protector.h
[PATCH 0/3] Add stack protector
Posted by Volodymyr Babchuk 2 weeks, 2 days ago
Both GCC and Clang support -fstack-protector feature, which add stack
canaries to functions where stack corruption is possible. This series
makes possible to use this feature in Xen. I tested this on ARM64 and
it is working as intended. Tested both with GCC and Clang.

My aim was to enable it on x86 also, but it appears that on x86 GCC
stores canary value in TLS, exactly at fs:40, which is hardcoded. As
Xen does not setup fs register for itself, any attempt to enable stack
protector leads to paging abort.

I also tested build-ability for RISCV platform, but didn't tested that
it does not break anything, so we will need RISCV maintainer's
approval.

Volodymyr Babchuk (3):
  xen: common: add ability to enable stack protector
  xen: arm: enable stack protector feature
  xen: riscv: enable stack protector feature

 Config.mk                            |  2 +-
 stubdom/Makefile                     |  2 ++
 tools/firmware/Rules.mk              |  2 ++
 tools/tests/x86_emulator/testcase.mk |  2 ++
 xen/Makefile                         |  6 ++++++
 xen/arch/arm/Kconfig                 |  1 +
 xen/arch/arm/setup.c                 |  3 +++
 xen/arch/riscv/Kconfig               |  1 +
 xen/arch/riscv/setup.c               |  3 +++
 xen/common/Kconfig                   | 13 ++++++++++++
 xen/common/Makefile                  |  1 +
 xen/common/stack_protector.c         | 16 +++++++++++++++
 xen/include/xen/stack_protector.h    | 30 ++++++++++++++++++++++++++++
 13 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 xen/common/stack_protector.c
 create mode 100644 xen/include/xen/stack_protector.h

-- 
2.47.0
Re: [PATCH 0/3] Add stack protector
Posted by Andrew Cooper 2 weeks, 2 days ago
On 22/11/2024 9:07 pm, Volodymyr Babchuk wrote:
> Both GCC and Clang support -fstack-protector feature, which add stack
> canaries to functions where stack corruption is possible. This series
> makes possible to use this feature in Xen. I tested this on ARM64 and
> it is working as intended. Tested both with GCC and Clang.
>
> My aim was to enable it on x86 also, but it appears that on x86 GCC
> stores canary value in TLS, exactly at fs:40, which is hardcoded. As
> Xen does not setup fs register for itself, any attempt to enable stack
> protector leads to paging abort.

It's rather worse than a #PF, if a devious PV guest decides to map
memory at 0.

There's a long and complicated history with stack-protector on x86.

It is %fs:40 by default, but this becomes %gs:40 with -mcmodel=kernel
(which Xen doesn't use, but is necessary to know if anyone wants to look
at how Linux does this.)

Xen on x86 uses neither %fs nor %gs for per-cpu variables, because PV
guests do.  This happens to have saved us from a number of privilege
escalation corner cases that exist in the x86 architecture.  I'm very
reluctant to open ourselves up to such problems just to support
stack-protector.


With GCC 8.1, we get -mstack-protector-{guard,reg,offset} to fine-tune
things a whole lot more.  I am not sure when these got into Clang.

In particular, -mstack-protector-guard=global lets us have a scheme that
seems to be the same as the ARM/RISCV, which is probably good enough to
start with, although it involves restarting the toolchain debate and is
probably not something you want to do in this series.

See
https://lore.kernel.org/lkml/20241105155801.1779119-1-brgerst@gmail.com/T/#u
for most of the gory details.

> I also tested build-ability for RISCV platform, but didn't tested that
> it does not break anything, so we will need RISCV maintainer's
> approval.

There's a RISC-V smoke test in gitlab CI, but you can run it locally too.

~Andrew