[PATCH v1 2/2] fuse: zero the correct range on a short read reply

[Joanne Koong <joannelkoong@gmail.com>]

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