[PATCH] dax: fix wrong size passed to dax_iomap_direct_access() in dax_range_compare_iter()

[PATCH] dax: fix wrong size passed to dax_iomap_direct_access() in dax_range_compare_iter()

[jianyungao89]

From: Jianyun Gao <jianyungao89@gmail.com>

dax_range_compare_iter() passes ALIGN(pos + len, PAGE_SIZE) as the size
argument to dax_iomap_direct_access(), but this 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), leading to unnecessarily large access ranges and
potential spurious -EHWPOISON from driver badblock scans.

Fix by computing the correct aligned size:
  ALIGN(len + (pos & ~PAGE_MASK), PAGE_SIZE)

which accounts for the page-in-page offset of pos, matching the pattern
used in dax_iomap_copy_around().

Fixes: 6f7db3894ae2 ("fsdax: dedup file range to use a compare function")
Cc: <stable@vger.kernel.org>
Signed-off-by: Jianyun Gao <jianyungao89@gmail.com>
---
 fs/dax.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 6ba50142eeb2..213b1c9e197c 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,13 +2213,13 @@ 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 + (pos1 & ~PAGE_MASK), 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 + (pos2 & ~PAGE_MASK), PAGE_SIZE);
+	ret = dax_iomap_direct_access(dmap, pos2, size2, &daddr, NULL);
 	if (ret < 0)
 		goto out_unlock;
 
-- 
2.34.1

Re: [PATCH] dax: fix wrong size passed to dax_iomap_direct_access() in dax_range_compare_iter()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] fsdax: dax_range_compare_iter() memcmp ignores page-in-page offset
- [High] fsdax: out-of-bounds access due to bypassed length check in dax_iomap_direct_access()
--

commit 05244bba82891c8d959214ace5da854f182c9e20
Author: Jianyun Gao <jianyungao89@gmail.com>

dax: fix wrong size passed to dax_iomap_direct_access() in dax_range_compare_iter()

dax_range_compare_iter() passes ALIGN(pos + len, PAGE_SIZE) as the size
argument to dax_iomap_direct_access(), but this 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), leading to unnecessarily large access ranges and
potential spurious -EHWPOISON from driver badblock scans.

> diff --git a/fs/dax.c b/fs/dax.c
> index 08adeaed2baf1..fb12d0c0dcd49 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,13 +2206,13 @@ 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 + (pos1 & ~PAGE_MASK), PAGE_SIZE);
> +	ret = dax_iomap_direct_access(smap, pos1, size1, &saddr, NULL);

[Severity: High]
Does this code risk an out-of-bounds access in the subsequent memcmp() if the
underlying physical DAX memory mapped is shorter than requested?

Looking at dax_iomap_direct_access(), it bypasses the length validation when
the pfnp argument is NULL:

fs/dax.c:dax_iomap_direct_access() {
    ...
	if (!pfnp)
		goto out_check_addr;
	rc = -EINVAL;
	if (PFN_PHYS(length) < size)
		goto out;
    ...
}

If the iomap extent crosses a device mapper target boundary, such as on
dm-linear, dm_dax_direct_access() truncates the mapping length and returns
fewer pages than requested.

Because dax_iomap_direct_access() is called here with a NULL pfnp, it skips
the length check and returns success even for short mappings.

Could this regression cause the subsequent memcmp() to read beyond the
physically contiguous mapped memory?

>  	if (ret < 0)
>  		goto out_unlock;
>  
> -	ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
> -				      &daddr, NULL);
> +	size2 = ALIGN(len + (pos2 & ~PAGE_MASK), PAGE_SIZE);
> +	ret = dax_iomap_direct_access(dmap, pos2, size2, &daddr, NULL);
>  	if (ret < 0)
>  		goto out_unlock;
>  

[Severity: High]
Is it possible that the memcmp() that follows compares the wrong file data
because it ignores the page-in-page offset?

The dax_iomap_direct_access() function translates a file offset to a kernel
virtual address, but the returned pointers saddr and daddr are strictly
page-aligned, ignoring any page-in-page offset (pos & ~PAGE_MASK).

When filesystem block sizes are smaller than the architecture's page size,
deduplication requests can be block-aligned but not page-aligned.

If memcmp() is called directly on saddr and daddr without adding the
pos1 & ~PAGE_MASK and pos2 & ~PAGE_MASK offsets:

fs/dax.c:dax_range_compare_iter() {
    ...
	*same = !memcmp(saddr, daddr, len);
    ...
}

