[PATCH] mm/truncate: unmap DAX entries before invalidating them

[PATCH] mm/truncate: unmap DAX entries before invalidating them

[Zi Yan]

invalidate_inode_pages2_range() invalidates the clean DAX entries in the
range through dax_invalidate_mapping_entry_sync() and only unmaps the range
afterwards. Dropping the last association of an entry releases its folio
by dissolving the folio into order-0 ones. If the entry is still mapped,
1. dax_folio_put() warns;
2. if the folio order is PMD, folio_remove_rmap_pmd() during the unmap
   afterwards operates on an order-0 folio (the original head page) and
   corrupts the original first tail page metadata.

This is reachable from userspace with a copy-on-write write to a mapped
reflinked file on XFS with DAX:

  WARNING: fs/dax.c:452 at dax_disassociate_entry.isra.0+0x147/0x1c0
  Call Trace:
   __dax_invalidate_entry+0x106/0x320
   invalidate_inode_pages2_range+0x3ce/0x830
   dax_iomap_rw+0x421/0xcc0
   xfs_file_dax_write+0x252/0x9e0
   vfs_write+0x53a/0xe70
   __x64_sys_pwrite64+0x195/0x250
   ...
  WARNING: ./include/linux/rmap.h:351 at folio_remove_rmap_pmd+0x8b2/0xaf0
  Call Trace:
   split_huge_pmd_locked+0x751/0x2350
   __split_huge_pmd+0x1bb/0x310
   ...
   unmap_mapping_pages+0x133/0x150
   invalidate_inode_pages2_range+0x700/0x830
   dax_iomap_rw+0x421/0xcc0
   ...

To fix it, unmap DAX ranges before invalidating them, as the page cache
path does for mapped folios. In __dax_invalidate_entry(), when
invalidating, skip an entry if the code is dropping its last association
but its folio still has outstanding references. -EBUSY is returned for the
skipped entries. The unmap before invalidation works because any remap of
the folio between the unmap and invalidation will bump the folio refcount.

Fixes: 38607c62b34b ("fs/dax: properly refcount fs dax pages")
Cc: stable@vger.kernel.org
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
I found this while auditing the paths that map pages into user page
tables. invalidate_inode_pages2_range() releases the folios of DAX
entries that are still mapped. For a PMD-mapped folio this corrupts what
used to be its first tail page, and a copy-on-write write to a mapped
reflinked file on XFS with DAX is enough to hit it.

In QEMU with emulated NVDIMMs and XFS on an fsdax namespace:

- Copy-on-write with the script below, PTE- and PMD-mapped, with only a
  mapped and with a and b mapped. Before this patch, the WARNs in the
  commit message and a corrupted first tail page in the PMD case. With this
  patch: the mapping of a sees the new data, b keeps the old data, and
  nothing warns, except for item 4 below.

  mkfs.xfs -f -m reflink=1 -d su=2m,sw=1 /dev/pmem1
  mount -o dax=always /dev/pmem1 /mnt && cd /mnt
  xfs_io -f -c "extsize 2m" -c "pwrite -q -S 0x5a 0 4m" -c fsync a
  xfs_io -f -c "reflink a 0 0 4m" b
  (echo "mmap -r 0 4m"; echo "madvise -h 0 4m"; echo "mread 0 4m";
   sleep 4; echo "mread -v 0 16") | xfs_io a &
  sleep 2; xfs_io -c "pwrite -q -S 0xa5 0 8k" -c fsync a; wait

Open issues that this patch does not address. They are races or
already exist without it.

1. A refault, or a short-term or speculative reference, between the
   early unmap and the invalidation keeps a last association in place,
   and invalidate_inode_pages2_range() returns -EBUSY.
   dax_iomap_iter(), dax_unshare_iter() and dax_zero_iter() ignore that
   and write the new extent anyway, which leaves a stale entry. Without
   this patch the same race tears down the referenced folio. Should
   these paths retry, or fail?

2. XFS device pre-removal ignores the result as well, so an entry kept
   because of a short-term reference is not revisited before the
   memmap goes away.

3. folio->share is updated under each file's i_pages lock, so the
   shared-count check here, like dax_folio_put() itself, can race with
   another file dropping its share.

