Thank you for the review! You are correct that dax_range_compare_iter() passes ALIGN(pos1 + len, PAGE_SIZE) as the size argument to dax_iomap_direct_access(), which is an absolute file offset rather than a length. This is indeed a bug that I will fix in a follow-up patch.
However, this is a pre-existing issue unrelated to the bug this patch addresses. The purpose of this patch is to fix the error path in dax_dedupe_file_range_compare(), where return ret can leak a positive value from iomap_iter() and cause vfs_dedupe_file_range_compare() to misinterpret an error as success. Changing return ret to return status ensures the actual error from dax_range_compare_iter() is correctly propagated, regardless of what that error happens to be. On Sat, Oct 10, 2026 at 10:24 AM <[email protected]> wrote: > > 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 <[email protected]> > > 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/[email protected]?part=1
