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

unix: sanity-check fds before closing #943

Closed
wants to merge 1 commit into from
Closed

unix: sanity-check fds before closing #943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/unix/aix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ int uv_exepath(char* buffer, size_t* size) {
return fd;

res = read(fd, &ps, sizeof(ps));
close(fd);
uv__close(fd);
if (res < 0)
return res;

Expand Down Expand Up @@ -179,7 +179,7 @@ int uv_resident_set_memory(size_t* rss) {
*rss = (size_t)psinfo.pr_rssize * 1024;
err = 0;
}
close(fd);
uv__close(fd);

return err;
}
Expand Down Expand Up @@ -291,14 +291,14 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
}

if (ioctl(sockfd, SIOCGSIZIFCONF, &size) == -1) {
close(sockfd);
uv__close(sockfd);
return -ENOSYS;
}

ifc.ifc_req = (struct ifreq*)malloc(size);
ifc.ifc_len = size;
if (ioctl(sockfd, SIOCGIFCONF, &ifc) == -1) {
close(sockfd);
uv__close(sockfd);
return -ENOSYS;
}

Expand All @@ -317,7 +317,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,

memcpy(flg.ifr_name, p->ifr_name, sizeof(flg.ifr_name));
if (ioctl(sockfd, SIOCGIFFLAGS, &flg) == -1) {
close(sockfd);
uv__close(sockfd);
return -ENOSYS;
}

Expand All @@ -331,7 +331,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,
*addresses = (uv_interface_address_t*)
malloc(*count * sizeof(uv_interface_address_t));
if (!(*addresses)) {
close(sockfd);
uv__close(sockfd);
return -ENOMEM;
}
address = *addresses;
Expand All @@ -348,7 +348,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,

memcpy(flg.ifr_name, p->ifr_name, sizeof(flg.ifr_name));
if (ioctl(sockfd, SIOCGIFFLAGS, &flg) == -1) {
close(sockfd);
uv__close(sockfd);
return -ENOSYS;
}

