All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Joanne Koong <joannelkoong@gmail.com>
Cc: miklos@szeredi.hu, fuse-devel@lists.linux.dev,
	willy@infradead.org, hch@lst.de, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v1] fuse: don't clear folio uptodate on writethrough errors
Date: Thu, 25 Jun 2026 11:55:30 -0700	[thread overview]
Message-ID: <20260625185530.GS6070@frogsfrogsfrogs> (raw)
In-Reply-To: <20260624205201.842714-1-joannelkoong@gmail.com>

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
> 
> 

      reply	other threads:[~2026-06-25 18:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-24 20:52 [PATCH v1] fuse: don't clear folio uptodate on writethrough errors Joanne Koong
2026-06-25 18:55 ` Darrick J. Wong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260625185530.GS6070@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=fuse-devel@lists.linux.dev \
    --cc=hch@lst.de \
    --cc=joannelkoong@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.