Re: [PATCH RFT v12 4/8] fork: Add shadow stack support to clone3()

2024-11-29 Thread Yury Khrustalev
On Thu, Oct 31, 2024 at 07:25:05PM +, Mark Brown wrote:
> Unlike with the normal stack there is no API for configuring the the shadow
> stack for a new thread, instead the kernel will dynamically allocate a new
> shadow stack with the same size as the normal stack. This appears to be due
> to the shadow stack series having been in development since before the more
> extensible clone3() was added rather than anything more deliberate.
> 
> Add a paramter to clone3() specifying the shadow stack pointer to use
> for the new thread, this is inconsistent with the way we specify the
> normal stack but during review concerns were expressed about having to
> identify where the shadow stack pointer should be placed especially in
> cases where the shadow stack has been previously active.  If no shadow
> stack is specified then the existing implicit allocation behaviour is
> maintained.
> 
> If a shadow stack pointer is specified then it is required to have an
> architecture defined token placed on the stack, this will be consumed by
> the new task.  If no valid token is present then this will be reported
> with -EINVAL.  This token prevents new threads being created pointing at
> the shadow stack of an existing running thread.
> 
> If the architecture does not support shadow stacks the shadow stack
> pointer must be not be specified, architectures that do support the
> feature are expected to enforce the same requirement on individual
> systems that lack shadow stack support.
> 
> Update the existing arm64 and x86 implementations to pay attention to
> the newly added arguments, in order to maintain compatibility we use the
> existing behaviour if no shadow stack is specified. Since we are now
> using more fields from the kernel_clone_args we pass that into the
> shadow stack code rather than individual fields.
> 
> Portions of the x86 architecture code were written by Rick Edgecombe.
> 
> Signed-off-by: Mark Brown 

Acked-by: Yury Khrustalev 

> @@ -101,12 +103,14 @@ struct clone_args {
>   __aligned_u64 set_tid;
>   __aligned_u64 set_tid_size;
>   __aligned_u64 cgroup;
> + __aligned_u64 shadow_stack_pointer;
>  };
>  #endif
>  
> -#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */
> -#define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */
> -#define CLONE_ARGS_SIZE_VER2 88 /* sizeof third published struct */
> +#define CLONE_ARGS_SIZE_VER0  64 /* sizeof first published struct */
> +#define CLONE_ARGS_SIZE_VER1  80 /* sizeof second published struct */
> +#define CLONE_ARGS_SIZE_VER2  88 /* sizeof third published struct */
> +#define CLONE_ARGS_SIZE_VER3  96 /* sizeof fourth published struct */

Acked.

Kind regards,
Yury




[PATCH RFT v12 4/8] fork: Add shadow stack support to clone3()

2024-10-31 Thread Mark Brown
Unlike with the normal stack there is no API for configuring the the shadow
stack for a new thread, instead the kernel will dynamically allocate a new
shadow stack with the same size as the normal stack. This appears to be due
to the shadow stack series having been in development since before the more
extensible clone3() was added rather than anything more deliberate.

Add a paramter to clone3() specifying the shadow stack pointer to use
for the new thread, this is inconsistent with the way we specify the
normal stack but during review concerns were expressed about having to
identify where the shadow stack pointer should be placed especially in
cases where the shadow stack has been previously active.  If no shadow
stack is specified then the existing implicit allocation behaviour is
maintained.

If a shadow stack pointer is specified then it is required to have an
architecture defined token placed on the stack, this will be consumed by
the new task.  If no valid token is present then this will be reported
with -EINVAL.  This token prevents new threads being created pointing at
the shadow stack of an existing running thread.

If the architecture does not support shadow stacks the shadow stack
pointer must be not be specified, architectures that do support the
feature are expected to enforce the same requirement on individual
systems that lack shadow stack support.

