Re: [PATCH v2 3/4] xen: arm: enable stack protector feature

[Julien Grall]

Hi Volodymyr,

On 30/11/2024 01:10, Volodymyr Babchuk wrote:
Enable previously added CONFIG_STACK_PROTECTOR feature for ARM
platform. Here we can call boot_stack_chk_guard_setup() in start_xen()
function, because it never returns, so stack protector code will not
be triggered because of changed canary.

It would be good to explain how you decided to call...


Signed-off-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>

---

In v2:
  - Reordered Kconfig entry
---
  xen/arch/arm/Kconfig | 1 +
  xen/arch/arm/setup.c | 3 +++
  2 files changed, 4 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 23bbc91aad..a24c88c327 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -16,6 +16,7 @@ config ARM
        select HAS_ALTERNATIVE if HAS_VMAP
        select HAS_DEVICE_TREE
        select HAS_PASSTHROUGH
+       select HAS_STACK_PROTECTOR
        select HAS_UBSAN
        select IOMMU_FORCE_PT_SHARE
  
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 2e27af4560..f855e97e25 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -13,6 +13,7 @@
  #include <xen/domain_page.h>
  #include <xen/grant_table.h>
  #include <xen/types.h>
+#include <xen/stack-protector.h>
  #include <xen/string.h>
  #include <xen/serial.h>
  #include <xen/sched.h>
@@ -341,6 +342,8 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr)
       */
      system_state = SYS_STATE_boot;
  
+    boot_stack_chk_guard_setup();

... the function here. If I am not mistaken, at this point, cpu_khz 
(used by NOW() in get_random()) would be zero. It is only initialized by 
preinit_xen_time() which happens later.

So I think it should be called a bit further down and gain a comment to 
about the call dependencies.

Cheers,

--
Julien Grall