[PATCH v2] dax: fix dax_range_compare_iter() to use correct size and address offset

[PATCH v2] dax: fix dax_range_compare_iter() to use correct size and address offset

[jianyungao89]

From: Jianyun Gao <jianyungao89@gmail.com>

dax_range_compare_iter() has two bugs in how it calls
dax_iomap_direct_access() and uses the returned addresses:

1. The size argument passed to dax_iomap_direct_access() is
   ALIGN(pos + len, PAGE_SIZE), which is an absolute file offset
   rather than a length. This causes the underlying dax_direct_access()
   to request a massively inflated nr_pages via PHYS_PFN(size).

2. The addresses returned by dax_iomap_direct_access() are page-aligned
   starting addresses, but memcmp() compares from saddr/daddr directly
   without adding the page-in-page offset of pos1/pos2. When pos1/pos2
   are not page-aligned, this compares the wrong data.

Fix the size by computing ALIGN(len + offset_in_page(pos), PAGE_SIZE),
matching the pattern used in dax_iomap_copy_around(). Fix the address
by adding offset_in_page(pos1)/offset_in_page(pos2) to saddr/daddr in
the memcmp() call.

Fixes: 6f7db3894ae2 ("fsdax: dedup file range to use a compare function")
Cc: <stable@vger.kernel.org>
Signed-off-by: Jianyun Gao <jianyungao89@gmail.com>
---

Changes in v2:
- Add offset_in_page(pos1)/offset_in_page(pos2) to the saddr/daddr
  arguments in memcmp() to fix the page-in-page offset issue pointed
  out in the v1 review.
- Use offset_in_page() macro instead of (pos & ~PAGE_MASK) for
  consistency with the rest of the codebase.

v1: https://lore.kernel.org/lkml/20261010071453.6028-1-jianyungao89@gmail.com/

 fs/dax.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 6ba50142eeb2..b08d408418bd 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -2197,6 +2197,7 @@ static int dax_range_compare_iter(struct iomap_iter *it_src,
 	const struct iomap *dmap = &it_dest->iomap;
 	loff_t pos1 = it_src->pos, pos2 = it_dest->pos;
 	void *saddr, *daddr;
+	size_t size1, size2;
 	int id, ret;
 
 	len = min(len, min(smap->length, dmap->length));
@@ -2212,17 +2213,18 @@ static int dax_range_compare_iter(struct iomap_iter *it_src,
 	}
 
 	id = dax_read_lock();
-	ret = dax_iomap_direct_access(smap, pos1, ALIGN(pos1 + len, PAGE_SIZE),
-				      &saddr, NULL);
+	size1 = ALIGN(len + offset_in_page(pos1), PAGE_SIZE);
+	ret = dax_iomap_direct_access(smap, pos1, size1, &saddr, NULL);
 	if (ret < 0)
 		goto out_unlock;
 
-	ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
-				      &daddr, NULL);
+	size2 = ALIGN(len + offset_in_page(pos2), PAGE_SIZE);
+	ret = dax_iomap_direct_access(dmap, pos2, size2, &daddr, NULL);
 	if (ret < 0)
 		goto out_unlock;
 
-	*same = !memcmp(saddr, daddr, len);
+	*same = !memcmp(saddr + offset_in_page(pos1),
+			daddr + offset_in_page(pos2), len);
 	if (!*same)
 		len = 0;
 	dax_read_unlock(id);
-- 
2.34.1

Re: [PATCH v2] dax: fix dax_range_compare_iter() to use correct size and address offset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] fs/dax: memcmp on IOMAP_UNWRITTEN exposes uninitialized PMEM data via dedupe
- [High] fsdax: CPU soft lockup due to unbounded memcmp in dedupe
- [High] fs/dax: dax_dedupe_file_range_compare() drops error and returns false dedup success
--

commit 035a3cbae242fa7d9d78dcd078b21ab3eeaecc67
Author: Jianyun Gao <jianyungao89@gmail.com>

dax: fix dax_range_compare_iter() to use correct size and address offset

This patch fixes two bugs in dax_range_compare_iter() when calling
dax_iomap_direct_access(). It corrects the size argument to use a length
rather than an absolute file offset, and it adds the page-in-page offset to
the source and destination addresses before comparing them with memcmp().

