Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Commit

Permalink
fs: uv__cloexec() opened fd
Browse files Browse the repository at this point in the history
Every file descriptor opened using libuv should be automatically marked
as CLOEXEC to prevent it from leaking to a child process. Note that
since we are opening fds in a thread pool, there is a possible race
condition between `uv_spawn()` and the `open()` + `uv__cloexec()`. The
rwlock was added to avoid it.

see nodejs/node-v0.x-archive#6905
  • Loading branch information
indutny committed Jan 31, 2014
1 parent 6c525df commit 6abe1e4
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 1 deletion.
1 change: 1 addition & 0 deletions include/uv-unix.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ typedef struct {
void* wq[2]; \
uv_mutex_t wq_mutex; \
uv_async_t wq_async; \
uv_rwlock_t cloexec_lock; \
uv_handle_t* closing_handles; \
void* process_handles[1][2]; \
void* prepare_handles[2]; \
Expand Down
33 changes: 32 additions & 1 deletion src/unix/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,9 @@ static void uv__fs_work(struct uv__work* w) {
int retry_on_eintr;
uv_fs_t* req;
ssize_t r;
#ifdef O_CLOEXEC
static int no_cloexec_support;
#endif /* O_CLOEXEC */

req = container_of(w, uv_fs_t, work_req);
retry_on_eintr = !(req->fs_type == UV_FS_CLOSE);
Expand All @@ -634,7 +637,6 @@ static void uv__fs_work(struct uv__work* w) {
X(LSTAT, uv__fs_lstat(req->path, &req->statbuf));
X(LINK, link(req->path, req->new_path));
X(MKDIR, mkdir(req->path, req->mode));
X(OPEN, open(req->path, req->flags, req->mode));
X(READ, uv__fs_read(req));
X(READDIR, uv__fs_readdir(req));
X(READLINK, uv__fs_readlink(req));
Expand All @@ -646,6 +648,35 @@ static void uv__fs_work(struct uv__work* w) {
X(UNLINK, unlink(req->path));
X(UTIME, uv__fs_utime(req));
X(WRITE, uv__fs_write(req));
case UV_FS_OPEN:
#ifdef O_CLOEXEC
/* Try O_CLOEXEC before entering locks */
if (!no_cloexec_support) {
r = open(req->path, req->flags | O_CLOEXEC, req->mode);
if (r >= 0)
break;
if (errno != EINVAL)
break;
no_cloexec_support = 1;
}
#endif /* O_CLOEXEC */
if (req->cb != NULL)
uv_rwlock_rdlock(&req->loop->cloexec_lock);
r = open(req->path, req->flags, req->mode);

/*
* In case of failure `uv__cloexec` will leave error in `errno`,
* so it is enough to just set `r` to `-1`.
*/
if (r >= 0 && uv__cloexec(r, 1) != 0) {
r = uv__close(r);
if (r != 0 && r != -EINPROGRESS)
abort();
r = -1;
}
if (req->cb != NULL)
uv_rwlock_rdunlock(&req->loop->cloexec_lock);
break;
default: abort();
}

Expand Down
9 changes: 9 additions & 0 deletions src/unix/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <assert.h>
#include <stdlib.h> /* abort */
#include <string.h> /* strrchr */
#include <fcntl.h> /* O_CLOEXEC, may be */

#if defined(__STRICT_ANSI__)
# define inline __inline
Expand Down Expand Up @@ -111,6 +112,14 @@
# define UV__POLLHUP 8
#endif

#if !defined(O_CLOEXEC) && defined(__FreeBSD__)
/*
* It may be that we are just missing `__POSIX_VISIBLE >= 200809`.
* Try using fixed value const and give up, if it doesn't work
*/
# define O_CLOEXEC 0x00100000
#endif

/* handle flags */
enum {
UV_CLOSING = 0x01, /* uv_close() called but not finished. */
Expand Down
9 changes: 9 additions & 0 deletions src/unix/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ static int uv__loop_init(uv_loop_t* loop, int default_loop) {
for (i = 0; i < ARRAY_SIZE(loop->process_handles); i++)
QUEUE_INIT(loop->process_handles + i);

if (uv_rwlock_init(&loop->cloexec_lock))
abort();

if (uv_mutex_init(&loop->wq_mutex))
abort();

Expand Down Expand Up @@ -151,6 +154,12 @@ static void uv__loop_delete(uv_loop_t* loop) {
uv_mutex_unlock(&loop->wq_mutex);
uv_mutex_destroy(&loop->wq_mutex);

/*
* Note that all thread pool stuff is finished at this point and
* it is safe to just destroy rw lock
*/
uv_rwlock_destroy(&loop->cloexec_lock);

#if 0
assert(QUEUE_EMPTY(&loop->pending_queue));
assert(QUEUE_EMPTY(&loop->watcher_queue));
Expand Down
5 changes: 5 additions & 0 deletions src/unix/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,13 @@ int uv_spawn(uv_loop_t* loop,

uv_signal_start(&loop->child_watcher, uv__chld, SIGCHLD);

/* Acquire write lock to prevent opening new fds in worker threads */
uv_rwlock_wrlock(&loop->cloexec_lock);
pid = fork();

if (pid == -1) {
err = -errno;
uv_rwlock_wrunlock(&loop->cloexec_lock);
uv__close(signal_pipe[0]);
uv__close(signal_pipe[1]);
goto error;
Expand All @@ -436,6 +439,8 @@ int uv_spawn(uv_loop_t* loop,
abort();
}

/* Release lock in parent process */
uv_rwlock_wrunlock(&loop->cloexec_lock);
uv__close(signal_pipe[1]);

process->status = 0;
Expand Down
11 changes: 11 additions & 0 deletions test/run-tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,5 +152,16 @@ static int maybe_run_test(int argc, char **argv) {
return 1;
}

#ifndef _WIN32
if (strcmp(argv[1], "spawn_helper8") == 0) {
int fd;
ASSERT(sizeof(fd) == read(0, &fd, sizeof(fd)));
ASSERT(fd > 2);
ASSERT(-1 == write(fd, "x", 1));

return 1;
}
#endif /* !_WIN32 */

return run_test(argv[1], 0, 1);
}
2 changes: 2 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ TEST_DECLARE (getsockname_udp)
TEST_DECLARE (fail_always)
TEST_DECLARE (pass_always)
TEST_DECLARE (spawn_fails)
TEST_DECLARE (spawn_fs_open)
TEST_DECLARE (spawn_exit_code)
TEST_DECLARE (spawn_stdout)
TEST_DECLARE (spawn_stdin)
Expand Down Expand Up @@ -445,6 +446,7 @@ TASK_LIST_START
TEST_ENTRY (poll_close)

TEST_ENTRY (spawn_fails)
TEST_ENTRY (spawn_fs_open)
TEST_ENTRY (spawn_exit_code)
TEST_ENTRY (spawn_stdout)
TEST_ENTRY (spawn_stdin)
Expand Down
38 changes: 38 additions & 0 deletions test/test-spawn.c
Original file line number Diff line number Diff line change
Expand Up @@ -1049,3 +1049,41 @@ TEST_IMPL(spawn_auto_unref) {
MAKE_VALGRIND_HAPPY();
return 0;
}


#ifndef _WIN32
TEST_IMPL(spawn_fs_open) {
int fd;
uv_fs_t fs_req;
uv_pipe_t in;
uv_write_t write_req;
uv_buf_t buf;
uv_stdio_container_t stdio[1];

fd = uv_fs_open(uv_default_loop(), &fs_req, "/dev/null", O_RDWR, 0, NULL);
ASSERT(fd >= 0);

init_process_options("spawn_helper8", exit_cb);

ASSERT(0 == uv_pipe_init(uv_default_loop(), &in, 0));

options.stdio = stdio;
options.stdio[0].flags = UV_CREATE_PIPE | UV_READABLE_PIPE;
options.stdio[0].data.stream = (uv_stream_t*) &in;
options.stdio_count = 1;

ASSERT(0 == uv_spawn(uv_default_loop(), &process, &options));

buf = uv_buf_init((char*) &fd, sizeof(fd));
ASSERT(0 == uv_write(&write_req, (uv_stream_t*) &in, &buf, 1, write_cb));

ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
ASSERT(0 == uv_fs_close(uv_default_loop(), &fs_req, fd, NULL));

ASSERT(exit_cb_called == 1);
ASSERT(close_cb_called == 2); /* One for `in`, one for process */

MAKE_VALGRIND_HAPPY();
return 0;
}
#endif /* !_WIN32 */

0 comments on commit 6abe1e4

Please sign in to comment.