Expand All @@ -374,7 +374,7 @@ int uv_interface_addresses(uv_interface_address_t** addresses,

#undef ADDR_SIZE

close(sockfd);
uv__close(sockfd);
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions src/unix/async.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,11 @@ void uv__async_stop(uv_loop_t* loop, struct uv__async* wa) {
return;

uv__io_stop(loop, &wa->io_watcher, UV__POLLIN);
close(wa->io_watcher.fd);
uv__close(wa->io_watcher.fd);
wa->io_watcher.fd = -1;

if (wa->wfd != -1) {
close(wa->wfd);
uv__close(wa->wfd);
wa->wfd = -1;
}
}
Expand Down
26 changes: 23 additions & 3 deletions src/unix/core.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ int uv__socket(int domain, int type, int protocol) {
err = uv__cloexec(sockfd, 1);

if (err) {
close(sockfd);
uv__close(sockfd);
return err;
}

Expand Down Expand Up @@ -397,7 +397,7 @@ int uv__accept(int sockfd) {
err = uv__nonblock(peerfd, 1);

if (err) {
close(peerfd);
uv__close(peerfd);
return err;
}

Expand All @@ -406,6 +406,26 @@ int uv__accept(int sockfd) {
}


int uv__close(int fd) {
int saved_errno;
int rc;

assert(fd > -1); /* Catch uninitialized io_watcher.fd bugs. */
assert(fd > STDERR_FILENO); /* Catch stdio close bugs. */

saved_errno = errno;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this we didn't save the errno (only in some cases) so is this now a Good Thing (TM) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early versions of uv-unix saved errno in a lot of places but I kind of scaled back on that. For uv__close(), it's mostly a convenience thing - it often gets called from error paths where the original errno needs to be preserved. Dealing with that in uv__close() makes that easier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks for explaining!

rc = close(fd);
if (rc == -1) {
rc = -errno;
if (rc == -EINTR)
rc = -EINPROGRESS; /* For platform/libc consistency. */
errno = saved_errno;
}

return rc;
}


#if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__)

int uv__nonblock(int fd, int set) {
Expand Down Expand Up @@ -514,7 +534,7 @@ int uv__dup(int fd) {

err = uv__cloexec(fd, 1);
if (err) {
close(fd);
uv__close(fd);
return err;
}

Expand Down
1 change: 1 addition & 0 deletions src/unix/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ enum {

/* core */
int uv__nonblock(int fd, int set);
int uv__close(int fd);
int uv__cloexec(int fd, int set);
int uv__socket(int domain, int type, int protocol);
int uv__dup(int fd);
Expand Down
2 changes: 1 addition & 1 deletion src/unix/kqueue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,6 @@ void uv__fs_event_close(uv_fs_event_t* handle) {
free(handle->filename);
handle->filename = NULL;

close(handle->event_watcher.fd);
uv__close(handle->event_watcher.fd);
handle->event_watcher.fd = -1;
}
4 changes: 2 additions & 2 deletions src/unix/linux-core.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ int uv__platform_loop_init(uv_loop_t* loop, int default_loop) {
void uv__platform_loop_delete(uv_loop_t* loop) {
if (loop->inotify_fd == -1) return;
uv__io_stop(loop, &loop->inotify_read_watcher, UV__POLLIN);
close(loop->inotify_fd);
uv__close(loop->inotify_fd);
loop->inotify_fd = -1;
}

Expand Down Expand Up @@ -309,7 +309,7 @@ int uv_resident_set_memory(size_t* rss) {
n = read(fd, buf, sizeof(buf) - 1);
while (n == -1 && errno == EINTR);

SAVE_ERRNO(close(fd));
uv__close(fd);
if (n == -1)
return -errno;
buf[n] = '\0';
Expand Down
2 changes: 1 addition & 1 deletion src/unix/linux-inotify.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static int new_inotify_fd(void) {
err = uv__nonblock(fd, 1);

if (err) {
close(fd);
uv__close(fd);
return err;
}

Expand Down
4 changes: 2 additions & 2 deletions src/unix/loop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ static void uv__loop_delete(uv_loop_t* loop) {
uv__async_stop(loop, &loop->async_watcher);

if (loop->emfile_fd != -1) {
close(loop->emfile_fd);
uv__close(loop->emfile_fd);
loop->emfile_fd = -1;
}

if (loop->backend_fd != -1) {
close(loop->backend_fd);
uv__close(loop->backend_fd);
loop->backend_fd = -1;
}

Expand Down
4 changes: 2 additions & 2 deletions src/unix/pipe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) {

out:
if (bound) {
/* unlink() before close() to avoid races. */
/* unlink() before uv__close() to avoid races. */
assert(pipe_fname != NULL);
unlink(pipe_fname);
}
close(sockfd);
uv__close(sockfd);
free((void*)pipe_fname);
return err;
}
Expand Down
22 changes: 12 additions & 10 deletions src/unix/process.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ static int uv__process_open_stream(uv_stdio_container_t* container,
if (!(container->flags & UV_CREATE_PIPE) || pipefds[0] < 0)
return 0;

if (close(pipefds[1]))
if (uv__close(pipefds[1]))
if (errno != EINTR && errno != EINPROGRESS)
abort();

Expand Down Expand Up @@ -285,8 +285,10 @@ static void uv__process_child_init(const uv_process_options_t* options,
close_fd = pipes[fd][0];
use_fd = pipes[fd][1];

if (use_fd >= 0)
close(close_fd);
if (use_fd >= 0) {
if (close_fd != -1)
uv__close(close_fd);
}
else if (fd >= 3)
continue;
else {
Expand All @@ -306,7 +308,7 @@ static void uv__process_child_init(const uv_process_options_t* options,
uv__cloexec(use_fd, 0);
else {
dup2(use_fd, fd);
close(use_fd);
uv__close(use_fd);
}

if (fd <= 2)
Expand Down Expand Up @@ -414,8 +416,8 @@ int uv_spawn(uv_loop_t* loop,

if (pid == -1) {
err = -errno;
close(signal_pipe[0]);
close(signal_pipe[1]);
uv__close(signal_pipe[0]);
uv__close(signal_pipe[1]);
goto error;
}

Expand All @@ -424,7 +426,7 @@ int uv_spawn(uv_loop_t* loop,
abort();
}

close(signal_pipe[1]);
uv__close(signal_pipe[1]);

process->errorno = 0;
do
Expand All @@ -440,7 +442,7 @@ int uv_spawn(uv_loop_t* loop,
else
abort();

close(signal_pipe[0]);
uv__close(signal_pipe[0]);

for (i = 0; i < options->stdio_count; i++) {
err = uv__process_open_stream(options->stdio + i, pipes[i], i == 0);
Expand All @@ -465,8 +467,8 @@ int uv_spawn(uv_loop_t* loop,

error:
for (i = 0; i < stdio_count; i++) {
close(pipes[i][0]);
close(pipes[i][1]);
uv__close(pipes[i][0]);
uv__close(pipes[i][1]);
}
free(pipes);

Expand Down
4 changes: 2 additions & 2 deletions src/unix/signal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@ void uv__signal_loop_cleanup(uv_loop_t* loop) {
}

if (loop->signal_pipefd[0] != -1) {
close(loop->signal_pipefd[0]);
uv__close(loop->signal_pipefd[0]);
loop->signal_pipefd[0] = -1;
}

if (loop->signal_pipefd[1] != -1) {
close(loop->signal_pipefd[1]);
uv__close(loop->signal_pipefd[1]);
loop->signal_pipefd[1] = -1;
}
}
Expand Down
32 changes: 18 additions & 14 deletions src/unix/stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ static int uv__open_cloexec(const char* path, int flags) {

err = uv__cloexec(fd, 1);
if (err) {
close(fd);
uv__close(fd);
return err;
}

Expand Down Expand Up @@ -309,7 +309,7 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) {
timeout.tv_nsec = 1;

ret = kevent(kq, filter, 1, events, 1, &timeout);
SAVE_ERRNO(close(kq));
uv__close(kq);

if (ret == -1)
return -errno;
Expand Down Expand Up @@ -357,8 +357,8 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) {
return 0;

fatal4:
close(s->fake_fd);
close(s->int_fd);
uv__close(s->fake_fd);
uv__close(s->int_fd);
s->fake_fd = -1;
s->int_fd = -1;
fatal3:
Expand Down Expand Up @@ -467,13 +467,13 @@ static int uv__emfile_trick(uv_loop_t* loop, int accept_fd) {
if (loop->emfile_fd == -1)
return -EMFILE;

close(loop->emfile_fd);
uv__close(loop->emfile_fd);

for (;;) {
fd = uv__accept(accept_fd);

if (fd != -1) {
close(fd);
uv__close(fd);
continue;
}

Expand Down Expand Up @@ -572,7 +572,7 @@ int uv_accept(uv_stream_t* server, uv_stream_t* client) {
UV_STREAM_READABLE | UV_STREAM_WRITABLE);
if (err) {
/* TODO handle error */
close(server->accepted_fd);
uv__close(server->accepted_fd);
server->accepted_fd = -1;
return err;
}
Expand All @@ -581,7 +581,7 @@ int uv_accept(uv_stream_t* server, uv_stream_t* client) {
case UV_UDP:
err = uv_udp_open((uv_udp_t*) client, server->accepted_fd);
if (err) {
close(server->accepted_fd);
uv__close(server->accepted_fd);
server->accepted_fd = -1;
return err;
}
Expand Down Expand Up @@ -1424,8 +1424,8 @@ void uv__stream_close(uv_stream_t* handle) {
uv_thread_join(&s->thread);
uv_sem_destroy(&s->close_sem);
uv_sem_destroy(&s->async_sem);
close(s->fake_fd);
close(s->int_fd);
uv__close(s->fake_fd);
uv__close(s->int_fd);
uv_close((uv_handle_t*) &s->async, uv__stream_osx_cb_close);

handle->select = NULL;
Expand All @@ -1436,11 +1436,15 @@ void uv__stream_close(uv_stream_t* handle) {
uv_read_stop(handle);
uv__handle_stop(handle);

close(handle->io_watcher.fd);
handle->io_watcher.fd = -1;
if (handle->io_watcher.fd != -1) {
/* Don't close stdio file descriptors. Nothing good comes from it. */
if (handle->io_watcher.fd > STDERR_FILENO)
uv__close(handle->io_watcher.fd);
handle->io_watcher.fd = -1;
}

if (handle->accepted_fd >= 0) {
close(handle->accepted_fd);
if (handle->accepted_fd != -1) {
uv__close(handle->accepted_fd);
handle->accepted_fd = -1;
}

Expand Down
Loading