* [PATCH v1 1/2] fuse: don't shorten the folio descriptor at LLONG_MAX
2026-09-04 4:56 [PATCH v1 0/2] fuse: fixes for iomap bugs reported by Sashiko Joanne Koong
@ 2026-09-04 4:56 ` Joanne Koong
2026-09-04 4:56 ` [PATCH v1 2/2] fuse: zero the correct range on a short read reply Joanne Koong
1 sibling, 0 replies; 3+ messages in thread
From: Joanne Koong @ 2026-09-04 4:56 UTC (permalink / raw)
To: miklos; +Cc: fuse-devel, Sashiko, stable
fuse_send_readpages() and fuse_do_readfolio() both decrement the folio
descriptor length when handling the overflow case where a read would
exceed LLONG_MAX after incrementing the file position by the number of
bytes that need to be read in.
Shortening it is unnecessary (and for the virtio-fs paths, a bug), and
with fuse using iomap for handling reads, shortening it is buggy.
For fuse_send_readpages(), ap->descs[] is also what fuse_readpages_end()
reports back to iomap_finish_folio_read(). iomap accounted the full
length when the range was submitted, so the lengths reported on
completion have to add up to what was submitted. Reporting one byte less
leaves ifs->read_bytes_pending nonzero, folio_end_read() is never called,
and the folio stays locked.
This is currently reachable on fuseblk mounts configured with block
sizes smaller than the page size.
Fix this by leaving the descriptor length untouched. The reply will then
be one byte shorter than what the descriptor lengths add up to, and the
last byte will be zeroed in fuse_copy_folios().
This also fixes a bug in virtio-fs that existed before any iomap changes
were added to fuse. For virtio-fs, data there arrives by DMA rather than
through fuse_copy_folios(), so the only zeroing is in
virtio_fs_request_complete(), which compares the reply size against the
descriptor length. With the descriptor length shortened, the two are
equal and nothing gets zeroed, which means the unrequested byte is left
holding whatever was in the folio, when the folio is marked uptodate.
This is both in the fuse_do_readfolio() and fuse_send_readpages() paths.
This dates back to commit 2f1398291bf3 ("fuse: don't overflow LLONG_MAX
with end offset").
Fixes: 4ea907108a5c ("fuse: use iomap for readahead")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/file.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 4bdf5dec2cb9..3bf7cb590538 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -864,18 +864,29 @@ static int fuse_do_readfolio(struct file *file, struct folio *folio,
attr_ver = fuse_get_attr_version(fm->fc);
- /* Don't overflow end offset */
- if (pos + (desc.length - 1) == LLONG_MAX)
- desc.length--;
+ /*
+ * Don't overflow end offset.
+ *
+ * Ask the server for len - 1 bytes. desc.length still holds the full
+ * length. When the reply comes back, it will be one byte shorter than
+ * desc.length and fuse_copy_folios() will zero that last byte.
+ *
+ * For this reason, desc.length must not be decremented too. The caller
+ * reports the full length to iomap_finish_folio_read(), which marks
+ * every block it covers uptodate. Shortening the descriptor would
+ * suppress zeroing and leave the last byte holding stale data.
+ */
+ if (pos + (len - 1) == LLONG_MAX)
+ len--;
- fuse_read_args_fill(&ia, file, pos, desc.length, FUSE_READ);
+ fuse_read_args_fill(&ia, file, pos, len, FUSE_READ);
res = fuse_simple_request(fm, &ia.ap.args);
if (res < 0)
return res;
/*
* Short read means EOF. If file size is larger, truncate it
*/
- if (res < desc.length)
+ if (res < len)
fuse_short_read(inode, attr_ver, res, &ia.ap);
return 0;
@@ -1068,11 +1079,20 @@ static void fuse_send_readpages(struct fuse_io_args *ia, struct file *file,
ap->args.page_zeroing = true;
ap->args.page_replace = true;
- /* Don't overflow end offset */
- if (pos + (count - 1) == LLONG_MAX) {
+ /*
+ * Don't overflow end offset.
+ *
+ * Ask the server for count - 1 bytes. The reply is then one byte
+ * shorter than what the descriptor lengths add up to, so
+ * fuse_copy_folios() zeroes the last byte when it walks the folios.
+ *
+ * ap->descs[] must not be decremented here. It is what
+ * fuse_readpages_end() reports back to iomap_finish_folio_read(), and
+ * iomap has already accounted the full descriptor length, so shortening
+ * it would leave ifs->read_bytes_pending nonzero and the folio locked.
+ */
+ if (pos + (count - 1) == LLONG_MAX)
count--;
- ap->descs[ap->num_folios - 1].length--;
- }
WARN_ON((loff_t) (pos + count) < 0);
fuse_read_args_fill(ia, file, pos, count, FUSE_READ);
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v1 2/2] fuse: zero the correct range on a short read reply
2026-09-04 4:56 [PATCH v1 0/2] fuse: fixes for iomap bugs reported by Sashiko Joanne Koong
2026-09-04 4:56 ` [PATCH v1 1/2] fuse: don't shorten the folio descriptor at LLONG_MAX Joanne Koong
@ 2026-09-04 4:56 ` Joanne Koong
1 sibling, 0 replies; 3+ messages in thread
From: Joanne Koong @ 2026-09-04 4:56 UTC (permalink / raw)
To: miklos; +Cc: fuse-devel, Sashiko, Kanishka De Silva
When a read reply is shorter than the requested range, fuse_copy_folio()
zeroes everything in the folio outside the copied bytes. However, for
folios with block sizes smaller than the folio size, this can clear
blocks outside the requested range which are uptodate, or dirty and have
not yet been written back.
Move the zeroing logic into fuse_copy_folios(), since descs[i].length
needs to be used, and clear only the tail of the requested range that
the server did not send.
The range is also no longer cleared upfront in the non
cs->skip_folio_copy case. This is fine since a failed copy leaves those
blocks non-uptodate, so the uncopied bytes are never visible. This also
matches the pre-existing behavior in the non-short-read case (a failed
copy doesn't zero out the range in the folio). This lets the zeroing
logic stay the same regardless of whether the folio copy is skipped or
not.
Fixes: a4c9ab1d4975 ("fuse: use iomap for buffered writes")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Reported-by: Kanishka De Silva <kpskanna1915@gmail.com>
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dev.c | 44 +++++++++++++++++++-------------------------
fs/fuse/dev.h | 2 +-
fs/fuse/notify.c | 2 +-
3 files changed, 21 insertions(+), 27 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 4fec31fc0b84..293b8a936e1e 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1252,31 +1252,10 @@ static int fuse_ref_folio(struct fuse_copy_state *cs, struct folio *folio,
* done atomically
*/
int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
- unsigned offset, unsigned count, int zeroing)
+ unsigned offset, unsigned count)
{
int err;
struct folio *folio = *foliop;
- size_t size;
-
- if (folio) {
- size = folio_size(folio);
- if (zeroing && count < size) {
- /*
- * When the copy is skipped the folio already holds the
- * payload, so only the bytes outside [offset, offset +
- * count) may be zeroed.
- *
- * Otherwise, the whole folio is cleared first so that a
- * failed copy leaves zeros rather than stale folio
- * contents.
- */
- if (cs->skip_folio_copy)
- folio_zero_segments(folio, 0, offset,
- offset + count, size);
- else
- folio_zero_range(folio, 0, size);
- }
- }
while (!cs->skip_folio_copy && count) {
if (cs->write && cs->pipebufs && folio) {
@@ -1293,7 +1272,7 @@ int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
}
} else if (!cs->len) {
if (cs->move_folios && folio &&
- offset == 0 && count == size) {
+ offset == 0 && count == folio_size(folio)) {
err = fuse_try_move_folio(cs, foliop);
if (err <= 0)
return err;
@@ -1334,10 +1313,25 @@ static int fuse_copy_folios(struct fuse_copy_state *cs, unsigned nbytes,
for (i = 0; i < ap->num_folios && (nbytes || zeroing); i++) {
int err;
+ struct folio *folio = ap->folios[i];
unsigned int offset = ap->descs[i].offset;
- unsigned int count = min(nbytes, ap->descs[i].length);
+ unsigned int length = ap->descs[i].length;
+ unsigned int count = min(nbytes, length);
+
+ /*
+ * The reply may be shorter than what was asked for. The full
+ * descs[i].length is reported as read, so the tail bytes the
+ * server did not send are about to be marked uptodate and need
+ * to be zeroed.
+ *
+ * Only [offset, offset + length) can be touched since the
+ * rest of the folio can hold blocks that are already uptodate
+ * or dirty, and clearing those would lose data.
+ */
+ if (folio && zeroing && count < length)
+ folio_zero_range(folio, offset + count, length - count);
- err = fuse_copy_folio(cs, &ap->folios[i], offset, count, zeroing);
+ err = fuse_copy_folio(cs, &ap->folios[i], offset, count);
if (err)
return err;
diff --git a/fs/fuse/dev.h b/fs/fuse/dev.h
index 8d25378c0918..f6c47ae0395b 100644
--- a/fs/fuse/dev.h
+++ b/fs/fuse/dev.h
@@ -90,7 +90,7 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id);
int fuse_copy_one(struct fuse_copy_state *cs, void *val, unsigned size);
int fuse_copy_folio(struct fuse_copy_state *cs, struct folio **foliop,
- unsigned offset, unsigned count, int zeroing);
+ unsigned offset, unsigned count);
void fuse_copy_finish(struct fuse_copy_state *cs);
#ifdef CONFIG_FUSE_IO_URING
diff --git a/fs/fuse/notify.c b/fs/fuse/notify.c
index 1ba763705d91..4b262928e8af 100644
--- a/fs/fuse/notify.c
+++ b/fs/fuse/notify.c
@@ -190,7 +190,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
folio_offset = offset_in_folio(folio, pos);
nr_bytes = min(num, folio_size(folio) - folio_offset);
- err = fuse_copy_folio(cs, &folio, folio_offset, nr_bytes, 0);
+ err = fuse_copy_folio(cs, &folio, folio_offset, nr_bytes);
if (!folio_test_uptodate(folio) && !err && folio_offset == 0 &&
(nr_bytes == folio_size(folio) || file_size == end)) {
folio_zero_segment(folio, nr_bytes, folio_size(folio));
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread