[PATCH v1] fuse: don't clear folio uptodate on writethrough errors

[PATCH v1] fuse: don't clear folio uptodate on writethrough errors

[Joanne Koong]

In the writethrough path (fuse_send_write_pages()), if the write to the
server failed or was a short write, the uptodate flag on the folios are
cleared.

As explained by Matthew in [1], this is dangerous because the folio may
be mapped into userspace. The mm code has the invariant that a
non-uptodate folio must never be visible to userspace (to avoid
potentially leaking confidental information to userspace) and has checks
in place for this that if violated can bring down the whole machine.

Practically speaking, the effect of this change for the fuse
writethrough error path is that if an application does a write and then
the server fails to persist the data or only services a short write, the
page cache folio keeps the data the application wrote instead of being
reverted to the server's contents on the next read. The failure is still
reported to the application synchronously through the short count /
error return of the write() syscall. Folios that were only partially
written are unaffected since they were never marked uptodate in the
first place (fuse_fill_write_page() only marks a folio as uptodate if
the whole folio was written to).

[1] https://lore.kernel.org/linux-fsdevel/ajtPMgO65FA1TXhi@casper.infradead.org/

Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/fuse/file.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index cb8da4c06d17..c4ae4009a423 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1227,8 +1227,7 @@ static ssize_t fuse_send_write_pages(struct fuse_io_args *ia,
 	struct file *file = iocb->ki_filp;
 	struct fuse_file *ff = file->private_data;
 	struct fuse_mount *fm = ff->fm;
-	unsigned int offset, i;
-	bool short_write;
+	unsigned int i;
 	int err;
 
 	for (i = 0; i < ap->num_folios; i++)
@@ -1243,24 +1242,9 @@ static ssize_t fuse_send_write_pages(struct fuse_io_args *ia,
 	if (!err && ia->write.out.size > count)
 		err = -EIO;
 
-	short_write = ia->write.out.size < count;
-	offset = ap->descs[0].offset;
-	count = ia->write.out.size;
 	for (i = 0; i < ap->num_folios; i++) {
 		struct folio *folio = ap->folios[i];
 
-		if (err) {
-			folio_clear_uptodate(folio);
-		} else {
-			if (count >= folio_size(folio) - offset)
-				count -= folio_size(folio) - offset;
-			else {
-				if (short_write)
-					folio_clear_uptodate(folio);
-				count = 0;
-			}
-			offset = 0;
-		}
 		if (ia->write.folio_locked && (i == ap->num_folios - 1))
 			folio_unlock(folio);
 		folio_put(folio);
-- 
2.52.0

Re: [PATCH v1] fuse: don't clear folio uptodate on writethrough errors

[Darrick J. Wong]

On Wed, Jun 24, 2026 at 01:52:01PM -0700, Joanne Koong wrote:
> In the writethrough path (fuse_send_write_pages()), if the write to the
> server failed or was a short write, the uptodate flag on the folios are
> cleared.
> 
> As explained by Matthew in [1], this is dangerous because the folio may
> be mapped into userspace. The mm code has the invariant that a
> non-uptodate folio must never be visible to userspace (to avoid
> potentially leaking confidental information to userspace) and has checks
> in place for this that if violated can bring down the whole machine.
> 
> Practically speaking, the effect of this change for the fuse
> writethrough error path is that if an application does a write and then
> the server fails to persist the data or only services a short write, the
> page cache folio keeps the data the application wrote instead of being
> reverted to the server's contents on the next read. The failure is still
> reported to the application synchronously through the short count /
> error return of the write() syscall. Folios that were only partially
> written are unaffected since they were never marked uptodate in the
> first place (fuse_fill_write_page() only marks a folio as uptodate if
> the whole folio was written to).
> 
> [1] https://lore.kernel.org/linux-fsdevel/ajtPMgO65FA1TXhi@casper.infradead.org/
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

Heh.  I suppose it's good that we no longer throw away dirty folios
because it's a bit rude if read() suddenly reverts.  Let's hope there's
not some weird third-level side effect that blows this up.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/fuse/file.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index cb8da4c06d17..c4ae4009a423 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1227,8 +1227,7 @@ static ssize_t fuse_send_write_pages(struct fuse_io_args *ia,
>  	struct file *file = iocb->ki_filp;
>  	struct fuse_file *ff = file->private_data;
>  	struct fuse_mount *fm = ff->fm;
> -	unsigned int offset, i;
> -	bool short_write;
> +	unsigned int i;
>  	int err;
>  
>  	for (i = 0; i < ap->num_folios; i++)
> @@ -1243,24 +1242,9 @@ static ssize_t fuse_send_write_pages(struct fuse_io_args *ia,
>  	if (!err && ia->write.out.size > count)
>  		err = -EIO;
>  
> -	short_write = ia->write.out.size < count;
> -	offset = ap->descs[0].offset;
> -	count = ia->write.out.size;
>  	for (i = 0; i < ap->num_folios; i++) {
>  		struct folio *folio = ap->folios[i];
>  
> -		if (err) {
> -			folio_clear_uptodate(folio);
> -		} else {
> -			if (count >= folio_size(folio) - offset)
> -				count -= folio_size(folio) - offset;
> -			else {
> -				if (short_write)
> -					folio_clear_uptodate(folio);
> -				count = 0;
> -			}
> -			offset = 0;
> -		}
>  		if (ia->write.folio_locked && (i == ap->num_folios - 1))
>  			folio_unlock(folio);
>  		folio_put(folio);
> -- 
> 2.52.0
> 
>