On Thu, Apr 01, 2021 at 03:10:53PM -0700, Yu-cheng Yu wrote:
> 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 <yu-cheng.yu@xxxxxxxxx>
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> ---
> 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
