| Message ID | 20220929222936.14584-20-rick.p.edgecombe@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Shadowstacks for userspace | expand |
On Thu, Sep 29, 2022 at 03:29:16PM -0700, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Account shadow stack pages to stack memory. > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> Reviewed-by: Kees Cook <keescook@chromium.org>
On Thu, Sep 29, 2022 at 03:29:16PM -0700, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Account shadow stack pages to stack memory. > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Cc: Kees Cook <keescook@chromium.org> > > --- > > v2: > - Remove is_shadow_stack_mapping() and just change it to directly bitwise > and VM_SHADOW_STACK. > > Yu-cheng v26: > - Remove redundant #ifdef CONFIG_MMU. > > Yu-cheng v25: > - Remove #ifdef CONFIG_ARCH_HAS_SHADOW_STACK for is_shadow_stack_mapping(). > > mm/mmap.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/mm/mmap.c b/mm/mmap.c > index f0d2e9143bd0..8569ef09614c 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -1682,6 +1682,9 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags) > if (file && is_file_hugepages(file)) > return 0; > > + if (vm_flags & VM_SHADOW_STACK) > + return 1; > + > return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == VM_WRITE; Hm. Isn't the last check true for shadow stack too? IIUC, shadow stack has VM_WRITE set, so accountable_mapping() should work correctly as is.
On Tue, 2022-10-04 at 03:03 +0300, Kirill A . Shutemov wrote: > > + if (vm_flags & VM_SHADOW_STACK) > > + return 1; > > + > > return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == > > VM_WRITE; > > Hm. Isn't the last check true for shadow stack too? IIUC, shadow > stack has > VM_WRITE set, so accountable_mapping() should work correctly as is. They are not always VM_WRITE, that can have it removed via mprotect(). But in that case it is just specially tagged read only memory, so probably isn't accountable. So, yea, I'll remove it. Thanks.
diff --git a/mm/mmap.c b/mm/mmap.c index f0d2e9143bd0..8569ef09614c 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1682,6 +1682,9 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags) if (file && is_file_hugepages(file)) return 0; + if (vm_flags & VM_SHADOW_STACK) + return 1; + return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == VM_WRITE; } @@ -3289,6 +3292,8 @@ void vm_stat_account(struct mm_struct *mm, vm_flags_t flags, long npages) mm->exec_vm += npages; else if (is_stack_mapping(flags)) mm->stack_vm += npages; + else if (flags & VM_SHADOW_STACK) + mm->stack_vm += npages; else if (is_data_mapping(flags)) mm->data_vm += npages; }