linux-ext4 - Re: [PATCH] ext4: Avoid remount errors with 'abort' mount option

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: <CAASaF6wKeEkAWW6SQOur+R7EHwC7YVx_C+DTcQojhOfhUCLvaw@mail.gmail.com>
Date: Mon, 7 Oct 2024 11:13:38 +0200
From: Jan Stancek <jstancek@...hat.com>
To: Jan Kara <jack@...e.cz>
Cc: Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org, 
	Amir Goldstein <amir73il@...il.com>
Subject: Re: [PATCH] ext4: Avoid remount errors with 'abort' mount option

On Sat, Oct 5, 2024 at 12:16 AM Jan Kara <jack@...e.cz> wrote:
>
> When we remount filesystem with 'abort' mount option while changing
> other mount options as well (as is LTP test doing), we can return error
> from the system call after commit d3476f3dad4a ("ext4: don't set
> SB_RDONLY after filesystem errors") because the application of mount
> option changes detects shutdown filesystem and refuses to do anything.
> The behavior of application of other mount options in presence of
> 'abort' mount option is currently rather arbitary as some mount option
> changes are handled before 'abort' and some after it.
>
> Move aborting of the filesystem to the end of remount handling so all
> requested changes are properly applied before the filesystem is shutdown
> to have a reasonably consistent behavior.
>
> Fixes: d3476f3dad4a ("ext4: don't set SB_RDONLY after filesystem errors")
> Reported-by: Jan Stancek <jstancek@...hat.com>
> Link: https://lore.kernel.org/all/Zvp6L+oFnfASaoHl@t14s
> Signed-off-by: Jan Kara <jack@...e.cz>

In case the mount option isn't getting deprecated right away:
Tested-by: Jan Stancek <jstancek@...hat.com>


> ---
>  fs/ext4/super.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 16a4ce704460..4645f1629673 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6518,9 +6518,6 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
>                 goto restore_opts;
>         }
>
> -       if (test_opt2(sb, ABORT))
> -               ext4_abort(sb, ESHUTDOWN, "Abort forced by user");
> -
>         sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
>                 (test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0);
>
> @@ -6689,6 +6686,14 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
>         if (!ext4_has_feature_mmp(sb) || sb_rdonly(sb))
>                 ext4_stop_mmpd(sbi);
>
> +       /*
> +        * Handle aborting the filesystem as the last thing during remount to
> +        * avoid obsure errors during remount when some option changes fail to
> +        * apply due to shutdown filesystem.
> +        */
> +       if (test_opt2(sb, ABORT))
> +               ext4_abort(sb, ESHUTDOWN, "Abort forced by user");
> +
>         return 0;
>
>  restore_opts:
> --
> 2.35.3
>