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

[sashiko-bot@kernel.org]

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