> diff --git a/fs/dax.c b/fs/dax.c
> index 08adeaed2baf1..7bee4314f905b 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -2190,6 +2190,7 @@ static int dax_range_compare_iter(struct iomap_iter *it_src,
>  	const struct iomap *dmap = &it_dest->iomap;
>  	loff_t pos1 = it_src->pos, pos2 = it_dest->pos;
>  	void *saddr, *daddr;
> +	size_t size1, size2;
>  	int id, ret;
>  
>  	len = min(len, min(smap->length, dmap->length));

[ ... ]

> @@ -2205,17 +2206,18 @@ static int dax_range_compare_iter(struct iomap_iter *it_src,
>  	}
>  
>  	id = dax_read_lock();
> -	ret = dax_iomap_direct_access(smap, pos1, ALIGN(pos1 + len, PAGE_SIZE),
> -				      &saddr, NULL);
> +	size1 = ALIGN(len + offset_in_page(pos1), PAGE_SIZE);
> +	ret = dax_iomap_direct_access(smap, pos1, size1, &saddr, NULL);
>  	if (ret < 0)
>  		goto out_unlock;

[Severity: High]
If dax_iomap_direct_access() fails here (e.g., from an I/O error or a size
issue) and returns a negative error code, does the upstream caller
dax_dedupe_file_range_compare() incorrectly mask the failure and report false
success?

When this function returns a negative error code,
dax_dedupe_file_range_compare() handles it like this:

fs/dax.c:dax_dedupe_file_range_compare() {
    ...
		status = dax_range_compare_iter(&src_iter, &dst_iter,
				min(src_iter.len, dst_iter.len), same);
		if (status < 0)
			return ret;
    ...
}

Because ret holds a positive bytes count from the loop's earlier iomap_iter()
call, a positive value is propagated back to vfs_dedupe_file_range(). 

The VFS intercepts the positive return as a success flag, bypasses block
remapping, and reports a successful deduplication result to userspace even
though the remapping aborted early.

>  
> -	ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
> -				      &daddr, NULL);
> +	size2 = ALIGN(len + offset_in_page(pos2), PAGE_SIZE);
> +	ret = dax_iomap_direct_access(dmap, pos2, size2, &daddr, NULL);
>  	if (ret < 0)
>  		goto out_unlock;
>  
> -	*same = !memcmp(saddr, daddr, len);
> +	*same = !memcmp(saddr + offset_in_page(pos1),
> +			daddr + offset_in_page(pos2), len);

[Severity: High]
Does this code expose uninitialized PMEM data if the extent is
IOMAP_UNWRITTEN?

While dax_range_compare_iter() explicitly checks for and skips IOMAP_HOLE
earlier in the function, it fails to check for IOMAP_UNWRITTEN.

As a result, the physical PMEM blocks for unwritten extents (e.g., from
fallocate) are mapped by dax_iomap_direct_access() without zero-filling
them.

This memcmp() then directly compares this uninitialized data against the
target extent.

Since early-exit from memcmp() is measurable, could this be used as a
timing side-channel to extract previous persistent memory contents, as well
as causing functional deduplication failures when comparing logically-zero
unwritten extents?

[Severity: High]
Can this memcmp() cause a CPU soft lockup and RCU stall?

When called via the FIDEDUPERANGE ioctl, vfs_dedupe_file_range() can cap the
deduplication length to 1GB.

Because DAX/PMEM mappings can be contiguous for gigabytes, this length could
be passed directly to a single synchronous memcmp() call here without any
cond_resched() yielding points.

