Skip to content

Commit e9c3a8e

Browse files
author
Darrick J. Wong
committed
iomap: don't invalidate folios after writeback errors
XFS has the unique behavior (as compared to the other Linux filesystems) that on writeback errors it will completely invalidate the affected folio and force the page cache to reread the contents from disk. All other filesystems leave the page mapped and up to date. This is a rude awakening for user programs, since (in the case where write fails but reread doesn't) file contents will appear to revert to old disk contents with no notification other than an EIO on fsync. This might have been annoying back in the days when iomap dealt with one page at a time, but with multipage folios, we can now throw away *megabytes* worth of data for a single write error. On *most* Linux filesystems, a program can respond to an EIO on write by redirtying the entire file and scheduling it for writeback. This isn't foolproof, since the page that failed writeback is no longer dirty and could be evicted, but programs that want to recover properly *also* have to detect XFS and regenerate every write they've made to the file. When running xfs/314 on arm64, I noticed a UAF when xfs_discard_folio invalidates multipage folios that could be undergoing writeback. If, say, we have a 256K folio caching a mix of written and unwritten extents, it's possible that we could start writeback of the first (say) 64K of the folio and then hit a writeback error on the next 64K. We then free the iop attached to the folio, which is really bad because writeback completion on the first 64k will trip over the "blocks per folio > 1 && !iop" assertion. This can't be fixed by only invalidating the folio if writeback fails at the start of the folio, since the folio is marked !uptodate, which trips other assertions elsewhere. Get rid of the whole behavior entirely. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de>
1 parent d74999c commit e9c3a8e

File tree

2 files changed

+1
-4
lines changed

2 files changed

+1
-4
lines changed

fs/iomap/buffered-io.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,6 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
13871387
if (wpc->ops->discard_folio)
13881388
wpc->ops->discard_folio(folio, pos);
13891389
if (!count) {
1390-
folio_clear_uptodate(folio);
13911390
folio_unlock(folio);
13921391
goto done;
13931392
}

fs/xfs/xfs_aops.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ xfs_discard_folio(
464464
int error;
465465

466466
if (xfs_is_shutdown(mp))
467-
goto out_invalidate;
467+
return;
468468

469469
xfs_alert_ratelimited(mp,
470470
"page discard on page "PTR_FMT", inode 0x%llx, pos %llu.",
@@ -474,8 +474,6 @@ xfs_discard_folio(
474474
i_blocks_per_folio(inode, folio) - pageoff_fsb);
475475
if (error && !xfs_is_shutdown(mp))
476476
xfs_alert(mp, "page discard unable to remove delalloc mapping.");
477-
out_invalidate:
478-
iomap_invalidate_folio(folio, offset, folio_size(folio) - offset);
479477
}
480478

481479
static const struct iomap_writeback_ops xfs_writeback_ops = {

0 commit comments

Comments
 (0)