2022-09-29 22:34:21

by Edgecombe, Rick P

Subject: [PATCH v2 19/39] mm/mmap: Add shadow stack pages to memory accounting

From: Yu-cheng Yu <[email protected]>

Account shadow stack pages to stack memory.

Signed-off-by: Yu-cheng Yu <[email protected]>
Co-developed-by: Rick Edgecombe <[email protected]>
Signed-off-by: Rick Edgecombe <[email protected]>
Cc: Kees Cook <[email protected]>

---

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;
}

@@ -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;
}
--
2.17.1

2022-10-03 19:00:23

by Kees Cook

Subject: Re: [PATCH v2 19/39] mm/mmap: Add shadow stack pages to memory accounting

On Thu, Sep 29, 2022 at 03:29:16PM -0700, Rick Edgecombe wrote:
> From: Yu-cheng Yu <[email protected]>
>
> Account shadow stack pages to stack memory.
>
> Signed-off-by: Yu-cheng Yu <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2022-10-04 00:51:34

by Kirill A. Shutemov

Subject: Re: [PATCH v2 19/39] mm/mmap: Add shadow stack pages to memory accounting

On Thu, Sep 29, 2022 at 03:29:16PM -0700, Rick Edgecombe wrote:
> From: Yu-cheng Yu <[email protected]>
>
> Account shadow stack pages to stack memory.
>
> Signed-off-by: Yu-cheng Yu <[email protected]>
> Co-developed-by: Rick Edgecombe <[email protected]>
> Signed-off-by: Rick Edgecombe <[email protected]>
> Cc: Kees Cook <[email protected]>
>
> ---
>
> 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.

--
Kiryl Shutsemau / Kirill A. Shutemov

2022-10-04 01:42:57

by Edgecombe, Rick P

Subject: Re: [PATCH v2 19/39] mm/mmap: Add shadow stack pages to memory accounting

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.