Thread (50 messages) 50 messages, 4 authors, 2021-04-10

Re: [PATCH v24 19/30] mm: Update can_follow_write_pte() for shadow stack

From: Kirill A. Shutemov <hidden>
Date: 2021-04-09 15:31:55
Also in: linux-arch, linux-doc, linux-mm, lkml

On Thu, Apr 01, 2021 at 03:10:53PM -0700, Yu-cheng Yu wrote:
quoted hunk ↗ jump to hunk
Can_follow_write_pte() ensures a read-only page is COWed by checking the
FOLL_COW flag, and uses pte_dirty() to validate the flag is still valid.

Like a writable data page, a shadow stack page is writable, and becomes
read-only during copy-on-write, but it is always dirty.  Thus, in the
can_follow_write_pte() check, it belongs to the writable page case and
should be excluded from the read-only page pte_dirty() check.  Apply
the same changes to can_follow_write_pmd().

Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
Cc: Kirill A. Shutemov <redacted>
---
v24:
- Change arch_shadow_stack_mapping() to is_shadow_stack_mapping().

 mm/gup.c         | 8 +++++---
 mm/huge_memory.c | 8 +++++---
 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index e40579624f10..c313cc988865 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -356,10 +356,12 @@ static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address,
  * FOLL_FORCE can write to even unwritable pte's, but only
  * after we've gone through a COW cycle and they are dirty.
  */
-static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
+static inline bool can_follow_write_pte(pte_t pte, unsigned int flags,
+					struct vm_area_struct *vma)
 {
 	return pte_write(pte) ||
-		((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
+		((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte) &&
+				  !is_shadow_stack_mapping(vma->vm_flags));
It's getting too ugly. I think it deserve to be rewritten. What about:

	if (pte_write(pte))
		return true;
	if ((flags & (FOLL_FORCE | FOLL_COW)) != (FOLL_FORCE | FOLL_COW))
		return false;
	if (!pte_dirty(pte))
		return false;
	if (is_shadow_stack_mapping(vma->vm_flags))
		return false;
	return true;

?

-- 
 Kirill A. Shutemov
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help