On Fri, 2023-03-17 at 10:28 -0700, Deepak Gupta wrote:

On Fri, Mar 17, 2023 at 10:16 AM Dave Hansen [off-list ref]
wrote:

quoted

On 3/17/23 10:12, Deepak Gupta wrote:

quoted

quoted

  /*
- * Stack area - automatically grows in one direction
+ * Stack area
   *
- * VM_GROWSUP / VM_GROWSDOWN VMAs are always private
anonymous:
- * do_mmap() forbids all other combinations.
+ * VM_GROWSUP, VM_GROWSDOWN VMAs are always private
+ * anonymous. do_mmap() forbids all other combinations.
   */
  static inline bool is_stack_mapping(vm_flags_t flags)
  {
-       return (flags & VM_STACK) == VM_STACK;
+       return ((flags & VM_STACK) == VM_STACK) || (flags &
VM_SHADOW_STACK);

Same comment here. `VM_SHADOW_STACK` is an x86 specific way of
encoding a shadow stack.
Instead let's have a proxy here which allows architectures to
have
their own encodings to represent a shadow stack.

This doesn't _preclude_ another architecture from coming along and
doing
that, right?  I'd just prefer that shadow stack architecture #2
comes
along and refactors this in precisely the way _they_ need it.

There are two issues here
 - Encoding of shadow stack: Another arch can choose different
encoding.
   And yes, another architecture can come in and re-factor it. But so
much thought and work has been given to x86 implementation to keep
   shadow stack to not impact arch agnostic parts of the kernel. So
why creep it in here.

- VM_SHADOW_STACK is coming out of the VM_HIGH_ARCH_XX bit position
which makes it arch specific.

VM_SHADOW_STACK is defined like this (trimmed for clarity):
#ifdef CONFIG_X86_USER_SHADOW_STACK
# define VM_SHADOW_STACK	VM_HIGH_ARCH_5
#else
# define VM_SHADOW_STACK	VM_NONE
#endif

Also, we actually had an is_shadow_stack_mapping(vma) in the past, but
it was dropped from other feedback. I think it might be too soon to say
whether other implementations won't end up with a similar vma flag, so
this would be premature refactoring. If not though, a helper like that
seems like a reasonable solution.