linux-ext4 - Re: [2/2] e2fsck: remove unnecessary reserve_stdio

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:   Mon, 13 Aug 2018 10:11:20 +0200
From:   Lukas Czerner <lczerner@...hat.com>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     linux-ext4@...r.kernel.org
Subject: Re: [2/2] e2fsck: remove unnecessary reserve_stdio_fds()

On Sat, Aug 11, 2018 at 08:47:43PM -0400, Theodore Y. Ts'o wrote:
> On Thu, Aug 09, 2018 at 10:35:46AM +0200, Lukas Czerner wrote:
> > Standard stream are always open, we do not need to "check" it
> > specifically. Remove reserve_stdio_fds().
> 
> I don't remember the exact circumstances, since it was twenty years
> ago, but that's not *always* true.  Remember that e2fsck is run by
> init or some other early boot sequence, and not all such programs
> are.... sane.  In fact, this was added because some buggy init called
> e2fsck with fd 2 closed, so the file system was opened using fd 2.
> And then an error message ended up corrupting the file system....

Uff, even though we do not really want to optimize for a broken code, it
does make sense to me to protect user from possible data loss in thi
crazy case.

> 
> This function was added in response to that failure, because
> sometimes, the world *is* out to get you.  It is fair to fix the file
> descriptor leak, so how about this?

Fair enough.

Reviewed-by: Lukas Czerner <lczerner@...hat.com>

Thanks!
-Lukas


> 
> 					- Ted
> 
> 
> commit 352701d9e2258299fe3bffb1d112566c0e4a7cdf
> Author: Theodore Ts'o <tytso@....edu>
> Date:   Sat Aug 11 20:47:08 2018 -0400
> 
>     e2fsck: fix fd leak in reserve_stdio_fds
>     
>     Signed-off-by: Theodore Ts'o <tytso@....edu>
> 
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 90065b395..2df22b171 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -617,9 +617,10 @@ static void reserve_stdio_fds(void)
>  			fprintf(stderr, _("ERROR: Couldn't open "
>  				"/dev/null (%s)\n"),
>  				strerror(errno));
> -			break;
> +			return;
>  		}
>  	}
> +	(void) close(fd);
>  }
>  
>  #ifdef HAVE_SIGNAL_H