[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

[Jan Kara]

On Sun 11-10-26 14:48:39, jianyungao89@gmail.com wrote:
> 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>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> 
> 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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Alison Schofield]

On Sun, Oct 11, 2026 at 02:48:39PM +0800, jianyungao89@gmail.com wrote:
> 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.


Applied to libnvdimm-for-next:
https://git.kernel.org/nvdimm/nvdimm/c/a4bfbfb4e98a


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