linux-ext4 - Re: [PATCH] fix "fsck -A" failure on a completely clean fs

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] [day] [month] [year] [list]

Date:   Fri, 10 May 2019 15:50:37 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Denys Vlasenko <dvlasenk@...hat.com>
Cc:     linux-ext4@...r.kernel.org
Subject: Re: [PATCH] fix "fsck -A" failure on a completely clean fs

I've replied on the bug:

	https://bugzilla.redhat.com/show_bug.cgi?id=1702342#c9

TL;DR: NACK

In theory you might be able to patch up e2fsck so that fsck -A
actually would work for ext[234].  It is not obvious to me that it
will work for other file systems, however --- due to some very subtle
issues such as the ones I pointed out in my reply.

Cheers,

					- Ted


On Fri, May 10, 2019 at 04:19:47PM +0200, Denys Vlasenko wrote:
> Before remounting root fs and mounting local filesystems
> in /etc/fstab, my boot scripts check them for errors with:
> 
> if ! fsck -A; then
>         echo "fsck exit code: $?. Boot will not continue."
>         while true; do sleep 9999; done
> fi
> mount -o remount,rw /
> mount -a
> 
> Looks like something very straightforward, right?
> 
> I have two filesystems in /etc/fstab:
> /dev/sda2 /                       ext4    defaults        1 1
> /dev/sda1 /boot                   ext4    defaults        1 2
> 
> If I use fsck from util-linux-2.33.2-1.fc31.x86_64 (IOW: rather recent code),
> it starts two instances of fsck.ext4 (all is fine with it).
> 
> The second one's stdio is redirected (probably to /dev/null),
> it is no longer the tty. (Which is fine too).
> 
> But now we hit a problem. Second fsck.ext4 flat out refuses to do its job,
> even before it looks at the filesystem.
> 
> Therefore, this condition does not trigger:
>         if (getenv("E2FSCK_FORCE_INTERACTIVE") || (isatty(0) && isatty(1))) {
>                 ctx->interactive = 1;
>         }
> and ctx->interactive stays 0.
> Therefore, later in main() fsck.ext4 dies with this message:
>         if (!(ctx->options & E2F_OPT_PREEN) &&
>             !(ctx->options & E2F_OPT_NO) &&
>             !(ctx->options & E2F_OPT_YES)) {
>                 if (!ctx->interactive)
>                         fatal_error(ctx,
>                                     _("need terminal for interactive repairs"));
>         }
> 
> This happens BEFORE any repairs are deemed necessary, IOW: it happens ALWAYS,
> even if filesystem is completely fine.
> 
> IOW: "fsck -A" is *completely unusable* in this scenario.
> I believe this is wrong. It is intended to be the generic, fs-agnostic way
> to run fsck's on all /etc/fstab filesystems.
> 
> I propose to change the code so that this abort happens only if we
> indeed need to interactively ask something.
> 
> Tested patch attached.
> 
> Fedora BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1702342
> 

> diff -uprN e2fsprogs-1.44.4.orig/e2fsck/unix.c e2fsprogs-1.44.4/e2fsck/unix.c
> --- e2fsprogs-1.44.4.orig/e2fsck/unix.c	2018-08-19 04:26:58.000000000 +0200
> +++ e2fsprogs-1.44.4/e2fsck/unix.c	2019-04-23 15:38:55.890507270 +0200
> @@ -1439,13 +1439,6 @@ int main (int argc, char *argv[])
>  
>  	check_mount(ctx);
>  
> -	if (!(ctx->options & E2F_OPT_PREEN) &&
> -	    !(ctx->options & E2F_OPT_NO) &&
> -	    !(ctx->options & E2F_OPT_YES)) {
> -		if (!ctx->interactive)
> -			fatal_error(ctx,
> -				    _("need terminal for interactive repairs"));
> -	}
>  	ctx->superblock = ctx->use_superblock;
>  
>  	flags = EXT2_FLAG_SKIP_MMP;
> diff -uprN e2fsprogs-1.44.4.orig/e2fsck/util.c e2fsprogs-1.44.4/e2fsck/util.c
> --- e2fsprogs-1.44.4.orig/e2fsck/util.c	2018-08-19 04:26:58.000000000 +0200
> +++ e2fsprogs-1.44.4/e2fsck/util.c	2019-04-23 15:39:27.571448855 +0200
> @@ -203,6 +203,14 @@ int ask_yn(e2fsck_t ctx, const char * st
>  	const char	*extra_prompt = "";
>  	static int	yes_answers;
>  
> +	if (!(ctx->options & E2F_OPT_PREEN) &&
> +	    !(ctx->options & E2F_OPT_NO) &&
> +	    !(ctx->options & E2F_OPT_YES)) {
> +		if (!ctx->interactive)
> +			fatal_error(ctx,
> +				    _("need terminal for interactive repairs"));
> +	}
> +
>  #ifdef HAVE_TERMIOS_H
>  	struct termios	termios, tmp;
>