Re: [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dax: dax_range_compare_iter() passes absolute offset instead of size
--

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

dax: fix wrong error code returned from dax_dedupe_file_range_compare()

This commit modifies dax_dedupe_file_range_compare() to return the actual
error status from dax_range_compare_iter() rather than leaking a positive
return value. This prevents vfs_dedupe_file_range() from misinterpreting
an error as a success.

> diff --git a/fs/dax.c b/fs/dax.c
> index 08adeaed2baf1..97b6212bfa898 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -2254,7 +2254,7 @@ int dax_dedupe_file_range_compare(struct inode *src, loff_t srcoff,
>  		status = dax_range_compare_iter(&src_iter, &dst_iter,
>  				min(src_iter.len, dst_iter.len), same);
>  		if (status < 0)
> -			return ret;
> +			return status;

[Severity: High]
Does returning status here expose a bug in dax_range_compare_iter()
where it passes an absolute file offset as a relative size?

Looking at dax_range_compare_iter():

fs/dax.c:dax_range_compare_iter() {
    ...
    ret = dax_iomap_direct_access(smap, pos1, ALIGN(pos1 + len, PAGE_SIZE),
                                  &saddr, NULL);
    ...
    ret = dax_iomap_direct_access(dmap, pos2, ALIGN(pos2 + len, PAGE_SIZE),
                                  &daddr, NULL);
    ...
}

The third argument to dax_iomap_direct_access() is a size, but it is
passed ALIGN(pos1 + len, PAGE_SIZE) and ALIGN(pos2 + len, PAGE_SIZE),
which are absolute file offsets rounded up to the page size.

For large file offsets, this requests a massive access size. The underlying
dax_direct_access() call computes nr_pages = PHYS_PFN(size), translating
the absolute offset into a huge page count.

This will overflow a 32-bit unsigned int for offsets >= 2TB. Additionally,
driver-level bad block scans like __pmem_direct_access() will iterate over
this massively inflated range.

If any hardware poison exists anywhere in the remainder of the device,
it can spuriously return -EHWPOISON. By propagating this error status to
userspace, will this cause deduplication to spuriously fail?

>  		src_iter.status = dst_iter.status = status;
>  	}
>  	return ret;

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