2023-01-29 03:49:59

by Zhang Yi

[permalink] [raw]
Subject: [PATCH] ext4: fix incorrect options show of original mount_opt and extend mount_opt2

From: Zhang Yi <[email protected]>

Current _ext4_show_options() do not distinguish MOPT_2 flag, so it mixed
extend sbi->s_mount_opt2 options with sbi->s_mount_opt, it could lead to
show incorrect options, e.g. show fc_debug_force if we mount with
errors=continue mode and miss it if we set.

$ mkfs.ext4 /dev/pmem0
$ mount -o errors=remount-ro /dev/pmem0 /mnt
$ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
#empty
$ mount -o remount,errors=continue /mnt
$ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
fc_debug_force
$ mount -o remount,errors=remount-ro,fc_debug_force /mnt
$ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
#empty

Fixes: 995a3ed67fc8 ("ext4: add fast_commit feature and handling for extended mount options")
Signed-off-by: Zhang Yi <[email protected]>
---
fs/ext4/ext4.h | 1 +
fs/ext4/super.c | 28 +++++++++++++++++++++-------
2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 140e1eb300d1..6479146140d2 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1529,6 +1529,7 @@ struct ext4_sb_info {
unsigned int s_mount_opt2;
unsigned long s_mount_flags;
unsigned int s_def_mount_opt;
+ unsigned int s_def_mount_opt2;
ext4_fsblk_t s_sb_block;
atomic64_t s_resv_clusters;
kuid_t s_resuid;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 260c1b3e3ef2..e6fe416a65a3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2894,7 +2894,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
{
struct ext4_sb_info *sbi = EXT4_SB(sb);
struct ext4_super_block *es = sbi->s_es;
- int def_errors, def_mount_opt = sbi->s_def_mount_opt;
+ int def_errors;
const struct mount_opts *m;
char sep = nodefs ? '\n' : ',';

@@ -2906,15 +2906,28 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,

for (m = ext4_mount_opts; m->token != Opt_err; m++) {
int want_set = m->flags & MOPT_SET;
+ int opt_2 = m->flags & MOPT_2;
+ unsigned int mount_opt, def_mount_opt;
+
if (((m->flags & (MOPT_SET|MOPT_CLEAR)) == 0) ||
m->flags & MOPT_SKIP)
continue;
- if (!nodefs && !(m->mount_opt & (sbi->s_mount_opt ^ def_mount_opt)))
- continue; /* skip if same as the default */
+
+ if (opt_2) {
+ mount_opt = sbi->s_mount_opt2;
+ def_mount_opt = sbi->s_def_mount_opt2;
+ } else {
+ mount_opt = sbi->s_mount_opt;
+ def_mount_opt = sbi->s_def_mount_opt;
+ }
+ /* skip if same as the default */
+ if (!nodefs && !(m->mount_opt & (mount_opt ^ def_mount_opt)))
+ continue;
+ /* select Opt_noFoo vs Opt_Foo */
if ((want_set &&
- (sbi->s_mount_opt & m->mount_opt) != m->mount_opt) ||
- (!want_set && (sbi->s_mount_opt & m->mount_opt)))
- continue; /* select Opt_noFoo vs Opt_Foo */
+ (mount_opt & m->mount_opt) != m->mount_opt) ||
+ (!want_set && (mount_opt & m->mount_opt)))
+ continue;
SEQ_OPTS_PRINT("%s", token2str(m->token));
}

@@ -2942,7 +2955,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
if (nodefs || sbi->s_stripe)
SEQ_OPTS_PRINT("stripe=%lu", sbi->s_stripe);
if (nodefs || EXT4_MOUNT_DATA_FLAGS &
- (sbi->s_mount_opt ^ def_mount_opt)) {
+ (sbi->s_mount_opt ^ sbi->s_def_mount_opt)) {
if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)
SEQ_OPTS_PUTS("data=journal");
else if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_ORDERED_DATA)
@@ -5086,6 +5099,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
goto failed_mount;

sbi->s_def_mount_opt = sbi->s_mount_opt;
+ sbi->s_def_mount_opt2 = sbi->s_mount_opt2;

err = ext4_check_opt_consistency(fc, sb);
if (err < 0)
--
2.31.1



2023-01-30 11:11:43

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix incorrect options show of original mount_opt and extend mount_opt2

