linux-kernel - Re: [PATCH v2 4/7] ext4: add more ext4_emergency_state() checks around sb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c4qihdjztjo4gn6srmuy3nl5qctxbf4vcc7j44rrtxosrfhlvj@qxivndjxqyms>
Date: Tue, 11 Feb 2025 15:28:19 +0100
From: Jan Kara <jack@...e.cz>
To: libaokun@...weicloud.com
Cc: linux-ext4@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca, 
	jack@...e.cz, linux-kernel@...r.kernel.org, yi.zhang@...wei.com, 
	yangerkun@...wei.com, Baokun Li <libaokun1@...wei.com>
Subject: Re: [PATCH v2 4/7] ext4: add more ext4_emergency_state() checks
 around sb_rdonly()

On Wed 22-01-25 19:41:27, libaokun@...weicloud.com wrote:
> From: Baokun Li <libaokun1@...wei.com>
> 
> Some functions check sb_rdonly() to make sure the file system isn't
> modified after it's read-only. Since we also don't want the file system
> modified if it's in an emergency state (shutdown or emergency_ro),
> we're adding additional ext4_emergency_state() checks where sb_rdonly()
> is checked.
> 
> Suggested-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Baokun Li <libaokun1@...wei.com>
> Reviewed-by: Zhang Yi <yi.zhang@...wei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
>  fs/ext4/file.c  |  3 ++-
>  fs/ext4/ioctl.c |  2 +-
>  fs/ext4/super.c | 26 +++++++++++++++-----------
>  3 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index d0c21e6503c6..45fc6586d41b 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -844,7 +844,8 @@ static int ext4_sample_last_mounted(struct super_block *sb,
>  	if (likely(ext4_test_mount_flag(sb, EXT4_MF_MNTDIR_SAMPLED)))
>  		return 0;
>  
> -	if (sb_rdonly(sb) || !sb_start_intwrite_trylock(sb))
> +	if (ext4_emergency_state(sb) || sb_rdonly(sb) ||
> +	    !sb_start_intwrite_trylock(sb))
>  		return 0;
>  
>  	ext4_set_mount_flag(sb, EXT4_MF_MNTDIR_SAMPLED);
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 7b9ce71c1c81..0c5ce9c2cdfc 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -1705,7 +1705,7 @@ int ext4_update_overhead(struct super_block *sb, bool force)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  
> -	if (sb_rdonly(sb))
> +	if (ext4_emergency_state(sb) || sb_rdonly(sb))
>  		return 0;
>  	if (!force &&
>  	    (sbi->s_overhead == 0 ||
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 4b089a5b760a..d8116c9c2bd0 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -473,8 +473,9 @@ static void ext4_maybe_update_superblock(struct super_block *sb)
>  	__u64 lifetime_write_kbytes;
>  	__u64 diff_size;
>  
> -	if (sb_rdonly(sb) || !(sb->s_flags & SB_ACTIVE) ||
> -	    !journal || (journal->j_flags & JBD2_UNMOUNT))
> +	if (ext4_emergency_state(sb) || sb_rdonly(sb) ||
> +	    !(sb->s_flags & SB_ACTIVE) || !journal ||
> +	    journal->j_flags & JBD2_UNMOUNT)
>  		return;
>  
>  	now = ktime_get_real_seconds();
> @@ -765,7 +766,8 @@ static void update_super_work(struct work_struct *work)
>  	 * We use directly jbd2 functions here to avoid recursing back into
>  	 * ext4 error handling code during handling of previous errors.
>  	 */
> -	if (!sb_rdonly(sbi->s_sb) && journal) {
> +	if (!ext4_emergency_state(sbi->s_sb) &&
> +	    !sb_rdonly(sbi->s_sb) && journal) {
>  		struct buffer_head *sbh = sbi->s_sbh;
>  		bool call_notify_err = false;
>  
> @@ -1325,13 +1327,14 @@ static void ext4_put_super(struct super_block *sb)
>  	ext4_mb_release(sb);
>  	ext4_ext_release(sb);
>  
> -	if (!sb_rdonly(sb) && !aborted) {
> -		ext4_clear_feature_journal_needs_recovery(sb);
> -		ext4_clear_feature_orphan_present(sb);
> -		es->s_state = cpu_to_le16(sbi->s_mount_state);
> -	}
> -	if (!sb_rdonly(sb))
> +	if (!ext4_emergency_state(sb) && !sb_rdonly(sb)) {
> +		if (!aborted) {
> +			ext4_clear_feature_journal_needs_recovery(sb);
> +			ext4_clear_feature_orphan_present(sb);
> +			es->s_state = cpu_to_le16(sbi->s_mount_state);
> +		}
>  		ext4_commit_super(sb);
> +	}
>  
>  	ext4_group_desc_free(sbi);
>  	ext4_flex_groups_free(sbi);
> @@ -3699,7 +3702,8 @@ static int ext4_run_li_request(struct ext4_li_request *elr)
>  		if (group >= elr->lr_next_group) {
>  			ret = 1;
>  			if (elr->lr_first_not_zeroed != ngroups &&
> -			    !sb_rdonly(sb) && test_opt(sb, INIT_INODE_TABLE)) {
> +			    !ext4_emergency_state(sb) && !sb_rdonly(sb) &&
> +			    test_opt(sb, INIT_INODE_TABLE)) {
>  				elr->lr_next_group = elr->lr_first_not_zeroed;
>  				elr->lr_mode = EXT4_LI_MODE_ITABLE;
>  				ret = 0;
> @@ -4004,7 +4008,7 @@ int ext4_register_li_request(struct super_block *sb,
>  		goto out;
>  	}
>  
> -	if (sb_rdonly(sb) ||
> +	if (ext4_emergency_state(sb) || sb_rdonly(sb) ||
>  	    (test_opt(sb, NO_PREFETCH_BLOCK_BITMAPS) &&
>  	     (first_not_zeroed == ngroups || !test_opt(sb, INIT_INODE_TABLE))))
>  		goto out;
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR