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 Reviewed-by: Darrick J. Wong Signed-off-by: Joanne Koong --- 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 e052a0d44dee..a72959dbcf12 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 Add an exported helper iomap_folio_mark_uptodate() to mark a folio as uptodate and update its uptodate bitmap if the folio has iomap state data attached. This is needed because there are some filesystems (eg fuse) that have paths outside of conventional iomap calls that need to mark a folio as uptodate (eg writing server-pushed data directly into the page cache) and need the iomap-internal uptodate bitmap to be in sync with the uptodate state of the folio. Reviewed-by: Christoph Hellwig Reviewed-by: Darrick J. Wong Signed-off-by: Joanne Koong --- fs/iomap/buffered-io.c | 6 ++++++ include/linux/iomap.h | 1 + 2 files changed, 7 insertions(+) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 27bc2455a98d..f6040199d114 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -105,6 +105,12 @@ static void iomap_set_range_uptodate(struct folio *folio, size_t off, folio_mark_uptodate(folio); } +void iomap_folio_mark_uptodate(struct folio *folio) +{ + iomap_set_range_uptodate(folio, 0, folio_size(folio)); +} +EXPORT_SYMBOL_GPL(iomap_folio_mark_uptodate); + /* * Find the next dirty block in the folio. end_blk is inclusive. * If no dirty block is found, this will return end_blk + 1. diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 3582ed1fe236..21e73cb9c51e 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -365,6 +365,7 @@ struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos, size_t len); bool iomap_release_folio(struct folio *folio, gfp_t gfp_flags); void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len); bool iomap_dirty_folio(struct address_space *mapping, struct folio *folio); +void iomap_folio_mark_uptodate(struct folio *folio); int iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len, const struct iomap_ops *ops, const struct iomap_write_ops *write_ops); -- 2.52.0 When fuse enables large folios, a large folio will be backed by iomap_folio_state that keeps track of uptodate and dirty state in an internal bitmap. Fuse writethrough and notify store paths currently set folio uptodate state with folio_mark_uptodate(), which touches only the folio-level flag, but on an iomap-backed folio, that leaves the uptodate bitmap out of sync. Use the iomap_folio_mark_uptodate() helper to update both the folio uptodate state and the iomap uptodate bitmap. Reviewed-by: Darrick J. Wong Signed-off-by: Joanne Koong --- fs/fuse/file.c | 2 +- fs/fuse/notify.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index a72959dbcf12..ea4a15a7635a 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1319,7 +1319,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia, /* If we copied full folio, mark it uptodate */ if (tmp == folio_size(folio)) - folio_mark_uptodate(folio); + iomap_folio_mark_uptodate(folio); if (folio_test_uptodate(folio)) { folio_unlock(folio); diff --git a/fs/fuse/notify.c b/fs/fuse/notify.c index 29578104ae6c..1ba763705d91 100644 --- a/fs/fuse/notify.c +++ b/fs/fuse/notify.c @@ -2,6 +2,8 @@ #include "dev.h" #include "fuse_i.h" + +#include #include static int fuse_notify_poll(struct fuse_conn *fc, unsigned int size, @@ -192,7 +194,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size, 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)); - folio_mark_uptodate(folio); + iomap_folio_mark_uptodate(folio); } folio_unlock(folio); folio_put(folio); -- 2.52.0