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

[Jan Kara]

On Fri 14-08-26 11:48:55, 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)
> 
> 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>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR