| Message ID | 20180809083546.17419-2-lczerner@redhat.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [1/2] e2fsprogs: remove unused datarootdir | expand |
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.... 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? - Ted commit 352701d9e2258299fe3bffb1d112566c0e4a7cdf Author: Theodore Ts'o <tytso@mit.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@mit.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
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@redhat.com> Thanks! -Lukas > > - Ted > > > commit 352701d9e2258299fe3bffb1d112566c0e4a7cdf > Author: Theodore Ts'o <tytso@mit.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@mit.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
diff --git a/e2fsck/unix.c b/e2fsck/unix.c index 90065b39..268feee0 100644 --- a/e2fsck/unix.c +++ b/e2fsck/unix.c @@ -603,25 +603,6 @@ static int e2fsck_update_progress(e2fsck_t ctx, int pass, #define PATH_SET "PATH=/sbin" -/* - * Make sure 0,1,2 file descriptors are open, so that we don't open - * the filesystem using the same file descriptor as stdout or stderr. - */ -static void reserve_stdio_fds(void) -{ - int fd = 0; - - while (fd <= 2) { - fd = open("/dev/null", O_RDWR); - if (fd < 0) { - fprintf(stderr, _("ERROR: Couldn't open " - "/dev/null (%s)\n"), - strerror(errno)); - break; - } - } -} - #ifdef HAVE_SIGNAL_H static void signal_progress_on(int sig EXT2FS_ATTR((unused))) { @@ -1411,7 +1392,6 @@ int main (int argc, char *argv[]) _("while trying to initialize program")); exit(FSCK_ERROR); } - reserve_stdio_fds(); set_up_logging(ctx); if (ctx->logf) {
Standard stream are always open, we do not need to "check" it specifically. Remove reserve_stdio_fds(). This also fixes the file descriptor leak. Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- e2fsck/unix.c | 20 -------------------- 1 file changed, 20 deletions(-)