threads/79887: commit references a PR

[dfilter service]

The following reply was made to PR threads/79887; it has been noted by GNATS.

From: dfilter at FreeBSD.ORG (dfilter service)
To: bug-followup at FreeBSD.org
Cc:  
Subject: Re: threads/79887: commit references a PR
Date: Thu,  9 Dec 2010 20:28:37 +0000 (UTC)

 Author: jhb
 Date: Thu Dec  9 20:28:30 2010
 New Revision: 216334
 URL: http://svn.freebsd.org/changeset/base/216334
 
 Log:
   When reopening a stream backed by an open file descriptor, do not close
   the existing file descriptor.  Instead, let dup2() atomically close the
   old file descriptor when assigning the newly opened file to the same
   descriptor.  This closes a race in a multithreaded application where a
   concurrent open() could allocate the existing file descriptor in between
   the calls to close() and dup2().
   
   PR:		threads/79887
   Submitted by:	Dmitrij Tejblum  tejblum of yandex-team.ru
   Reviewed by:	davidxu
   MFC after:	1 week
 
 Modified:
   head/lib/libc/stdio/freopen.c
 
 Modified: head/lib/libc/stdio/freopen.c
 ==============================================================================
 --- head/lib/libc/stdio/freopen.c	Thu Dec  9 20:16:00 2010	(r216333)
 +++ head/lib/libc/stdio/freopen.c	Thu Dec  9 20:28:30 2010	(r216334)
 @@ -150,14 +150,6 @@ freopen(file, mode, fp)
  
  	/* 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:
 @@ -165,9 +157,11 @@ finish:
  	 * Finish closing fp.  Even if the open succeeded above, we cannot
  	 * keep fp->_base: it may be the wrong size.  This loses the effect
  	 * of any setbuffer calls, but stdio has always done this before.
 +	 *
 +	 * Leave the existing file descriptor open until dup2() is called
 +	 * below to avoid races where a concurrent open() in another thread
 +	 * could claim the existing descriptor.
  	 */
 -	if (isopen)
 -		(void) (*fp->_close)(fp->_cookie);
  	if (fp->_flags & __SMBF)
  		free((char *)fp->_bf._base);
  	fp->_w = 0;
 @@ -186,6 +180,8 @@ finish:
  	memset(&fp->_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 */
  		FUNLOCKFILE(fp);
  		errno = sverrno;	/* restore in case _close clobbered */
 @@ -197,11 +193,12 @@ finish:
  	 * 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
 +			(void)_close(fp->_file);
  	}
  
  	/*
 _______________________________________________
 svn-src-all at freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/svn-src-all
 To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"