Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20121024215245.GM254@brightrain.aerifal.cx>
Date: Wed, 24 Oct 2012 17:52:45 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: Possible file stream bug

On Wed, Oct 24, 2012 at 11:41:43PM +0200, Szabolcs Nagy wrote:
> * Paul Schutte <sjpschutte@...il.com> [2012-10-24 23:22:13 +0200]:
> > It is not my code, but I can not see why it is invalid.
> 
> beacuse the standard says so
> 
> http://port70.net/~nsz/c/c99/n1256.html#7.19.3p4
> 
> and in annex j.2 in the undefined behaviour list there is:
> "- The value of a pointer to a FILE object is used after the associated file is closed"
> 
> 
> also note that the freopen spec says that it closes the underlying
> file so there is no reason for a separate fclose (or fflush) anyway
> 
> http://port70.net/~nsz/c/c99/n1256.html#7.19.5.4p4
> 
> > Here is a strace when linked agains glibc:
> > 
> > close(1)                                = 0
> > open("/dev/tty", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 1
> > fstat(1, {st_mode=S_IFCHR|0666, st_rdev=makedev(5, 0), ...}) = 0
> > ioctl(1, SNDCTL_TMR_TIMEBASE or TCGETS, {B38400 opost isig icanon echo
> > ...}) = 0
> 
> glibc clearly does not do close in freopen
> 
> so it detects closed file streams and handles it in some way
> masking the error in the program

The issue is that musl does a dup2 followed by close to ensure that
freopen keeps the original file descriptor in a thread-safe way, and
it assumes the two file descriptor numbers passed to dup2 will not be
the same. This could also bite us in the case where fclose(f) had not
been called, but close(fileno(fd)) had been; such usage definitely
isn't valid in multi-threaded programs, and it's questionable to begin
with, but it does have some semi-legitimate uses in single-threaded
programs and might be worth supporting.

In any case, fixing that will not make the usage in libwebserver as
_supported_ usage (i.e. it may be subject to breakage at any time,
since it's invoking UB), but it might happen to make it work for now.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.