Hi Pieter,
On Fri, Dec 29, 2017 at 04:19:21PM +0100, PiBa-NL wrote:
> Op 29-12-2017 om 11:35 schreef William Lallemand:
> > On Fri, Dec 29, 2017 at 10:46:40AM +0100, Willy Tarreau wrote:
> > > Hi William,
> > >
> > > In fact it still doesn't fclose() the streams, which worries me a little
> > > bit
> > > for the long term, because eventhough any printf() will end up being
> > > written
> > > into /dev/null, it's still preferable to mark the FILE* as being closed
> > > and
> > > only then reopen 0,1,2 to ensure they cannot be abusively reused. It will
> > > also remove some confusion in strace by avoiding seeing some spurious
> > > fcntl()
> > > or write(2, foo, strlen(foo)) being sent there caused by alerts for
> > > example.
> > > [...]
> > > Otherwise I'm reasonably confident that this should be enough to close
> > > all pending issues related to the master-worker now.
> > >
> > > Willy
> > >
> > I agree, it's better to merge them with fclose()
> >
> But shouldn't be needed, as i read dup2 will close them?
No, here's the catch : dup2() works on file descriptors, while stdin,
stdout and stderr are FILE*, which are in fact a wrapper provided by
glibc to present a stream on top of a file descriptor. So in short,
your FILE struct looks more or less like this :
struct FILE {
int fd;
char buffer[];
other stuff;
}
When you call fprintf(stderr, "foo"), what it does is to write "foo" to
a temporary buffer, then try to write the largest possible part of this
buffer's contents to fileno(stderr) which reports the fd number using
the write() syscall. If you just did a close() on fd #2, then an open
on /dev/null, fd #2 was effectively closed, but replaced by the new one
mapped to /dev/null. Then the next subsequent fprintf(stderr, "blah")
will do the same as above, and write(2, buffer, length). So the FILE
struct is still not closed by doing this. This is a common misconception
that is commonly encountered that close(0..2) is enough to cleanly close
the I/O streams, but it's not.
Performing an fclose() instead will close both the FILE and the fd. Then
we can reassign the fd to /dev/null be sure it's never assigned to anything
sensitive so that upon next reload it's totally safe.
Regards,
Willy
