[PATCH] ext4: reject delalloc to nodelalloc before applying remount options

[guzebing]

Commit 97f5ec3b166d ("ext4: prevent delalloc to nodelalloc on
remount") rejects switching a mounted filesystem from delalloc to
nodelalloc.  However, it performs the check after ext4_apply_options()
has already cleared EXT4_MOUNT_DELALLOC in the live superblock.  The
failure path eventually restores the bit, but leaves a window where
other CPUs can observe nodelalloc.

test_opt() reads s_mount_opt directly without a lock shared with
remount.  If a concurrent truncate's __es_remove_extent() hits this
window and observes nodelalloc, it sets count_reserved to false.
Delayed extent status entries are then removed without calculating
their cluster reservations, leaving reserved at zero.
ext4_es_remove_extent() therefore calls ext4_da_release_space() with
zero, leaving i_reserved_data_blocks and s_dirtyclusters_counter
elevated.  When the inode is later evicted after unlink,
ext4_destroy_inode() reports:

  i_reserved_data_blocks (...) not cleared!

CPU 0                             CPU 1
                                  ksys_truncate()
                                    ...
                                    ext4_es_remove_extent()
ext4_reconfigure()
  ext4_check_opt_consistency()
  __ext4_remount()
    ext4_apply_options()
      clear EXT4_MOUNT_DELALLOC
                                      __es_remove_extent()
                                        test_opt() sees !DELALLOC
                                        count_reserved = false
    reject delalloc -> nodelalloc
    restore EXT4_MOUNT_DELALLOC
                                        remove delayed ES
                                    ext4_da_release_space(0)

Follow the pre-apply validation approach used by
ext4_check_quota_consistency() and reject the transition in
ext4_check_opt_consistency(), before ext4_apply_options() changes live
state.  Use mask_s_mount_opt to determine whether delalloc/nodelalloc
was specified and ctx_test_mount_opt() to check the final parsed value.

Fixes: 97f5ec3b166d ("ext4: prevent delalloc to nodelalloc on remount")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: guzebing <guzebing1612@xxxxxxxxx>
---

This issue was first observed in a production environment.  It can now
be reproduced with a shell script.

  1. Create a 768 MiB ext4 filesystem and mount it with delalloc.
  2. Start 16 workers.  Each worker repeatedly runs:

       xfs_io -f -c 'pwrite -q 0 32m' file-N
       truncate -s 262144 file-N

  3. In parallel, repeatedly run "mount -o remount,nodelalloc".  Every
     remount is expected to fail.
  4. Continuously sample the live mount options with findmnt and record
     any transient nodelalloc state.
  5. After 300 seconds, stop the workers and unmount the filesystem,
     then inspect dmesg for "i_reserved_data_blocks (...) not cleared!".

On an affected kernel, 7 transient nodelalloc samples and 9 reservation
warnings were observed during 101 rejected remounts.

With this fix, no transient nodelalloc state or reservation warning was
observed.

 fs/ext4/super.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 245f67d10ded3..9e1ea94664775 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2816,6 +2816,14 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
 	}
 
 	if (is_remount) {
+		if (test_opt(sb, DELALLOC) &&
+		    (ctx->mask_s_mount_opt & EXT4_MOUNT_DELALLOC) &&
+		    !ctx_test_mount_opt(ctx, EXT4_MOUNT_DELALLOC)) {
+			ext4_msg(sb, KERN_ERR,
+				 "can't disable delalloc during remount");
+			return -EINVAL;
+		}
+
 		if (!sbi->s_journal &&
 		    ctx_test_mount_opt(ctx, EXT4_MOUNT_DATA_ERR_ABORT)) {
 			ext4_msg(NULL, KERN_WARNING,
@@ -6660,13 +6668,6 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
 		goto restore_opts;
 	}
 
-	if ((old_opts.s_mount_opt & EXT4_MOUNT_DELALLOC) &&
-	    !test_opt(sb, DELALLOC)) {
-		ext4_msg(sb, KERN_ERR, "can't disable delalloc during remount");
-		err = -EINVAL;
-		goto restore_opts;
-	}
-
 	sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
 		(test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0);