On Apr 15, 2020, at 11:48 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> Rejecting the mount options in ext4_remount() means that some mount
> options would be enabled for a small amount of time, and then the
> mount option change would be reverted.  In the case of "mount -o
> remount,dax", this can cause a race where files would temporarily
> treated as DAX --- and then not.
> 
> Cc: stable@kernel.org
> Reported-and-tested-by: syzbot+bca9799bf129256190da@syzkaller.appspotmail.com
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext4/super.c | 37 +++++++++++--------------------------
> 1 file changed, 11 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bf5fcb477f66..6fe32f9aa889 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1726,6 +1726,7 @@ static int clear_qf_name(struct super_block *sb, int qtype)
> #define MOPT_NO_EXT3	0x0200
> #define MOPT_EXT4_ONLY	(MOPT_NO_EXT2 | MOPT_NO_EXT3)
> #define MOPT_STRING	0x0400
> +#define MOPT_NO_REMOUNT	0x0800
> 
> static const struct mount_opts {
> 	int	token;
> @@ -1775,12 +1776,12 @@ static const struct mount_opts {
> 	{Opt_min_batch_time, 0, MOPT_GTE0},
> 	{Opt_inode_readahead_blks, 0, MOPT_GTE0},
> 	{Opt_init_itable, 0, MOPT_GTE0},
> -	{Opt_dax, EXT4_MOUNT_DAX, MOPT_SET},
> +	{Opt_dax, EXT4_MOUNT_DAX, MOPT_SET | MOPT_NO_REMOUNT},
> 	{Opt_stripe, 0, MOPT_GTE0},
> 	{Opt_resuid, 0, MOPT_GTE0},
> 	{Opt_resgid, 0, MOPT_GTE0},
> -	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0},
> -	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING},
> +	{Opt_journal_dev, 0, MOPT_NO_EXT2 | MOPT_GTE0 | MOPT_NO_REMOUNT},
> +	{Opt_journal_path, 0, MOPT_NO_EXT2 | MOPT_STRING | MOPT_NO_REMOUNT},
> 	{Opt_journal_ioprio, 0, MOPT_NO_EXT2 | MOPT_GTE0},
> 	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
> 	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
> @@ -1817,7 +1818,7 @@ static const struct mount_opts {
> 	{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
> 	{Opt_max_dir_size_kb, 0, MOPT_GTE0},
> 	{Opt_test_dummy_encryption, 0, MOPT_GTE0},
> -	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
> +	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET | MOPT_NO_REMOUNT},
> 	{Opt_err, 0, 0}
> };
> 
> @@ -1915,6 +1916,12 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> 			 "Mount option \"%s\" incompatible with ext3", opt);
> 		return -1;
> 	}
> +	if ((m->flags & MOPT_NO_REMOUNT) && is_remount) {
> +		ext4_msg(sb, KERN_ERR,
> +			 "Mount option \"%s\" not supported when remounting",
> +			 opt);
> +		return -1;
> +	}
> 
> 	if (args->from && !(m->flags & MOPT_STRING) && match_int(args, &arg))
> 		return -1;
> @@ -1994,11 +2001,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> 		}
> 		sbi->s_resgid = gid;
> 	} else if (token == Opt_journal_dev) {
> -		if (is_remount) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "Cannot specify journal on remount");
> -			return -1;
> -		}
> 		*journal_devnum = arg;
> 	} else if (token == Opt_journal_path) {
> 		char *journal_path;
> @@ -2006,11 +2008,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> 		struct path path;
> 		int error;
> 
> -		if (is_remount) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "Cannot specify journal on remount");
> -			return -1;
> -		}
> 		journal_path = match_strdup(&args[0]);
> 		if (!journal_path) {
> 			ext4_msg(sb, KERN_ERR, "error: could not dup "
> @@ -5427,18 +5424,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> 		}
> 	}
> 
> -	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;
> -	}
> -
> -	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX) {
> -		ext4_msg(sb, KERN_WARNING, "warning: refusing change of "
> -			"dax flag with busy inodes while remounting");
> -		sbi->s_mount_opt ^= EXT4_MOUNT_DAX;
> -	}
> -
> 	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
> 		ext4_abort(sb, EXT4_ERR_ESHUTDOWN, "Abort forced by user");
> 
> --
> 2.24.1
> 


Cheers, Andreas