Re: [PATCH v3 18/37] mm: Add guard pages around a shadow stack.
From: Peter Zijlstra <peterz@infradead.org>
Date: 2022-11-15 19:49:04
Also in:
linux-api, linux-doc, linux-mm, lkml
On Fri, Nov 04, 2022 at 03:35:45PM -0700, Rick Edgecombe wrote:
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c index c90c20904a60..66da1f3298b0 100644 --- a/arch/x86/mm/mmap.c +++ b/arch/x86/mm/mmap.c@@ -248,3 +248,26 @@ bool pfn_modify_allowed(unsigned long pfn, pgprot_t prot) return false; return true; } + +unsigned long stack_guard_start_gap(struct vm_area_struct *vma) +{ + if (vma->vm_flags & VM_GROWSDOWN) + return stack_guard_gap; + + /* + * Shadow stack pointer is moved by CALL, RET, and INCSSP(Q/D).
Can we perhaps write this like: INCSPP[QD] ? The () notation makes it look like a function.
quoted hunk ↗ jump to hunk
+ * INCSSPQ moves shadow stack pointer up to 255 * 8 = ~2 KB + * (~1KB for INCSSPD) and touches the first and the last element + * in the range, which triggers a page fault if the range is not + * in a shadow stack. Because of this, creating 4-KB guard pages + * around a shadow stack prevents these instructions from going + * beyond. + * + * Creation of VM_SHADOW_STACK is tightly controlled, so a vma + * can't be both VM_GROWSDOWN and VM_SHADOW_STACK + */ + if (vma->vm_flags & VM_SHADOW_STACK) + return PAGE_SIZE; + + return 0; +}diff --git a/include/linux/mm.h b/include/linux/mm.h index 5d9536fa860a..0a3f7e2b32df 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h@@ -2832,15 +2832,16 @@ struct vm_area_struct *vma_lookup(struct mm_struct *mm, unsigned long addr) return mtree_load(&mm->mm_mt, addr); } +unsigned long stack_guard_start_gap(struct vm_area_struct *vma); + static inline unsigned long vm_start_gap(struct vm_area_struct *vma) { + unsigned long gap = stack_guard_start_gap(vma); unsigned long vm_start = vma->vm_start; - if (vma->vm_flags & VM_GROWSDOWN) { - vm_start -= stack_guard_gap; - if (vm_start > vma->vm_start) - vm_start = 0; - } + vm_start -= gap; + if (vm_start > vma->vm_start) + vm_start = 0; return vm_start; }diff --git a/mm/mmap.c b/mm/mmap.c index 2def55555e05..f67606fbc464 100644 --- a/mm/mmap.c +++ b/mm/mmap.c@@ -281,6 +281,13 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) return origbrk; } +unsigned long __weak stack_guard_start_gap(struct vm_area_struct *vma) +{ + if (vma->vm_flags & VM_GROWSDOWN) + return stack_guard_gap; + return 0; +}
I'm thinking perhaps this wants to be an inline function?