4. With reflink, after a copy-on-write write, a PTE fault can associate
   an order-0 entry with a shared folio that another file still maps
   with a PMD. That trips WARN_ON_ONCE(dax_entry_order(entry) !=
   folio_order(folio)) in dax_associate_entry(), with and without this
   patch.

Based on an LLM's analysis, 1 and 2 can be fixed by holding the invalidate
lock and breaking DAX layouts in the callers; 3 needs a per-folio lock
for ->share; 4 needs a way to share a folio at different granularities,
or no PMD mappings of shared extents as a simple fix.

LLMs were used to analyze the issue, write the patch, and review the code.
---
 fs/dax.c      | 28 ++++++++++++++++++++++++++--
 mm/truncate.c | 16 +++++++++++-----
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 94ba41273347c..e448552d8080d 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -527,6 +527,24 @@ static struct page *dax_busy_page(void *entry)
 		return NULL;
 }
 
+/*
+ * Determine if an entry can be disassociated. It cannot be if it is its folio's
+ * last association and the folio still has a reference.
+ */
+static bool dax_can_disassociate(void *entry)
+{
+	struct folio *folio;
+
+	if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry))
+		return true;
+
+	folio = dax_to_folio(entry);
+	if (dax_folio_is_shared(folio) && folio->share > 1)
+		return true;
+
+	return !folio_ref_count(folio);
+}
+
 /**
  * dax_lock_folio - Lock the DAX entry corresponding to a folio
  * @folio: The folio whose entry we want to lock
@@ -853,9 +871,14 @@ static int __dax_invalidate_entry(struct address_space *mapping,
 	entry = get_next_unlocked_entry(&xas, 0);
 	if (!entry || WARN_ON_ONCE(!xa_is_value(entry)))
 		goto out;
+	/*
+	 * Removing the last association of a folio releases the folio, so make
+	 * sure the folio has no reference.
+	 */
 	if (!trunc &&
 	    (xas_get_mark(&xas, PAGECACHE_TAG_DIRTY) ||
-	     xas_get_mark(&xas, PAGECACHE_TAG_TOWRITE)))
+	     xas_get_mark(&xas, PAGECACHE_TAG_TOWRITE) ||
+	     !dax_can_disassociate(entry)))
 		goto out;
 	dax_disassociate_entry(entry, mapping, trunc);
 	xas_store(&xas, NULL);
@@ -1017,7 +1040,8 @@ void dax_break_layout_final(struct inode *inode)
 EXPORT_SYMBOL_GPL(dax_break_layout_final);
 
 /*
- * Invalidate DAX entry if it is clean.
+ * Invalidate DAX entry if it is clean and dropping it would not release a folio
+ * that is still in use.
  */
 int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
 				      pgoff_t index)
diff --git a/mm/truncate.c b/mm/truncate.c
index 9e9bf029635cf..b03cd99c4bba8 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -732,6 +732,15 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
 	if (mapping_empty(mapping))
 		return 0;
 
+	/*
+	 * Invalidating a DAX entry can release its folio, so unmap DAX entries
+	 * before invalidating them. dax_invalidate_mapping_entry_sync() skips
+	 * the last association of a folio that still has references, e.g.
+	 * because it was faulted in again in the meantime.
+	 */
+	if (dax_mapping(mapping))
+		unmap_mapping_pages(mapping, start, end - start + 1, false);
+
 	folio_batch_init(&fbatch);
 	index = start;
 	while (find_get_entries(mapping, &index, end, &fbatch, indices)) {
@@ -787,11 +796,8 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
 		cond_resched();
 	}
 	/*
-	 * For DAX we invalidate page tables after invalidating page cache.  We
-	 * could invalidate page tables while invalidating each entry however
-	 * that would be expensive. And doing range unmapping before doesn't
-	 * work as we have no cheap way to find whether page cache entry didn't
-	 * get remapped later.
+	 * For DAX, also zap the mappings that were faulted in while we were
+	 * invalidating.
 	 */
 	if (dax_mapping(mapping)) {
 		unmap_mapping_pages(mapping, start, end - start + 1, false);

---
base-commit: 63919467fb5239ab824de6a7a128b882055b24af
change-id: 20261103-fix-dax-invalidate-mapped-d0c2296f5543

Best regards,
--  
Yan, Zi