All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext4: reject delalloc to nodelalloc before applying remount options
@ 2026-08-14  3:48 guzebing
  2026-08-14  5:13 ` sashiko-bot
  2026-08-14  7:41 ` Ojaswin Mujoo
  0 siblings, 2 replies; 3+ messages in thread
From: guzebing @ 2026-08-14  3:48 UTC (permalink / raw)
  To: linux-ext4, linux-kernel
  Cc: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
	yi.zhang, bretznic, guzebing, stable

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@vger.kernel.org
Signed-off-by: guzebing <guzebing1612@gmail.com>
---

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ext4: reject delalloc to nodelalloc before applying remount options
  2026-08-14  3:48 [PATCH] ext4: reject delalloc to nodelalloc before applying remount options guzebing
@ 2026-08-14  5:13 ` sashiko-bot
  2026-08-14  7:41 ` Ojaswin Mujoo
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-14  5:13 UTC (permalink / raw)
  To: guzebing; +Cc: linux-ext4, tytso

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch incompletely fixes the TOCTOU vulnerability and resource leaks. Post-apply validations in `__ext4_remount()` still expose transient invalid states during failure paths, and the error path restores options locklessly.
--

--- Patch [1]: [PATCH] ext4: reject delalloc to nodelalloc before applying remount options ---
commit 3e2a754f5c730db5d2d9a5a35cc756e9e8b67276
Author: guzebing <guzebing1612@gmail.com>

ext4: reject delalloc to nodelalloc before applying remount options

This patch rejects switching from delalloc to nodelalloc during remount
prior to applying options. This prevents a race window where concurrent
truncates observe nodelalloc and leak reserved data blocks.

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bca0dc87d0b7c..ec21ed1cb6ded 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
[ ... ]
> @@ -6688,13 +6696,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);

[Severity: High]
This is a pre-existing issue, but does this approach leave other mount
options vulnerable to similar transient states and lockless rollbacks?

While this patch successfully addresses the delalloc to nodelalloc
transition by moving the check to ext4_check_opt_consistency(), other
validations in __ext4_remount() are still performed after
ext4_apply_options() has been called.

For example, if a user mounts with nodelalloc and then remounts with
-o delalloc,nombcache, the nombcache check in __ext4_remount() fails
after delalloc has already been applied:

__ext4_remount() {
        ...
        if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_NO_MBCACHE) {
                ext4_msg(sb, KERN_ERR, "can't enable nombcache during remount");
                err = -EINVAL;
                goto restore_opts;
        }
        ...
}

This creates a window where delalloc is transiently true, allowing
concurrent tasks to create an inode with delayed allocation and reserve
data blocks, which could leak when delalloc is rolled back to false.

Furthermore, the rollback at the restore_opts label:

restore_opts:
        sbi->s_mount_opt = old_opts.s_mount_opt;
        sbi->s_mount_opt2 = old_opts.s_mount_opt2;

restores the mount options without acquiring the s_writepages_rwsem.
Could rolling back options like DIOREAD_NOLOCK or DELALLOC locklessly
cause data races with concurrent writepages operations, potentially
triggering the WARN_ON in ext4_add_complete_io()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814034855.1573759-1-guzebing1612@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ext4: reject delalloc to nodelalloc before applying remount options
  2026-08-14  3:48 [PATCH] ext4: reject delalloc to nodelalloc before applying remount options guzebing
  2026-08-14  5:13 ` sashiko-bot
@ 2026-08-14  7:41 ` Ojaswin Mujoo
  1 sibling, 0 replies; 3+ messages in thread
From: Ojaswin Mujoo @ 2026-08-14  7:41 UTC (permalink / raw)
  To: guzebing
  Cc: linux-ext4, linux-kernel, tytso, adilger.kernel, libaokun, jack,
	ritesh.list, yi.zhang, bretznic, stable

On Fri, Aug 14, 2026 at 11:48:55AM +0800, guzebing wrote:
> 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)
> 

Hi guzebing,

This looks like a valid issue. I think the unrelated issues sashiko has
pointed is also valid and we can probably move back more of these checks
from __ext4_remount to ext4_check_opt_consistency, but that can be done
in separate patches.

Feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

> 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@vger.kernel.org
> Signed-off-by: guzebing <guzebing1612@gmail.com>
> ---
> 
> 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.

Can you please add an xfstests for this. The test can stress this path and
then use the _check_dmesg_for helper to detect if there were any
i_reserved_data_blocks related warnings.

Regards,
ojaswin

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-14  7:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14  3:48 [PATCH] ext4: reject delalloc to nodelalloc before applying remount options guzebing
2026-08-14  5:13 ` sashiko-bot
2026-08-14  7:41 ` Ojaswin Mujoo

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.