>  	if (!*same)
>  		len = 0;
>  	dax_read_unlock(id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011064840.1296-1-jianyungao89@gmail.com?part=1

Re: [PATCH v2] dax: fix dax_range_compare_iter() to use correct size and address offset

[Jianyun Gao]

Regarding the first issue (`dax_dedupe_file_range_compare()` drops
error and returns false dedup success), this is already addressed by a
separate patch that changes `return ret` to `return status`:
https://lore.kernel.org/lkml/20261010021010.19575-1-jianyungao89@gmail.com/

Regarding the second issue (memcmp on IOMAP_UNWRITTEN exposes
uninitialized PMEM data), this is a pre-existing issue unrelated to
the current patch. The current patch only fixes the size and address
offset bugs and does not change the handling of IOMAP_UNWRITTEN.

Regarding the third issue (CPU soft lockup due to unbounded memcmp),
this is also a pre-existing concern. `dax_range_compare_iter()`
operates within an iomap iteration that bounds each iteration's length
to a single iomap mapping via `len = min(len, min(smap->length,
dmap->length))`, so memcmp typically does not run across the entire
1GB dedupe request.

On Sun, Oct 11, 2026 at 3:09 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] fs/dax: memcmp on IOMAP_UNWRITTEN exposes uninitialized PMEM data via dedupe
> - [High] fsdax: CPU soft lockup due to unbounded memcmp in dedupe
> - [High] fs/dax: dax_dedupe_file_range_compare() drops error and returns false dedup success
> --
>
> commit 035a3cbae242fa7d9d78dcd078b21ab3eeaecc67
> Author: Jianyun Gao <jianyungao89@gmail.com>
>
> dax: fix dax_range_compare_iter() to use correct size and address offset
>
> This patch fixes two bugs in dax_range_compare_iter() when calling
> dax_iomap_direct_access(). It corrects the size argument to use a length
> rather than an absolute file offset, and it adds the page-in-page offset to
> the source and destination addresses before comparing them with memcmp().
>
> > diff --git a/fs/dax.c b/fs/dax.c
> > index 08adeaed2baf1..7bee4314f905b 100644
> > --- a/fs/dax.c
> > +++ b/fs/dax.c
> > @@ -2190,6 +2190,7 @@ static int dax_range_compare_iter(struct iomap_iter *it_src,
> >       const struct iomap *dmap = &it_dest->iomap;
> >       loff_t pos1 = it_src->pos, pos2 = it_dest->pos;
> >       void *saddr, *daddr;
> > +     size_t size1, size2;
> >       int id, ret;
> >
> >       len = min(len, min(smap->length, dmap->length));
>
> [ ... ]
>
> > @@ -2205,17 +2206,18 @@ static int dax_range_compare_iter(struct iomap_iter *it_src,
> >       }
> >
> >       id = dax_read_lock();
> > -     ret = dax_iomap_direct_access(smap, pos1, ALIGN(pos1 + len, PAGE_SIZE),
> > -                                   &saddr, NULL);
> > +     size1 = ALIGN(len + offset_in_page(pos1), PAGE_SIZE);
> > +     ret = dax_iomap_direct_access(smap, pos1, size1, &saddr, NULL);
> >       if (ret < 0)
> >               goto out_unlock;
>
> [Severity: High]
> If dax_iomap_direct_access() fails here (e.g., from an I/O error or a size
> issue) and returns a negative error code, does the upstream caller
> dax_dedupe_file_range_compare() incorrectly mask the failure and report false
> success?
>
> When this function returns a negative error code,
> dax_dedupe_file_range_compare() handles it like this:
>
> fs/dax.c:dax_dedupe_file_range_compare() {
>     ...
>                 status = dax_range_compare_iter(&src_iter, &dst_iter,
>                                 min(src_iter.len, dst_iter.len), same);
>                 if (status < 0)
>                         return ret;
>     ...
> }
>
> Because ret holds a positive bytes count from the loop's earlier iomap_iter()
> call, a positive value is propagated back to vfs_dedupe_file_range().
>
> The VFS intercepts the positive return as a success flag, bypasses block
> remapping, and reports a successful deduplication result to userspace even
> though the remapping aborted early.
>
> >
> > -     ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
> > -                                   &daddr, NULL);
> > +     size2 = ALIGN(len + offset_in_page(pos2), PAGE_SIZE);
> > +     ret = dax_iomap_direct_access(dmap, pos2, size2, &daddr, NULL);
> >       if (ret < 0)
> >               goto out_unlock;
> >
> > -     *same = !memcmp(saddr, daddr, len);
> > +     *same = !memcmp(saddr + offset_in_page(pos1),
> > +                     daddr + offset_in_page(pos2), len);
>
> [Severity: High]
> Does this code expose uninitialized PMEM data if the extent is
> IOMAP_UNWRITTEN?
>
> While dax_range_compare_iter() explicitly checks for and skips IOMAP_HOLE
> earlier in the function, it fails to check for IOMAP_UNWRITTEN.
>
> As a result, the physical PMEM blocks for unwritten extents (e.g., from
> fallocate) are mapped by dax_iomap_direct_access() without zero-filling
> them.
>
> This memcmp() then directly compares this uninitialized data against the
> target extent.
>
> Since early-exit from memcmp() is measurable, could this be used as a
> timing side-channel to extract previous persistent memory contents, as well
> as causing functional deduplication failures when comparing logically-zero
> unwritten extents?
>
> [Severity: High]
> Can this memcmp() cause a CPU soft lockup and RCU stall?
>
> When called via the FIDEDUPERANGE ioctl, vfs_dedupe_file_range() can cap the
> deduplication length to 1GB.
>
> Because DAX/PMEM mappings can be contiguous for gigabytes, this length could
> be passed directly to a single synchronous memcmp() call here without any
> cond_resched() yielding points.
>
> >       if (!*same)
> >               len = 0;
> >       dax_read_unlock(id);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261011064840.1296-1-jianyungao89@gmail.com?part=1