Will this regression compare memory starting from the page boundary instead of
the actual file offset? Could this lead to FIDEDUPERANGE incorrectly
deduplicating non-identical blocks and causing data corruption?

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

Re: [PATCH] dax: fix wrong size passed to dax_iomap_direct_access() in dax_range_compare_iter()

[Jianyun Gao]

Thank you for the review!

Regarding the first issue (memcmp ignores page-in-page offset), you
are correct. This will be fixed in the v2 version of this patch, which
adds offset_in_page(pos1)/offset_in_page(pos2) to the saddr/daddr
arguments in the memcmp() call.

Regarding the second issue (bypassed length check when pfnp is NULL),
this is a pre-existing behavior of dax_iomap_direct_access() shared by
all callers that pass a NULL pfnp (e.g., dax_iomap_copy_around(),
dax_unshare_iter()). In the dedupe path, the access range is
guaranteed to be within the iomap mapping via len = min(len,
min(smap->length, dmap->length)), and filesystem iomap mappings do not
cross device boundaries. This is outside the scope of this patch and
would be better addressed as a separate fix to
dax_iomap_direct_access() itself.

On Sat, Oct 10, 2026 at 3:35 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] fsdax: dax_range_compare_iter() memcmp ignores page-in-page offset
> - [High] fsdax: out-of-bounds access due to bypassed length check in dax_iomap_direct_access()
> --
>
> commit 05244bba82891c8d959214ace5da854f182c9e20
> Author: Jianyun Gao <jianyungao89@gmail.com>
>
> dax: fix wrong size passed to dax_iomap_direct_access() in dax_range_compare_iter()
>
> dax_range_compare_iter() passes ALIGN(pos + len, PAGE_SIZE) as the size
> argument to dax_iomap_direct_access(), but this 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), leading to unnecessarily large access ranges and
> potential spurious -EHWPOISON from driver badblock scans.
>
> > diff --git a/fs/dax.c b/fs/dax.c
> > index 08adeaed2baf1..fb12d0c0dcd49 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,13 +2206,13 @@ 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 + (pos1 & ~PAGE_MASK), PAGE_SIZE);
> > +     ret = dax_iomap_direct_access(smap, pos1, size1, &saddr, NULL);
>
> [Severity: High]
> Does this code risk an out-of-bounds access in the subsequent memcmp() if the
> underlying physical DAX memory mapped is shorter than requested?
>
> Looking at dax_iomap_direct_access(), it bypasses the length validation when
> the pfnp argument is NULL:
>
> fs/dax.c:dax_iomap_direct_access() {
>     ...
>         if (!pfnp)
>                 goto out_check_addr;
>         rc = -EINVAL;
>         if (PFN_PHYS(length) < size)
>                 goto out;
>     ...
> }
>
> If the iomap extent crosses a device mapper target boundary, such as on
> dm-linear, dm_dax_direct_access() truncates the mapping length and returns
> fewer pages than requested.
>
> Because dax_iomap_direct_access() is called here with a NULL pfnp, it skips
> the length check and returns success even for short mappings.
>
> Could this regression cause the subsequent memcmp() to read beyond the
> physically contiguous mapped memory?
>
> >       if (ret < 0)
> >               goto out_unlock;
> >
> > -     ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
> > -                                   &daddr, NULL);
> > +     size2 = ALIGN(len + (pos2 & ~PAGE_MASK), PAGE_SIZE);
> > +     ret = dax_iomap_direct_access(dmap, pos2, size2, &daddr, NULL);
> >       if (ret < 0)
> >               goto out_unlock;
> >
>
> [Severity: High]
> Is it possible that the memcmp() that follows compares the wrong file data
> because it ignores the page-in-page offset?
>
> The dax_iomap_direct_access() function translates a file offset to a kernel
> virtual address, but the returned pointers saddr and daddr are strictly
> page-aligned, ignoring any page-in-page offset (pos & ~PAGE_MASK).
>
> When filesystem block sizes are smaller than the architecture's page size,
> deduplication requests can be block-aligned but not page-aligned.
>
> If memcmp() is called directly on saddr and daddr without adding the
> pos1 & ~PAGE_MASK and pos2 & ~PAGE_MASK offsets:
>
> fs/dax.c:dax_range_compare_iter() {
>     ...
>         *same = !memcmp(saddr, daddr, len);
>     ...
> }
>
> Will this regression compare memory starting from the page boundary instead of
> the actual file offset? Could this lead to FIDEDUPERANGE incorrectly
> deduplicating non-identical blocks and causing data corruption?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261010071453.6028-1-jianyungao89@gmail.com?part=1