From 09cbe9d64088b825a520fcba279cbca3df5c4035 Mon Sep 17 00:00:00 2001 From: nobu Date: Sat, 12 Sep 2015 05:30:05 +0000 Subject: [PATCH] process.c: do not inherit saved fds * process.c (save_redirect_fd): make saved FDs close-on-exec not to be inherited. * process.c (run_exec_dup2): restore close-on-exec flags too. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51832 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 7 ++++- process.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index f32bd51d0a..5a47bb62c9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,9 @@ -Sat Sep 12 14:23:20 2015 Nobuyoshi Nakada +Sat Sep 12 14:30:03 2015 Nobuyoshi Nakada + + * process.c (save_redirect_fd): make saved FDs close-on-exec not + to be inherited. + + * process.c (run_exec_dup2): restore close-on-exec flags too. * win32/win32.c (fcntl): implement F_GETFD, F_SETFD, and F_DUPFD_CLOEXEC. diff --git a/process.c b/process.c index c812e91a9b..3a32848179 100644 --- a/process.c +++ b/process.c @@ -349,6 +349,24 @@ redirect_dup2(int oldfd, int newfd) return ret; } +static int +redirect_cloexec_dup(int oldfd) +{ + int ret; + ret = rb_cloexec_dup(oldfd); + ttyprintf("cloexec_dup(%d) => %d\n", oldfd, ret); + return ret; +} + +static int +redirect_cloexec_dup2(int oldfd, int newfd) +{ + int ret; + ret = rb_cloexec_dup2(oldfd, newfd); + ttyprintf("cloexec_dup2(%d, %d) => %d\n", oldfd, newfd, ret); + return ret; +} + static int redirect_close(int fd) { @@ -379,6 +397,8 @@ parent_redirect_close(int fd) #else #define redirect_dup(oldfd) dup(oldfd) #define redirect_dup2(oldfd, newfd) dup2((oldfd), (newfd)) +#define redirect_cloexec_dup(oldfd) rb_cloexec_dup(oldfd) +#define redirect_cloexec_dup2(oldfd, newfd) rb_cloexec_dup2((oldfd), (newfd)) #define redirect_close(fd) close_unless_reserved(fd) #define parent_redirect_open(pathname, flags, perm) rb_cloexec_open((pathname), (flags), (perm)) #define parent_redirect_close(fd) close_unless_reserved(fd) @@ -2598,12 +2618,16 @@ rb_f_exec(int argc, const VALUE *argv) #define ERRMSG1(str, a) do { if (errmsg && 0 < errmsg_buflen) snprintf(errmsg, errmsg_buflen, (str), (a)); } while (0) #define ERRMSG2(str, a, b) do { if (errmsg && 0 < errmsg_buflen) snprintf(errmsg, errmsg_buflen, (str), (a), (b)); } while (0) +static int fd_get_cloexec(int fd, char *errmsg, size_t errmsg_buflen); +static int fd_set_cloexec(int fd, char *errmsg, size_t errmsg_buflen); +static int fd_clear_cloexec(int fd, char *errmsg, size_t errmsg_buflen); + static int save_redirect_fd(int fd, struct rb_execarg *sargp, char *errmsg, size_t errmsg_buflen) { if (sargp) { - VALUE newary; - int save_fd = redirect_dup(fd); + VALUE newary, redirection; + int save_fd = redirect_cloexec_dup(fd), cloexec; if (save_fd == -1) { if (errno == EBADF) return 0; @@ -2616,8 +2640,10 @@ save_redirect_fd(int fd, struct rb_execarg *sargp, char *errmsg, size_t errmsg_b newary = hide_obj(rb_ary_new()); sargp->fd_dup2 = newary; } - rb_ary_push(newary, - hide_obj(rb_assoc_new(INT2FIX(fd), INT2FIX(save_fd)))); + cloexec = fd_get_cloexec(fd, errmsg, errmsg_buflen); + redirection = hide_obj(rb_assoc_new(INT2FIX(fd), INT2FIX(save_fd))); + if (cloexec) rb_ary_push(redirection, Qtrue); + rb_ary_push(newary, redirection); newary = sargp->fd_close; if (newary == Qfalse) { @@ -2647,6 +2673,7 @@ struct run_exec_dup2_fd_pair { int newfd; long older_index; long num_newer; + int cloexec; }; static long @@ -2655,6 +2682,45 @@ run_exec_dup2_tmpbuf_size(long n) return sizeof(struct run_exec_dup2_fd_pair) * n; } +/* This function should be async-signal-safe. Actually it is. */ +static int +fd_get_cloexec(int fd, char *errmsg, size_t errmsg_buflen) +{ +#ifdef F_GETFD + int ret = 0; + ret = fcntl(fd, F_GETFD); /* async-signal-safe */ + if (ret == -1) { + ERRMSG("fcntl(F_GETFD)"); + return -1; + } + if (ret & FD_CLOEXEC) return 1; +#endif + return 0; +} + +/* This function should be async-signal-safe. Actually it is. */ +static int +fd_set_cloexec(int fd, char *errmsg, size_t errmsg_buflen) +{ +#ifdef F_GETFD + int ret = 0; + ret = fcntl(fd, F_GETFD); /* async-signal-safe */ + if (ret == -1) { + ERRMSG("fcntl(F_GETFD)"); + return -1; + } + if (!(ret & FD_CLOEXEC)) { + ret |= FD_CLOEXEC; + ret = fcntl(fd, F_SETFD, ret); /* async-signal-safe */ + if (ret == -1) { + ERRMSG("fcntl(F_SETFD)"); + return -1; + } + } +#endif + return 0; +} + /* This function should be async-signal-safe. Actually it is. */ static int fd_clear_cloexec(int fd, char *errmsg, size_t errmsg_buflen) @@ -2695,6 +2761,7 @@ run_exec_dup2(VALUE ary, VALUE tmpbuf, struct rb_execarg *sargp, char *errmsg, s VALUE elt = RARRAY_AREF(ary, i); pairs[i].oldfd = FIX2INT(RARRAY_AREF(elt, 1)); pairs[i].newfd = FIX2INT(RARRAY_AREF(elt, 0)); /* unique */ + pairs[i].cloexec = RARRAY_LEN(elt) > 2 && RTEST(RARRAY_AREF(elt, 2)); pairs[i].older_index = -1; } @@ -2733,6 +2800,10 @@ run_exec_dup2(VALUE ary, VALUE tmpbuf, struct rb_execarg *sargp, char *errmsg, s ERRMSG("dup2"); goto fail; } + if (pairs[j].cloexec && + fd_set_cloexec(pairs[j].newfd, errmsg, errmsg_buflen)) { + goto fail; + } rb_update_max_fd(pairs[j].newfd); /* async-signal-safe but don't need to call it in a child process. */ pairs[j].oldfd = -1; j = pairs[j].older_index;