From owner-freebsd-bugs@FreeBSD.ORG Wed Jun 20 07:40:09 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 314DA1065670 for ; Wed, 20 Jun 2012 07:40:09 +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 09E948FC14 for ; Wed, 20 Jun 2012 07:40:09 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q5K7e8df012237 for ; Wed, 20 Jun 2012 07:40:08 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q5K7e8A7012236; Wed, 20 Jun 2012 07:40:08 GMT (envelope-from gnats) Resent-Date: Wed, 20 Jun 2012 07:40:08 GMT Resent-Message-Id: <201206200740.q5K7e8A7012236@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Jukka Ukkonen Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A9230106566C for ; Wed, 20 Jun 2012 07:32:59 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from red.freebsd.org (red.freebsd.org [IPv6:2001:4f8:fff6::22]) by mx1.freebsd.org (Postfix) with ESMTP id 92F798FC12 for ; Wed, 20 Jun 2012 07:32:59 +0000 (UTC) Received: from red.freebsd.org (localhost [127.0.0.1]) by red.freebsd.org (8.14.4/8.14.4) with ESMTP id q5K7Wxsx078336 for ; Wed, 20 Jun 2012 07:32:59 GMT (envelope-from nobody@red.freebsd.org) Received: (from nobody@localhost) by red.freebsd.org (8.14.4/8.14.4/Submit) id q5K7WxAo078335; Wed, 20 Jun 2012 07:32:59 GMT (envelope-from nobody) Message-Id: <201206200732.q5K7WxAo078335@red.freebsd.org> Date: Wed, 20 Jun 2012 07:32:59 GMT From: Jukka Ukkonen To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: misc/169259: libfetch to use O_CLOEXEC to avoid file descriptor leaks in threaded code X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Jun 2012 07:40:09 -0000 >Number: 169259 >Category: misc >Synopsis: libfetch to use O_CLOEXEC to avoid file descriptor leaks in threaded code >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Jun 20 07:40:08 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Jukka Ukkonen >Release: FreeBSD 9.0-STABLE >Organization: ----- >Environment: FreeBSD sleipnir 9.0-STABLE FreeBSD 9.0-STABLE #0: Tue Jun 19 22:26:46 EEST 2012 root@sleipnir:/usr/obj/usr/src/sys/Sleipnir amd64 >Description: Libfetch has been using a call sequence ... f = fopen (...); fcntl (fileno(f), F_SETFD, FD_CLOEXEC); when opening files. This leaves a short time window which in threaded code could lead to the newly opened file descriptor leaking to the child program, if another thread happens to call exec(). An alternative approach avoiding the potential fd leak is fd = open (..., O_CLOEXEC); .. f = fdopen (fd, ...); >How-To-Repeat: See full description. >Fix: Find a patch attached. Patch attached with submission follows: --- lib/libfetch/file.c.orig 2012-05-26 19:34:39.000000000 +0300 +++ lib/libfetch/file.c 2012-06-19 21:28:46.000000000 +0300 @@ -44,11 +44,17 @@ fetchXGetFile(struct url *u, struct url_stat *us, const char *flags) { FILE *f; + int fd; if (us && fetchStatFile(u, us, flags) == -1) return (NULL); - f = fopen(u->doc, "r"); + fd = open (u->doc, (O_RDONLY | O_CLOEXEC)); + + if (fd == -1) + fetch_syserr(); + + f = fdopen (fd, "r"); if (f == NULL) fetch_syserr(); @@ -58,7 +64,6 @@ fetch_syserr(); } - fcntl(fileno(f), F_SETFD, FD_CLOEXEC); return (f); } @@ -72,11 +77,36 @@ fetchPutFile(struct url *u, const char *flags) { FILE *f; + int fd; + int fdflags; + const char *mode; + + fdflags = (O_CREAT | O_CLOEXEC); + + if (CHECK_FLAG('a')) { + /* + * Match the functionality of + * f = fopen(u->doc, "a"); + */ + fdflags |= (O_WRONLY | O_APPEND); + mode = "a"; + } + else { + /* + * Match the functionality of + * f = fopen(u->doc, "w+"); + */ + + fdflags = (O_RDWR | O_TRUNC); + mode = "w+"; + } + + fd = open (u->doc, fdflags); + + if (fd == -1) + fetch_syserr(); - if (CHECK_FLAG('a')) - f = fopen(u->doc, "a"); - else - f = fopen(u->doc, "w+"); + f = fdopen (fd, mode); if (f == NULL) fetch_syserr(); @@ -86,7 +116,6 @@ fetch_syserr(); } - fcntl(fileno(f), F_SETFD, FD_CLOEXEC); return (f); } >Release-Note: >Audit-Trail: >Unformatted: