linux-kernel - Re: [PATCH v2 3/8] ext4: reject the 'data

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: <2hr7ifk6f7ojn3iubi3qj2yp5m72fvbch22afkmw2emsypz5kv@uah27xxstly3>
Date: Tue, 21 Jan 2025 12:37:15 +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 3/8] ext4: reject the 'data_err=abort' option in
 nojournal mode

On Tue 21-01-25 15:10:45, libaokun@...weicloud.com wrote:
> From: Baokun Li <libaokun1@...wei.com>
> 
> data_err=abort aborts the journal on I/O errors. However, this option is
> meaningless if journal is disabled, so it is rejected in nojournal mode
> to reduce unnecessary checks. Also, this option is ignored upon remount.
> 
> 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>

> ---
> Or maybe we should make mount and remount consistent?
> Rejecting it in both would make things a lot easier.

The reason why it is ignored on remount is that we cannot modify
JBD2_ABORT_ON_SYNCDATA_ERR on remount. Now that we don't really depend on
JBD2_ABORT_ON_SYNCDATA_ERR, we could indeed make mount and remount
consistent and probably just drop JBD2_ABORT_ON_SYNCDATA_ERR altogether
(and only warn in jbd2 as it used to be long ago).

								Honza
> 
>  fs/ext4/super.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index a50e5c31b937..34a7b6523f8b 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2785,6 +2785,13 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
>  	}
>  
>  	if (is_remount) {
> +		if (!sbi->s_journal &&
> +		    ctx_test_mount_opt(ctx, EXT4_MOUNT_DATA_ERR_ABORT)) {
> +			ext4_msg(NULL, KERN_WARNING,
> +				 "Remounting fs w/o journal so ignoring data_err option");
> +			ctx_clear_mount_opt(ctx, EXT4_MOUNT_DATA_ERR_ABORT);
> +		}
> +
>  		if (ctx_test_mount_opt(ctx, EXT4_MOUNT_DAX_ALWAYS) &&
>  		    (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)) {
>  			ext4_msg(NULL, KERN_ERR, "can't mount with "
> @@ -5428,6 +5435,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  				 "data=, fs mounted w/o journal");
>  			goto failed_mount3a;
>  		}
> +		if (test_opt(sb, DATA_ERR_ABORT)) {
> +			ext4_msg(sb, KERN_ERR,
> +				 "can't mount with data_err=abort, fs mounted w/o journal");
> +			goto failed_mount3a;
> +		}
>  		sbi->s_def_mount_opt &= ~EXT4_MOUNT_JOURNAL_CHECKSUM;
>  		clear_opt(sb, JOURNAL_CHECKSUM);
>  		clear_opt(sb, DATA_FLAGS);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR