Re: [PATCH v3 25/28] userfaultfd: wp: fixup swap entries in change_pte_range

From: Jerome Glisse
Date: Thu Apr 18 2019 - 17:01:58 EST


On Wed, Mar 20, 2019 at 10:06:39AM +0800, Peter Xu wrote:
> In change_pte_range() we do nothing for uffd if the PTE is a swap
> entry. That can lead to data mismatch if the page that we are going
> to write protect is swapped out when sending the UFFDIO_WRITEPROTECT.
> This patch applies/removes the uffd-wp bit even for the swap entries.
>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>

This one seems to address some of the comments i made on patch 17
not all thought. Maybe squash them together ?

> ---
>
> I kept this patch a standalone one majorly to make review easier. The
> patch can be considered as standalone or to squash into the patch
> "userfaultfd: wp: support swap and page migration".
> ---
> mm/mprotect.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 96c0f521099d..a23e03053787 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -183,11 +183,11 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> }
> ptep_modify_prot_commit(mm, addr, pte, ptent);
> pages++;
> - } else if (IS_ENABLED(CONFIG_MIGRATION)) {
> + } else if (is_swap_pte(oldpte)) {
> swp_entry_t entry = pte_to_swp_entry(oldpte);
> + pte_t newpte;
>
> if (is_write_migration_entry(entry)) {
> - pte_t newpte;
> /*
> * A protection check is difficult so
> * just be safe and disable write
> @@ -198,22 +198,24 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> newpte = pte_swp_mksoft_dirty(newpte);
> if (pte_swp_uffd_wp(oldpte))
> newpte = pte_swp_mkuffd_wp(newpte);
> - set_pte_at(mm, addr, pte, newpte);
> -
> - pages++;
> - }
> -
> - if (is_write_device_private_entry(entry)) {
> - pte_t newpte;
> -
> + } else if (is_write_device_private_entry(entry)) {
> /*
> * We do not preserve soft-dirtiness. See
> * copy_one_pte() for explanation.
> */
> make_device_private_entry_read(&entry);
> newpte = swp_entry_to_pte(entry);
> - set_pte_at(mm, addr, pte, newpte);
> + } else {
> + newpte = oldpte;
> + }
>
> + if (uffd_wp)
> + newpte = pte_swp_mkuffd_wp(newpte);
> + else if (uffd_wp_resolve)
> + newpte = pte_swp_clear_uffd_wp(newpte);
> +
> + if (!pte_same(oldpte, newpte)) {
> + set_pte_at(mm, addr, pte, newpte);
> pages++;
> }
> }
> --
> 2.17.1
>