On Sun 29-01-23 11:49:39, Zhang Yi wrote:
> From: Zhang Yi <[email protected]>
>
> Current _ext4_show_options() do not distinguish MOPT_2 flag, so it mixed
> extend sbi->s_mount_opt2 options with sbi->s_mount_opt, it could lead to
> show incorrect options, e.g. show fc_debug_force if we mount with
> errors=continue mode and miss it if we set.
>
> $ mkfs.ext4 /dev/pmem0
> $ mount -o errors=remount-ro /dev/pmem0 /mnt
> $ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
> #empty
> $ mount -o remount,errors=continue /mnt
> $ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
> fc_debug_force
> $ mount -o remount,errors=remount-ro,fc_debug_force /mnt
> $ cat /proc/fs/ext4/pmem0/options | grep fc_debug_force
> #empty
>
> Fixes: 995a3ed67fc8 ("ext4: add fast_commit feature and handling for extended mount options")
> Signed-off-by: Zhang Yi <[email protected]>

Good catch! Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/ext4.h | 1 +
> fs/ext4/super.c | 28 +++++++++++++++++++++-------
> 2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 140e1eb300d1..6479146140d2 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1529,6 +1529,7 @@ struct ext4_sb_info {
> unsigned int s_mount_opt2;
> unsigned long s_mount_flags;
> unsigned int s_def_mount_opt;
> + unsigned int s_def_mount_opt2;
> ext4_fsblk_t s_sb_block;
> atomic64_t s_resv_clusters;
> kuid_t s_resuid;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 260c1b3e3ef2..e6fe416a65a3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2894,7 +2894,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
> {
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> struct ext4_super_block *es = sbi->s_es;
> - int def_errors, def_mount_opt = sbi->s_def_mount_opt;
> + int def_errors;
> const struct mount_opts *m;
> char sep = nodefs ? '\n' : ',';
>
> @@ -2906,15 +2906,28 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
>
> for (m = ext4_mount_opts; m->token != Opt_err; m++) {
> int want_set = m->flags & MOPT_SET;
> + int opt_2 = m->flags & MOPT_2;
> + unsigned int mount_opt, def_mount_opt;
> +
> if (((m->flags & (MOPT_SET|MOPT_CLEAR)) == 0) ||
> m->flags & MOPT_SKIP)
> continue;
> - if (!nodefs && !(m->mount_opt & (sbi->s_mount_opt ^ def_mount_opt)))
> - continue; /* skip if same as the default */
> +
> + if (opt_2) {
> + mount_opt = sbi->s_mount_opt2;
> + def_mount_opt = sbi->s_def_mount_opt2;
> + } else {
> + mount_opt = sbi->s_mount_opt;
> + def_mount_opt = sbi->s_def_mount_opt;
> + }
> + /* skip if same as the default */
> + if (!nodefs && !(m->mount_opt & (mount_opt ^ def_mount_opt)))
> + continue;
> + /* select Opt_noFoo vs Opt_Foo */
> if ((want_set &&
> - (sbi->s_mount_opt & m->mount_opt) != m->mount_opt) ||
> - (!want_set && (sbi->s_mount_opt & m->mount_opt)))
> - continue; /* select Opt_noFoo vs Opt_Foo */
> + (mount_opt & m->mount_opt) != m->mount_opt) ||
> + (!want_set && (mount_opt & m->mount_opt)))
> + continue;
> SEQ_OPTS_PRINT("%s", token2str(m->token));
> }
>
> @@ -2942,7 +2955,7 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
> if (nodefs || sbi->s_stripe)
> SEQ_OPTS_PRINT("stripe=%lu", sbi->s_stripe);
> if (nodefs || EXT4_MOUNT_DATA_FLAGS &
> - (sbi->s_mount_opt ^ def_mount_opt)) {
> + (sbi->s_mount_opt ^ sbi->s_def_mount_opt)) {
> if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)
> SEQ_OPTS_PUTS("data=journal");
> else if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_ORDERED_DATA)
> @@ -5086,6 +5099,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> goto failed_mount;
>
> sbi->s_def_mount_opt = sbi->s_mount_opt;
> + sbi->s_def_mount_opt2 = sbi->s_mount_opt2;
>
> err = ext4_check_opt_consistency(fc, sb);
> if (err < 0)
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-02-19 05:42:04

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix incorrect options show of original mount_opt and extend mount_opt2

On Sun, 29 Jan 2023 11:49:39 +0800, Zhang Yi wrote:
> From: Zhang Yi <[email protected]>
>
> Current _ext4_show_options() do not distinguish MOPT_2 flag, so it mixed
> extend sbi->s_mount_opt2 options with sbi->s_mount_opt, it could lead to
> show incorrect options, e.g. show fc_debug_force if we mount with
> errors=continue mode and miss it if we set.
>
> [...]

Applied, thanks!

[1/1] ext4: fix incorrect options show of original mount_opt and extend mount_opt2
commit: 2c2dec1e86cc43266e084a8782f91613d52eeddf

Best regards,
--
Theodore Ts'o <[email protected]>