Update the existing arm64 and x86 implementations to pay attention to
the newly added arguments, in order to maintain compatibility we use the
existing behaviour if no shadow stack is specified. Since we are now
using more fields from the kernel_clone_args we pass that into the
shadow stack code rather than individual fields.

Portions of the x86 architecture code were written by Rick Edgecombe.

Signed-off-by: Mark Brown 
---
 arch/arm64/mm/gcs.c  | 54 +-
 arch/x86/include/asm/shstk.h | 11 +++--
 arch/x86/kernel/process.c|  2 +-
 arch/x86/kernel/shstk.c  | 57 +---
 include/asm-generic/cacheflush.h | 11 +
 include/linux/sched/task.h   | 17 +++
 include/uapi/linux/sched.h   | 10 +++--
 kernel/fork.c| 96 +++-
 8 files changed, 232 insertions(+), 26 deletions(-)

diff --git a/arch/arm64/mm/gcs.c b/arch/arm64/mm/gcs.c
index 
1f633a482558b59aac5427963d42b37fce08c8a6..c4e93b7ce05c5dfa1128923ad587f9b5a7fb0051
 100644
--- a/arch/arm64/mm/gcs.c
+++ b/arch/arm64/mm/gcs.c
@@ -43,8 +43,24 @@ int gcs_alloc_thread_stack(struct task_struct *tsk,
 {
unsigned long addr, size;
 
-   if (!system_supports_gcs())
+   if (!system_supports_gcs()) {
+   if (args->shadow_stack_pointer)
+   return -EINVAL;
+
+   return 0;
+   }
+
+   /*
+* If the user specified a GCS then use it, otherwise fall
+* back to a default allocation strategy. Validation is done
+* in arch_shstk_validate_clone().
+*/
+   if (args->shadow_stack_pointer) {
+   tsk->thread.gcs_base = 0;
+   tsk->thread.gcs_size = 0;
+   tsk->thread.gcspr_el0 = args->shadow_stack_pointer;
return 0;
+   }
 
if (!task_gcs_el0_enabled(tsk))
return 0;
@@ -68,6 +84,42 @@ int gcs_alloc_thread_stack(struct task_struct *tsk,
return 0;
 }
 
+static bool gcs_consume_token(struct vm_area_struct *vma, struct page *page,
+ unsigned long user_addr)
+{
+   u64 expected = GCS_CAP(user_addr);
+   u64 *token = page_address(page) + offset_in_page(user_addr);
+
+   if (!cmpxchg_to_user_page(vma, page, user_addr, token, expected, 0))
+   return false;
+   set_page_dirty_lock(page);
+
+   return true;
+}
+
+int arch_shstk_validate_clone(struct task_struct *tsk,
+ struct vm_area_struct *vma,
+ struct page *page,
+ struct kernel_clone_args *args)
+{
+unsigned long gcspr_el0;
+int ret = 0;
+
+   /* Ensure that a token written as a result of a pivot is visible */
+   gcsb_dsync();
+
+   gcspr_el0 = args->shadow_stack_pointer;
+   if (!gcs_consume_token(vma, page, gcspr_el0))
+   return -EINVAL;
+
+   tsk->thread.gcspr_el0 = gcspr_el0 + sizeof(u64);
+
+   /* Ensure that our token consumption visible */
+   gcsb_dsync();
+
+   return ret;
+}
+
 SYSCALL_DEFINE3(map_shadow_stack, unsigned long, addr, unsigned long, size, 
unsigned int, flags)
 {
unsigned long alloc_size;
diff --git a/arch/x86/include/asm/shstk.h b/arch/x86/include/asm/shstk.h
index 
4cb77e004615dff003426a2eb594460ca1015f4e..252feeda69991e939942c74556e23e27c835e766
 100644
--- a/arch/x86/include/asm/shstk.h
+++ b/arch/x86/include/asm/shstk.h
@@ -6,6 +6,7 @@
 #include 
 
 struct task_struct;
+struct kernel_clone_args;
 struct ksignal;
 
 #ifdef