Re: [PATCH v2] mm/swapfile: unuse_pte can map random data if swap read fails

[Peter Xu <peterx@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

On Sat, Apr 16, 2022 at 11:05:49AM +0800, Miaohe Lin wrote:
> @@ -1797,6 +1797,17 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
>  		goto out;
>  	}
>  
> +	if (unlikely(!PageUptodate(page))) {
> +		pte_t pteval;
> +
> +		dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
> +		pteval = swp_entry_to_pte(make_swapin_error_entry(page));
> +		set_pte_at(vma->vm_mm, addr, pte, pteval);
> +		swap_free(entry);
> +		ret = 0;
> +		goto out;
> +	}
> +
>  	/* See do_swap_page() */
>  	BUG_ON(!PageAnon(page) && PageMappedToDisk(page));
>  	BUG_ON(PageAnon(page) && PageAnonExclusive(page));

Totally off-topic, but.. today when I was looking at the unuse path I just
found that the swp bits could have got lost for either soft-dirty and
uffd-wp here?  A quick patch attached.

Maybe at some point we should start to have some special helpers for
set_pte_at() when we're converting between present/non-present ptes, so as
to make sure all these will always be taken care of properly.

-- 
Peter Xu

[-- Attachment #2: 0001-mm-swap-Fix-lost-swap-bits-in-unuse_pte.patch --]
[-- Type: text/plain, Size: 1520 bytes --]

From 60ba535f73f7a7aeab2275d370bf8291cf53755e Mon Sep 17 00:00:00 2001
From: Peter Xu <peterx@redhat.com>
Date: Tue, 19 Apr 2022 17:28:34 -0400
Subject: [PATCH] mm/swap: Fix lost swap bits in unuse_pte()
Content-type: text/plain

This is observed by code review only but not any real report.

When we turn off swapping we could have lost the bits stored in the swap
ptes.  The new rmap-exclusive bit is fine since that turned into a page
flag, but not for soft-dirty and uffd-wp.  Add them.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 mm/swapfile.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 9398e915b36b..eaea7a1f000b 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1783,7 +1783,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
 {
 	struct page *swapcache;
 	spinlock_t *ptl;
-	pte_t *pte;
+	pte_t *pte, newpte;
 	int ret = 1;
 
 	swapcache = page;
@@ -1821,8 +1821,12 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
 		page_add_new_anon_rmap(page, vma, addr);
 		lru_cache_add_inactive_or_unevictable(page, vma);
 	}
-	set_pte_at(vma->vm_mm, addr, pte,
-		   pte_mkold(mk_pte(page, vma->vm_page_prot)));
+	new_pte = pte_mkold(mk_pte(page, vma->vm_page_prot));
+	if (pte_swp_soft_dirty(*pte))
+		pte = pte_mksoft_dirty(new_pte);
+	if (pte_swp_uffd_wp(*pte))
+		pte = pte_mkuffd_wp(new_pte);
+	set_pte_at(vma->vm_mm, addr, pte, new_pte);
 	swap_free(entry);
 out:
 	pte_unmap_unlock(pte, ptl);
-- 
2.32.0