ext2: Fix data integrity writeout issues

Ext2 could fail to properly write out inode on fsync(2) due to races
with WB_SYNC_NONE writeback. Several racing fsyncs could also result in
some fsync returning earlier than all metadata buffers were properly
persisted. Finally DIRSYNC handling was not properly persisting all
inode related metadata.

Fix all these issues by using new .sync_inode_metadata method which
makes sure all inode related metadata is written to disk during any
WB_SYNC_ALL writeback.

Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260727104923.3828017-29-jack@suse.cz
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 278d4be8ecbe7790204b5ba985a7ce088fadb181..e17bbc7598c143b5562df91b4dfee7e845a6b096 100644 (file)
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -734,6 +734,6 @@ const struct file_operations ext2_dir_operations = {
 #ifdef CONFIG_COMPAT
        .compat_ioctl   = ext2_compat_ioctl,
 #endif
-       .fsync          = ext2_fsync,
+       .fsync          = simple_fsync,
        .setlease       = generic_setlease,
 };

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 79f7b395258c2ac3d7b2d9a14c468c65beb0e6bb..5642451bf191730b20d699afb2de96e72cd0c521 100644 (file)
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -735,6 +735,7 @@ extern unsigned long ext2_count_free (struct buffer_head *, unsigned);
 /* inode.c */
 extern struct inode *ext2_iget (struct super_block *, unsigned long);
 extern int ext2_write_inode (struct inode *, struct writeback_control *);
+extern int ext2_sync_inode_metadata(struct inode *, struct writeback_control *);
 extern void ext2_evict_inode(struct inode *);
 void ext2_write_failed(struct address_space *mapping, loff_t to);
 extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
@@ -772,8 +773,6 @@ extern void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es,
 extern const struct file_operations ext2_dir_operations;
 
 /* file.c */
-extern int ext2_fsync(struct file *file, loff_t start, loff_t end,
-                     int datasync);
 extern const struct inode_operations ext2_file_inode_operations;
 extern const struct file_operations ext2_file_operations;
 

diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index 8dca9ec4cacd9afbf4b2f6ed6535547e6e8b432b..b9020df7d89e033aed06ca3b2d956753cdd5b21e 100644 (file)
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -47,21 +47,6 @@ static int ext2_release_file (struct inode * inode, struct file * filp)
        return 0;
 }
 
-int ext2_fsync(struct file *file, loff_t start, loff_t end, int datasync)
-{
-       int ret;
-       struct inode *inode = file->f_mapping->host;
-       struct super_block *sb = inode->i_sb;
-
-       ret = mmb_fsync(file, &EXT2_I(inode)->i_metadata_bhs,
-                       start, end, datasync);
-       if (ret == -EIO)
-               /* We don't really know where the IO error happened... */
-               ext2_error(sb, __func__,
-                          "detected IO error when writing metadata buffers");
-       return ret;
-}
-
 static ssize_t ext2_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
        struct file *file = iocb->ki_filp;
@@ -213,7 +198,7 @@ const struct file_operations ext2_file_operations = {
        .mmap_prepare   = generic_file_mmap_prepare,
        .open           = ext2_file_open,
        .release        = ext2_release_file,
-       .fsync          = ext2_fsync,
+       .fsync          = simple_fsync,
        .get_unmapped_area = thp_get_unmapped_area,
        .splice_read    = filemap_splice_read,
        .splice_write   = iter_file_splice_write,

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 904e70f3140eaca551f52a1e31bb9b4f755b93ca..b5c958db9ecf1fd03d54d1638fc6c2de29cd1dd6 100644 (file)
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -81,16 +81,11 @@ void ext2_evict_inode(struct inode * inode)
        truncate_inode_pages_final(&inode->i_data);
 
        if (want_delete) {
-               struct writeback_control wbc = {
-                       .sync_mode = inode_needs_sync(inode) ? WB_SYNC_ALL :
-                                                              WB_SYNC_NONE,
-               };
-
                sb_start_intwrite(inode->i_sb);
                /* set dtime */
                EXT2_I(inode)->i_dtime  = ktime_get_real_seconds();
                mark_inode_dirty(inode);
-               ext2_write_inode(inode, &wbc);
+               sync_inode_metadata(inode, inode_needs_sync(inode));
                /* truncate to 0 */
                inode->i_size = 0;
                if (inode->i_blocks)
@@ -1560,20 +1555,37 @@ int ext2_write_inode(struct inode *inode, struct writeback_control *wbc)
        } else for (n = 0; n < EXT2_N_BLOCKS; n++)
                raw_inode->i_block[n] = ei->i_data[n];
        mark_buffer_dirty(bh);
-       /*
-        * For sync(2) the generic code will call sync_blockdev() to write
-        * all metadata more efficiently.
-        */
-       if (wbc->sync_mode == WB_SYNC_ALL && !wbc->for_sync) {
-               sync_dirty_buffer(bh);
-               if (buffer_req(bh) && !buffer_uptodate(bh)) {
-                       printk ("IO error syncing ext2 inode [%s:%08lx]\n",
-                               sb->s_id, (unsigned long) ino);
-                       err = -EIO;
-               }
-       }
        ei->i_state &= ~EXT2_STATE_NEW;
        brelse (bh);
+       set_inode_metadata_writeback(inode);
+       return err;
+}
+
+int ext2_sync_inode_metadata(struct inode *inode, struct writeback_control *wbc)
+{
+       struct buffer_head *bh;
+       struct ext2_inode *raw_inode = ext2_get_inode(inode->i_sb, inode->i_ino,
+                                                     &bh);
+       int err = 0;
+
+       if (IS_ERR(raw_inode))
+               return -EIO;
+       err = mmb_sync(&EXT2_I(inode)->i_metadata_bhs);
+       if (err) {
+               ext2_error(inode->i_sb, __func__,
+                       "Error syncing inode metadata ino=%lu\n",
+                       (unsigned long)inode->i_ino);
+               goto out;
+       }
+       sync_dirty_buffer(bh);
+       if (buffer_write_io_error(bh)) {
+               ext2_error(inode->i_sb, __func__,
+                       "IO error syncing inode %lu\n",
+                       (unsigned long)inode->i_ino);
+               err = -EIO;
+       }
+out:
+       brelse(bh);
        return err;
 }
 

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 3999f8f3b156e345811e211e8e0372931a825d55..a40f530872a407142dbfb873a4941d685ffcb6a2 100644 (file)
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -362,6 +362,7 @@ static const struct super_operations ext2_sops = {
        .alloc_inode    = ext2_alloc_inode,
        .free_inode     = ext2_free_in_core_inode,
        .write_inode    = ext2_write_inode,
+       .sync_inode_metadata = ext2_sync_inode_metadata,
        .evict_inode    = ext2_evict_inode,
        .put_super      = ext2_put_super,
        .sync_fs        = ext2_sync_fs,