[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] mm/truncate: unmap full DAX_PMD range in
  invalidate_inode_pages2_range()
- [High] fs/dax: premature disassociation of shared folios in
  dax_can_disassociate()
- [High] fs/dax: stale i_pages entry when __dax_invalidate_entry() skips
  busy folio
- [Medium] dax: cross-file TOCTOU and data race on folio->share in
  dax_can_disassociate()
--

commit cd725b6545b68d39f6ee84c70fd91581edad91be
Author: Zi Yan <ziy@nvidia.com>

mm/truncate: unmap DAX entries before invalidating them

This commit unmaps DAX ranges before invalidating entries in
invalidate_inode_pages2_range() and skips disassociating clean DAX entries
whose last association still has elevated folio references.

> 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);
> +}

[Severity: High]
Can returning true whenever folio->share > 1 allow a shared PMD folio to be
dissolved while it is still PMD-mapped?

During a CoW write on file A, xfs_file_dax_write() holds XFS_IOLOCK_EXCL
without XFS_MMAPLOCK_EXCL, so a read fault on file A can refault the shared
PMD between the initial unmap_mapping_pages() and __dax_invalidate_entry():

CPU0 (CoW write on file A)        CPU1 (fault on A / truncate on B)
invalidate_inode_pages2_range(A)
  unmap_mapping_pages(A)
                                  xfs_dax_read_fault(A)
                                    dax_fault_iter()
                                      vmf_insert_folio_pmd()
                                      (refcount == 1, mapcount == 1)
  __dax_invalidate_entry(A)
    dax_can_disassociate()
      folio->share == 2 -> true
    dax_disassociate_entry()
      --folio->share (now 1)
                                  dax_break_layout(B)
                                    unmap_mapping_pages(B)
                                    dax_busy_page() -> NULL
                                      (refcount - mapcount == 0)
                                    dax_delete_mapping_entry(B)
                                      --folio->share (now 0)
                                      dax_folio_reset_order()
  unmap_mapping_pages(A)
    folio_remove_rmap_pmd()  <-- corrupts dissolved tail page

In addition, if file A holds a GUP pin on the shared folio,
dax_can_disassociate() removes the entry from file A's i_pages while
folio->share > 1, so a later dax_break_layout() on file A will no longer find
the entry in i_pages or wait for the GUP pin.

[Severity: Medium]
Can concurrent invalidations on two files sharing a folio race on
folio->share in dax_can_disassociate()?

__dax_invalidate_entry() only holds the current mapping's i_pages lock:

CPU0 (file A)                     CPU1 (file B)
__dax_invalidate_entry(A)         __dax_invalidate_entry(B)
  xas_lock_irq(&A->i_pages)         xas_lock_irq(&B->i_pages)
  dax_can_disassociate()            dax_can_disassociate()
    folio->share == 2 -> true         folio->share == 2 -> true
  dax_folio_put()                   dax_folio_put()
    --folio->share                    --folio->share (reaches 0)
                                      dax_folio_reset_order()  <-- folio busy

Both callers bypass the folio_ref_count(folio) check when folio->share == 2
and then non-atomically decrement folio->share in dax_folio_put(), which can
either corrupt folio->share (a data race) or dissolve a referenced compound
folio when the second decrement reaches 0.

> @@ -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);

[Severity: High]
What happens when dax_can_disassociate() returns false during a CoW write and
leaves the old DAX entry in mapping->i_pages?

If the folio has an elevated refcount (for example from a short-term GUP pin
or a racing fault), __dax_invalidate_entry() skips the entry and
invalidate_inode_pages2_range() returns -EBUSY.  dax_iomap_iter(),
dax_unshare_iter() and dax_zero_iter() ignore that return value and complete
the CoW remap to a new unshared extent while the old entry remains in
mapping->i_pages.

On a subsequent fault to the now-unshared extent (shared == false),
dax_insert_entry() sees a non-empty entry and neither replaces it nor calls
dax_associate_entry() on the new PFN:

fs/dax.c:dax_insert_entry() {
    ...
	if (shared || dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
		struct folio *folio = pfn_folio(pfn);
    ...
}

dax_fault_iter() then takes a reference on the old folio from *entry while
passing the uninitialized, zero-refcount new PFN to vmf_insert_folio_pmd() or
vmf_insert_page_mkwrite(), which fails with VM_FAULT_SIGBUS.  Meanwhile, the
stale entry left in the first file's i_pages still points to the physical
folio now owned exclusively by the other reflinked file, so truncating or
evicting the first file later calls dax_folio_put() ->
dax_folio_reset_order() on the second file's live PMD folio.

[ ... ]

> 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;

[Severity: High]
Does unmapping only [start, end] leave PTE mappings in place when [start, end]
partially overlaps an order-9 DAX_PMD entry in mapping->i_pages?

grab_mapping_entry() and dax_insert_entry() keep an existing storage-backed
DAX_PMD entry in mapping->i_pages when servicing PTE faults (such as in a
smaller or unaligned VMA).  When a sub-PMD write calls
invalidate_inode_pages2_range(mapping, start, end), unmap_mapping_pages()
only unmaps [start, end], whereas dax_invalidate_mapping_entry_sync() ->
__dax_invalidate_entry() disassociates and removes the entire 2MB DAX_PMD
entry covering [index & ~PG_PMD_COLOUR, index | PG_PMD_COLOUR].

If the PMD folio is not shared (or this is its last share), the un-zapped PTEs
outside [start, end] keep folio_ref_count(folio) > 0, so
dax_can_disassociate() deterministically fails with -EBUSY and leaves the
stale DAX_PMD entry in i_pages after CoW remapping.  If folio->share > 1,
dax_can_disassociate() succeeds and drops this file's share while those PTEs
outside [start, end] remain mapped, so when the other file drops the last
share, dax_folio_put() dissolves the order-9 folio while its tail pages are
still mapped via PTEs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-fix-dax-invalidate-mapped-v1-1-292ce5958d67@nvidia.com?part=1