threads/79887: [patch] freopen() isn't thread-safe

[Dmitrij Tejblum]

>Number:         79887
>Category:       threads
>Synopsis:       [patch] freopen() isn't thread-safe
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-threads
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Apr 13 23:20:26 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dmitrij Tejblum
>Release:        FreeBSD 5.4-STABLE i386
>Organization:
>Environment:

>Description:
The freopen() function 
(1) open the new file, 
(2) close the old file
(3) dup2() the newly opened file to the file descriptor (wantfd) previously
    held by the old file. (As comments says, "Various C library routines
    assume stderr is always fd STDERR_FILENO").
So, between step (2) and step (3) the wantfd file descriptor is closed, and
if another thread open a file it make take the wantfd descriptor. Then, after
the dup2() call in step (3) above, a mess will happen.
>How-To-Repeat:

>Fix:
Don't close the old file descriptor, the dup2() call will do the job.

Index: freopen.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdio/freopen.c,v
retrieving revision 1.13
diff -u -r1.13 freopen.c
--- freopen.c	22 May 2004 15:19:41 -0000	1.13
+++ freopen.c	13 Apr 2005 22:56:48 -0000
@@ -155,14 +155,6 @@
 
 	/* Get a new descriptor to refer to the new file. */
 	f = _open(file, oflags, DEFFILEMODE);
-	if (f < 0 && isopen) {
-		/* If out of fd's close the old one and try again. */
-		if (errno == ENFILE || errno == EMFILE) {
-			(void) (*fp->_close)(fp->_cookie);
-			isopen = 0;
-			f = _open(file, oflags, DEFFILEMODE);
-		}
-	}
 	sverrno = errno;
 
 finish:
@@ -171,8 +163,6 @@
 	 * keep fp->_base: it may be the wrong size.  This loses the effect
 	 * of any setbuffer calls, but stdio has always done this before.
 	 */
-	if (isopen)
-		(void) (*fp->_close)(fp->_cookie);
 	if (fp->_flags & __SMBF)
 		free((char *)fp->_bf._base);
 	fp->_w = 0;
@@ -191,6 +181,8 @@
 	memset(&fp->_extra->mbstate, 0, sizeof(mbstate_t));
 
 	if (f < 0) {			/* did not get it after all */
+		if (isopen)
+			(void) (*fp->_close)(fp->_cookie);
 		fp->_flags = 0;		/* set it free */
 		errno = sverrno;	/* restore in case _close clobbered */
 		FUNLOCKFILE(fp);
@@ -202,10 +194,13 @@
 	 * to maintain the descriptor.  Various C library routines (perror)
 	 * assume stderr is always fd STDERR_FILENO, even if being freopen'd.
 	 */
-	if (wantfd >= 0 && f != wantfd) {
+	if (wantfd >= 0) {
 		if (_dup2(f, wantfd) >= 0) {
 			(void)_close(f);
 			f = wantfd;
+		} else {
+			if (isopen)
+				(void) (*fp->_close)(fp->_cookie);
 		}
 	}
 

>Release-Note:
>Audit-Trail:
>Unformatted: