From owner-freebsd-threads@FreeBSD.ORG Wed Dec 8 04:13:37 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6CA531065679 for ; Wed, 8 Dec 2010 04:13:37 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.10]) by mx1.freebsd.org (Postfix) with ESMTP id 29DD98FC15 for ; Wed, 8 Dec 2010 04:13:36 +0000 (UTC) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.14.4/8.14.4/NETPLEX) with ESMTP id oB83qEHB023167; Tue, 7 Dec 2010 22:52:14 -0500 (EST) X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.2.2 (mail.netplex.net [204.213.176.10]); Tue, 07 Dec 2010 22:52:14 -0500 (EST) Date: Tue, 7 Dec 2010 22:52:14 -0500 (EST) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net To: David Xu In-Reply-To: <201012080250.oB82o6X5072936@freefall.freebsd.org> Message-ID: References: <201012080250.oB82o6X5072936@freefall.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-threads@freebsd.org Subject: Re: threads/79887: [patch] freopen() isn't thread-safe X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Daniel Eischen List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2010 04:13:37 -0000 On Wed, 8 Dec 2010, David Xu wrote: > John Baldwin wrote: > > David, > > > > I think the submitter's analysis is correct that the only place that can set > > the close function pointer is funopen() and that for that case (and any other > > "fake" files), the file descriptor will be -1. If the fd is >= 0, then it > > must be a file-descriptor-backed FILE, and relying on dup2() to close the fd > > is ok. > > > > As the manpage notes, the most common usage is to redirect stderr or stdout by > > doing 'freopen("/dev/null", "w", stderr)'. The bug allows some other random > > code that is calling open() in another thread to have that open() return 2 > > during the window where fd '2' is closed during freopen(). That other file > > descriptor then gets trounced by the dup2() call in freopen() to point to > > something else. > > > > The code likely uses _close() rather than close() directly to be cleaner. > > Given that this is stdio, I don't think we are really worried about the > > performance impact of one extra wrapper function. > > > > I think the original patch is most likely correct. > > > > The patch works, I just don't like the design of the > (*fp->_close)(fp->_cookie) > it seems the patch make freopen bypass it. > I think the patch can be committed, but I am busy and have > no time to do it by myself. I can do it, but will be away on vacation until later next week. If you want to wait, I can commit it. Would you like to replace the (*fp->_close)(fp->_cookie) with just _close(fp->_file) as you suggest in one of your followups? -- DE