Date: Wed, 20 Jun 2012 07:32:59 GMT From: Jukka Ukkonen <jau@iki.fi> To: freebsd-gnats-submit@FreeBSD.org Subject: misc/169259: libfetch to use O_CLOEXEC to avoid file descriptor leaks in threaded code Message-ID: <201206200732.q5K7WxAo078335@red.freebsd.org> Resent-Message-ID: <201206200740.q5K7e8A7012236@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201206200732.q5K7WxAo078335>