From owner-freebsd-threads@FreeBSD.ORG Mon Dec 6 11:07:09 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 36B9E10656C5 for ; Mon, 6 Dec 2010 11:07:09 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 093598FC27 for ; Mon, 6 Dec 2010 11:07:09 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB6B78S2068385 for ; Mon, 6 Dec 2010 11:07:08 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB6B78wp068383 for freebsd-threads@FreeBSD.org; Mon, 6 Dec 2010 11:07:08 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 6 Dec 2010 11:07:08 GMT Message-Id: <201012061107.oB6B78wp068383@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-threads@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Dec 2010 11:07:09 -0000 Note: to view an individual PR, use: http://www.freebsd.org/cgi/query-pr.cgi?pr=(number). The following is a listing of current problems submitted by FreeBSD users. These represent problem reports covering all versions including experimental development code and obsolete releases. S Tracker Resp. Description -------------------------------------------------------------------------------- o threa/150959 threads [libc] Stub pthread_once in libc should call _libc_onc o threa/149366 threads pthread_cleanup_pop never runs the configured routine o threa/148515 threads Memory / syslog strangeness in FreeBSD 8.x ( possible o threa/141721 threads rtprio(1): (id|rt)prio priority resets when new thread o threa/136345 threads Recursive read rwlocks in thread A cause deadlock with o threa/135673 threads databases/mysql50-server - MySQL query lock-ups on 7.2 o threa/133734 threads 32 bit libthr failing pthread_create() o threa/128922 threads threads hang with xorg running o threa/127225 threads bug in lib/libthr/thread/thr_init.c o threa/122923 threads 'nice' does not prevent background process from steali o threa/121336 threads lang/neko threading ok on UP, broken on SMP (FreeBSD 7 o threa/116668 threads can no longer use jdk15 with libthr on -stable SMP o threa/116181 threads /dev/io-related io access permissions are not propagat o threa/115211 threads pthread_atfork misbehaves in initial thread o threa/110636 threads [request] gdb(1): using gdb with multi thread applicat o threa/110306 threads apache 2.0 segmentation violation when calling gethost o threa/103975 threads Implicit loading/unloading of libpthread.so may crash o threa/101323 threads [patch] fork(2) in threaded programs broken. f threa/100815 threads FBSD 5.5 broke nanosleep in libc_r s threa/84483 threads problems with devel/nspr and -lc_r on 4.x o threa/80992 threads abort() sometimes not caught by gdb depending on threa o threa/79887 threads [patch] freopen() isn't thread-safe o threa/79683 threads svctcp_create() fails if multiple threads call at the s threa/76694 threads fork cause hang in dup()/close() function in child (-l s threa/76690 threads fork hang in child for -lc_r s threa/69020 threads pthreads library leaks _gc_mutex s threa/48856 threads Setting SIGCHLD to SIG_IGN still leaves zombies under s threa/40671 threads pthread_cancel doesn't remove thread from condition qu s threa/34536 threads accept() blocks other threads s threa/30464 threads pthread mutex attributes -- pshared 30 problems total. From owner-freebsd-threads@FreeBSD.ORG Tue Dec 7 07:50:11 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AC692106564A for ; Tue, 7 Dec 2010 07:50:11 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 82CB38FC18 for ; Tue, 7 Dec 2010 07:50:11 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB77oBsS087316 for ; Tue, 7 Dec 2010 07:50:11 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB77oB8m087315; Tue, 7 Dec 2010 07:50:11 GMT (envelope-from gnats) Date: Tue, 7 Dec 2010 07:50:11 GMT Message-Id: <201012070750.oB77oB8m087315@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Vasil Dimov Cc: 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: Vasil Dimov List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Dec 2010 07:50:11 -0000 The following reply was made to PR threads/79887; it has been noted by GNATS. From: Vasil Dimov To: bug-followup@FreeBSD.org, tejblum@yandex-team.ru Cc: Subject: Re: threads/79887: [patch] freopen() isn't thread-safe Date: Tue, 7 Dec 2010 09:42:02 +0200 --jRHKVT23PllUwdXP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi, This bug is biting MySQL badly (data corruption due to writing to the wrong file). See http://bugs.mysql.com/51023 Bug#51023 Mysql server crashes on SIGHUP and destroys InnoDB files Attaching a simple prog to reproduce this (tested on 8.1-STABLE/amd64). -- Vasil Dimov gro.DSBeerF@dv % The difference between life and the movies is that a script has to make sense, and life doesn't. -- Joseph L. Mankiewicz --jRHKVT23PllUwdXP Content-Type: text/x-csrc; charset=us-ascii Content-Disposition: attachment; filename="pr79887.c" #include #include #include #include #include #include #include #include #include FILE* l; void* openmany(void* p) { int fds[1500]; unsigned long long i, j; char filename[100]; for (i = 0; ; i++) { for (j = 0; j < 1500; j++) { snprintf(filename, sizeof(filename), "/tmp/ib%04llu", j); fds[j] = open(filename, O_RDWR | O_CREAT, 0644); fprintf(l, "open(): %s: %d\n", filename, fds[j]); if (fds[j] == -1) { fprintf(l, "open(): %s: %s\n", filename, strerror(errno)); exit(1); } write(fds[j], "ABCABCABC", 9); } for (j = 0; j < 1500; j++) { int ret; ret = close(fds[j]); fprintf(l, "close(): %d: %d\n", fds[j], ret); if (ret == -1) { fprintf(l, "error: close(%d): %s\n", fds[j], strerror(errno)); exit(1); } } } } void* reopenstd(void* p) { unsigned long long i; for (i = 0; ; i++) { freopen("/tmp/std_redirected", "a+", stdout); freopen("/tmp/std_redirected", "a+", stderr); printf("XXXXXXXXXXX %llu\n", i); } return(NULL); } int main(int argc, char **argv, char **envp) { pthread_t t1; pthread_t t2; void* ret; l = fopen("/tmp/log", "w"); if (l == NULL) { fprintf(stderr, "error: fopen(): /tmp/log: %s\n", strerror(errno)); exit(1); } pthread_create(&t1, NULL, reopenstd, NULL); pthread_create(&t2, NULL, openmany, NULL); pthread_join(t1, &ret); pthread_join(t2, &ret); fclose(l); return(0); } --jRHKVT23PllUwdXP-- From owner-freebsd-threads@FreeBSD.ORG Tue Dec 7 10:00:28 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 25D36106566B for ; Tue, 7 Dec 2010 10:00:28 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id EF3888FC14 for ; Tue, 7 Dec 2010 10:00:27 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB7A0R6h025618 for ; Tue, 7 Dec 2010 10:00:27 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB7A0RqX025593; Tue, 7 Dec 2010 10:00:27 GMT (envelope-from gnats) Date: Tue, 7 Dec 2010 10:00:27 GMT Message-Id: <201012071000.oB7A0RqX025593@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Vasil Dimov Cc: 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: Vasil Dimov List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Dec 2010 10:00:28 -0000 The following reply was made to PR threads/79887; it has been noted by GNATS. From: Vasil Dimov To: bug-followup@FreeBSD.org, tejblum@yandex-team.ru Cc: Subject: Re: threads/79887: [patch] freopen() isn't thread-safe Date: Tue, 7 Dec 2010 11:52:00 +0200 --wac7ysb48OaltWcw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Attached is a prog which demonstrates what happens when another thread calls open() when freopen() is executing. The output is: open(): /tmp/f1: 3 trx1: freopen() begins, will associate fd=3 with /tmp/f2 trx1: freopen(): open(): /tmp/f2: 4 trx1: freopen(): close(): 3: 0 trx2: open(): /tmp/f_other: 3 trx1: freopen(): dup2(): 4,3: 3 trx1: freopen(): end, now fd=3 is associated with /tmp/f2 trx1: write to fd=3 (supposed to be /tmp/f2) trx2: write to fd=3 (supposed to be /tmp/f_other) At the end /tmp/f2 contains "ABCXYZ", /tmp/f_other is empty. -- Vasil Dimov gro.DSBeerF@dv % The difference between life and the movies is that a script has to make sense, and life doesn't. -- Joseph L. Mankiewicz --wac7ysb48OaltWcw Content-Type: text/x-csrc; charset=us-ascii Content-Disposition: attachment; filename="dup2.c" #include #include #include #include #include int main(int argc, char **argv) { int fd1; int fd1_orig; int fd_other; int ret; fd1 = open("/tmp/f1", O_RDWR | O_CREAT, 0644); printf("open(): /tmp/f1: %d\n", fd1); printf("trx1: freopen() begins, will associate fd=%d with /tmp/f2\n", fd1); fd1_orig = fd1; fd1 = open("/tmp/f2", O_RDWR | O_CREAT, 0644); printf("trx1: freopen(): open(): /tmp/f2: %d\n", fd1); ret = close(fd1_orig); printf("trx1: freopen(): close(): %d: %d\n", fd1_orig, ret); /* another thread executes in the meantime, opening some file */ fd_other = open("/tmp/f_other", O_RDWR | O_CREAT, 0644); printf("trx2: open(): /tmp/f_other: %d\n", fd_other); /* end of another thread, freopen() continues */ ret = dup2(fd1, fd1_orig); printf("trx1: freopen(): dup2(): %d,%d: %d\n", fd1, fd1_orig, ret); fd1 = fd1_orig; printf("trx1: freopen(): end, now fd=%d is associated with /tmp/f2\n", fd1); printf("trx1: write to fd=%d (supposed to be /tmp/f2)\n", fd1); write(fd1, "ABC", 3); printf("trx2: write to fd=%d (supposed to be /tmp/f_other)\n", fd_other); write(fd_other, "XYZ", 3); close(fd1); close(fd_other); return(0); } --wac7ysb48OaltWcw-- From owner-freebsd-threads@FreeBSD.ORG Tue Dec 7 14:40:12 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 87D5610656B0 for ; Tue, 7 Dec 2010 14:40:12 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 555008FC23 for ; Tue, 7 Dec 2010 14:40:12 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB7EeCi0018512 for ; Tue, 7 Dec 2010 14:40:12 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB7EeCk5018511; Tue, 7 Dec 2010 14:40:12 GMT (envelope-from gnats) Date: Tue, 7 Dec 2010 14:40:12 GMT Message-Id: <201012071440.oB7EeCk5018511@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: 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: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Dec 2010 14:40:14 -0000 The following reply was made to PR threads/79887; it has been noted by GNATS. From: John Baldwin To: bug-followup@freebsd.org, tejblum@yandex-team.ru Cc: David Xu Subject: Re: threads/79887: [patch] freopen() isn't thread-safe Date: Tue, 7 Dec 2010 09:31:36 -0500 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. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Wed Dec 8 02:50:06 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 818731065696 for ; Wed, 8 Dec 2010 02:50:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 50DF38FC16 for ; Wed, 8 Dec 2010 02:50:06 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB82o6ZX072937 for ; Wed, 8 Dec 2010 02:50:06 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB82o6X5072936; Wed, 8 Dec 2010 02:50:06 GMT (envelope-from gnats) Date: Wed, 8 Dec 2010 02:50:06 GMT Message-Id: <201012080250.oB82o6X5072936@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: David Xu Cc: 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: David Xu List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2010 02:50:06 -0000 The following reply was made to PR threads/79887; it has been noted by GNATS. From: David Xu To: John Baldwin Cc: bug-followup@freebsd.org, tejblum@yandex-team.ru Subject: Re: threads/79887: [patch] freopen() isn't thread-safe Date: Wed, 08 Dec 2010 10:43:35 +0800 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. From owner-freebsd-threads@FreeBSD.ORG Wed Dec 8 04:08:11 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 B0A11106564A; Wed, 8 Dec 2010 04:08:11 +0000 (UTC) (envelope-from davidxu@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 9F2998FC0C; Wed, 8 Dec 2010 04:08:11 +0000 (UTC) Received: from xyf.my.dom (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB848ARx056082; Wed, 8 Dec 2010 04:08:11 GMT (envelope-from davidxu@freebsd.org) Message-ID: <4CFF04AA.6060905@freebsd.org> Date: Wed, 08 Dec 2010 12:08:10 +0800 From: David Xu User-Agent: Thunderbird 2.0.0.24 (X11/20100630) MIME-Version: 1.0 To: Daniel Eischen References: <201012080250.oB82o6X5072936@freefall.freebsd.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2010 04:08:11 -0000 Daniel Eischen wrote: > 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? > Thanks, I think you can keep it. 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 From owner-freebsd-threads@FreeBSD.ORG Wed Dec 8 15:20:13 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6DE0E1065693 for ; Wed, 8 Dec 2010 15:20:13 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 42F2F8FC1C for ; Wed, 8 Dec 2010 15:20:13 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB8FKDBF088587 for ; Wed, 8 Dec 2010 15:20:13 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB8FKDB2088586; Wed, 8 Dec 2010 15:20:13 GMT (envelope-from gnats) Date: Wed, 8 Dec 2010 15:20:13 GMT Message-Id: <201012081520.oB8FKDB2088586@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: 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: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2010 15:20:13 -0000 The following reply was made to PR threads/79887; it has been noted by GNATS. From: John Baldwin To: David Xu Cc: bug-followup@freebsd.org, tejblum@yandex-team.ru Subject: Re: threads/79887: [patch] freopen() isn't thread-safe Date: Wed, 8 Dec 2010 10:03:34 -0500 On Tuesday, December 07, 2010 9:43:35 pm 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. Actually, the freopen() code honors custom _close() routines earlier when it checks for _file being < 0. I do really think this is ok. _close() is not public, it is only allowed to be set via funopen(). We also need the dup2() change to effectively implement this function's rationale, which is a way to redirect stdin, stdout, and stderr. I will take care of committing this today, with an extra bit of comment. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Wed Dec 8 15:39:19 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 C0301106564A for ; Wed, 8 Dec 2010 15:39:19 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 8F5728FC12 for ; Wed, 8 Dec 2010 15:39:19 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 3B38E46B35 for ; Wed, 8 Dec 2010 10:39:19 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 2D6478A01D for ; Wed, 8 Dec 2010 10:39:18 -0500 (EST) From: John Baldwin To: freebsd-threads@freebsd.org Date: Wed, 8 Dec 2010 10:39:16 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20101102; KDE/4.4.5; amd64; ; ) References: <201012081520.oB8FKDB2088586@freefall.freebsd.org> In-Reply-To: <201012081520.oB8FKDB2088586@freefall.freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201012081039.16470.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Wed, 08 Dec 2010 10:39:18 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Subject: Re: threads/79887: [patch] freopen() isn't thread-safe X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Dec 2010 15:39:19 -0000 On Wednesday, December 08, 2010 10:20:13 am John Baldwin wrote: > The following reply was made to PR threads/79887; it has been noted by GNATS. > > From: John Baldwin > To: David Xu > Cc: bug-followup@freebsd.org, > tejblum@yandex-team.ru > Subject: Re: threads/79887: [patch] freopen() isn't thread-safe > Date: Wed, 8 Dec 2010 10:03:34 -0500 > > On Tuesday, December 07, 2010 9:43:35 pm 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. > > Actually, the freopen() code honors custom _close() routines earlier when it > checks for _file being < 0. I do really think this is ok. _close() is not > public, it is only allowed to be set via funopen(). We also need the dup2() > change to effectively implement this function's rationale, which is a way to > redirect stdin, stdout, and stderr. > > I will take care of committing this today, with an extra bit of comment. I hadn't seen Daniel's reply when I wrote this last bit. Here is a slightly updated version of the patch with an extra comment and a change to use _close() directly: Index: freopen.c =================================================================== --- freopen.c (revision 216266) +++ freopen.c (working copy) @@ -150,14 +150,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: @@ -165,9 +157,11 @@ * 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 @@ 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 @@ * 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); } /* We could use 'wantfd' instead of 'fp->_file' in the last hunk above if folks feel that is clearer. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Dec 9 20:30:15 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7E453106566B for ; Thu, 9 Dec 2010 20:30:15 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 6D3F38FC15 for ; Thu, 9 Dec 2010 20:30:15 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB9KUFaX042423 for ; Thu, 9 Dec 2010 20:30:15 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB9KUFTv042418; Thu, 9 Dec 2010 20:30:15 GMT (envelope-from gnats) Date: Thu, 9 Dec 2010 20:30:15 GMT Message-Id: <201012092030.oB9KUFTv042418@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: dfilter@FreeBSD.ORG (dfilter service) Cc: Subject: Re: threads/79887: commit references a PR X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: dfilter service List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Dec 2010 20:30:15 -0000 The following reply was made to PR threads/79887; it has been noted by GNATS. From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@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@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From owner-freebsd-threads@FreeBSD.ORG Thu Dec 9 20:46:22 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 38CF0106566C; Thu, 9 Dec 2010 20:46:22 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 0F5358FC13; Thu, 9 Dec 2010 20:46:22 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oB9KkL5q064445; Thu, 9 Dec 2010 20:46:21 GMT (envelope-from jhb@freefall.freebsd.org) Received: (from jhb@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oB9KkLrh064441; Thu, 9 Dec 2010 20:46:21 GMT (envelope-from jhb) Date: Thu, 9 Dec 2010 20:46:21 GMT Message-Id: <201012092046.oB9KkLrh064441@freefall.freebsd.org> To: tejblum@yandex-team.ru, jhb@FreeBSD.org, freebsd-threads@FreeBSD.org, jhb@FreeBSD.org From: jhb@FreeBSD.org Cc: Subject: Re: threads/79887: [patch] freopen() isn't thread-safe X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Dec 2010 20:46:22 -0000 Synopsis: [patch] freopen() isn't thread-safe State-Changed-From-To: open->patched State-Changed-By: jhb State-Changed-When: Thu Dec 9 20:45:50 UTC 2010 State-Changed-Why: Fix committed to HEAD, will attempt to merge it for 7.4 and 8.2. Responsible-Changed-From-To: freebsd-threads->jhb Responsible-Changed-By: jhb Responsible-Changed-When: Thu Dec 9 20:45:50 UTC 2010 Responsible-Changed-Why: Fix committed to HEAD, will attempt to merge it for 7.4 and 8.2. http://www.freebsd.org/cgi/query-pr.cgi?pr=79887