* [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare()
@ 2026-10-10 2:10 jianyungao89
2026-10-10 2:24 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: jianyungao89 @ 2026-10-10 2:10 UTC (permalink / raw)
To: linux-kernel
Cc: Jianyun Gao, Dan Williams, Matthew Wilcox, Jan Kara,
Alexander Viro, Christian Brauner,
open list:FILESYSTEM DIRECT ACCESS (DAX),
open list:FILESYSTEM DIRECT ACCESS (DAX)
From: Jianyun Gao <jianyungao89@gmail.com>
When dax_range_compare_iter() returns a negative error code (e.g. -EIO),
dax_dedupe_file_range_compare() incorrectly returns 'ret' which holds the
positive return value from iomap_iter() indicating more iterations remain.
This positive value leaks to the caller, where vfs_dedupe_file_range()
interprets it as success (bytes_deduped = len), causing userspace to
incorrectly believe deduplication completed.
Return 'status' instead to propagate the actual error code.
Signed-off-by: Jianyun Gao <jianyungao89@gmail.com>
---
fs/dax.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dax.c b/fs/dax.c
index 6ba50142eeb2..1f6a4fbb97bd 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -2261,7 +2261,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;
src_iter.status = dst_iter.status = status;
}
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() 2026-10-10 2:10 [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() jianyungao89 @ 2026-10-10 2:24 ` sashiko-bot 2026-10-10 6:28 ` Jianyun Gao 2026-10-12 9:47 ` Jan Kara 2026-10-12 13:40 ` Markus Elfring 2 siblings, 1 reply; 6+ messages in thread From: sashiko-bot @ 2026-10-10 2:24 UTC (permalink / raw) To: jianyungao89; +Cc: nvdimm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() 2026-10-10 2:24 ` sashiko-bot @ 2026-10-10 6:28 ` Jianyun Gao 0 siblings, 0 replies; 6+ messages in thread From: Jianyun Gao @ 2026-10-10 6:28 UTC (permalink / raw) To: sashiko-reviews; +Cc: nvdimm 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 <sashiko-bot@kernel.org> 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 <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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() 2026-10-10 2:10 [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() jianyungao89 2026-10-10 2:24 ` sashiko-bot @ 2026-10-12 9:47 ` Jan Kara 2026-10-12 13:40 ` Markus Elfring 2 siblings, 0 replies; 6+ messages in thread From: Jan Kara @ 2026-10-12 9:47 UTC (permalink / raw) To: jianyungao89 Cc: linux-kernel, Dan Williams, Matthew Wilcox, Jan Kara, Alexander Viro, Christian Brauner, open list:FILESYSTEM DIRECT ACCESS (DAX), open list:FILESYSTEM DIRECT ACCESS (DAX) On Sat 10-10-26 10:10:09, jianyungao89@gmail.com wrote: > From: Jianyun Gao <jianyungao89@gmail.com> > > When dax_range_compare_iter() returns a negative error code (e.g. -EIO), > dax_dedupe_file_range_compare() incorrectly returns 'ret' which holds the > positive return value from iomap_iter() indicating more iterations remain. > This positive value leaks to the caller, where vfs_dedupe_file_range() > interprets it as success (bytes_deduped = len), causing userspace to > incorrectly believe deduplication completed. > > Return 'status' instead to propagate the actual error code. > > Signed-off-by: Jianyun Gao <jianyungao89@gmail.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/dax.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/dax.c b/fs/dax.c > index 6ba50142eeb2..1f6a4fbb97bd 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -2261,7 +2261,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; > src_iter.status = dst_iter.status = status; > } > return ret; > -- > 2.34.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() 2026-10-10 2:10 [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() jianyungao89 2026-10-10 2:24 ` sashiko-bot 2026-10-12 9:47 ` Jan Kara @ 2026-10-12 13:40 ` Markus Elfring 2026-10-12 14:13 ` Jianyun Gao 2 siblings, 1 reply; 6+ messages in thread From: Markus Elfring @ 2026-10-12 13:40 UTC (permalink / raw) To: Jianyun Gao, linux-fsdevel, nvdimm Cc: LKML, Alexander Viro, Christian Brauner, Dan Williams, Jan Kara, Matthew Wilcox … > Return 'status' instead to propagate the actual error code. How do you think about to add any tags (like “Fixes” and “Cc”) accordingly? See also: * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.3-rc7#n145 * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.3-rc7#n34 Regards, Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() 2026-10-12 13:40 ` Markus Elfring @ 2026-10-12 14:13 ` Jianyun Gao 0 siblings, 0 replies; 6+ messages in thread From: Jianyun Gao @ 2026-10-12 14:13 UTC (permalink / raw) To: Markus Elfring Cc: linux-fsdevel, nvdimm, LKML, Alexander Viro, Christian Brauner, Dan Williams, Jan Kara, Matthew Wilcox Hi Markus, Thank you for the suggestion! I will add the "Fixes" and "Cc" tags in the next version. Best regards, Jianyun On Mon, Oct 12, 2026 at 9:40 PM Markus Elfring <Markus.Elfring@web.de> wrote: > > … > > Return 'status' instead to propagate the actual error code. > > How do you think about to add any tags (like “Fixes” and “Cc”) accordingly? > > See also: > * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.3-rc7#n145 > * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.3-rc7#n34 > > > Regards, > Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-12 14:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-10 2:10 [PATCH] dax: fix wrong error code returned from dax_dedupe_file_range_compare() jianyungao89 2026-10-10 2:24 ` sashiko-bot 2026-10-10 6:28 ` Jianyun Gao 2026-10-12 9:47 ` Jan Kara 2026-10-12 13:40 ` Markus Elfring 2026-10-12 14:13 ` Jianyun Gao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.