From ebec9176c8bb039e4d3db89871322e585d88f12b Mon Sep 17 00:00:00 2001 From: Andrey Mazo Date: Thu, 28 Nov 2013 16:45:47 +0400 Subject: [PATCH] run_buffer(): unblock all signals for spawned scripts. Currently, all scripts, specified as "lxc.network.script.up", inherit lxc-execute's signal mask. This, for example, includes blocked SIGALRM signal which, in turn, makes alarm(2), sleep(3) and setitimer(2) functions silently unusable in all programs, invoked in turn by the "lxc.network.script.up". To fix this, run_buffer() should restore default signal mask prior to executing "lxc.network.script.up". A naive implementation would temprorary unblock all signals just before calling popen() and block them back immediately after it. But that would result in an immediate delivery of all pending signals just after their unblocking. Thus, we should restore default signal mask exactly in child (after fork()) just before calling exec(). To achieve this, a home-brewed popen() alternative is needed. The added lxc_popen() and lxc_pclose() are mostly taken from glibc with several simplifications (as we currently need only "re" mode). The implementation uses Linux-specific pipe2() system-call, which is only available since Linux 2.6.27 and supported by glibc since version 2.9 (according to pipe(2) man-page), but this shouldn't be a problem as lxc requires a fairly recent kernel too. lxc_popen()/lxc_pclose() are meant to be direct replacements for their stdio counterparts, so they perform no process_lock() locking themselves. (as fopen_cloexec() does) All existing users of popen()/pclose() are converted to the new lxc_popen()/lxc_pclose(). (mazo: don't clear close-on-exec flag for parent's end; place the new functions in utils.c; convert bdev.c to use the new functions; coding style fixes; comments fixes; commit message tweaks) Signed-off-by: Ivan Bolsunov Signed-off-by: Andrey Mazo --- src/lxc/bdev.c | 16 +++--- src/lxc/conf.c | 10 ++-- src/lxc/utils.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ src/lxc/utils.h | 28 ++++++++++ 4 files changed, 174 insertions(+), 13 deletions(-) diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c index 249815e72..f7a6031ed 100644 --- a/src/lxc/bdev.c +++ b/src/lxc/bdev.c @@ -501,24 +501,24 @@ struct bdev_ops dir_ops = { static int zfs_list_entry(const char *path, char *output, size_t inlen) { - FILE *f; + struct lxc_popen_FILE *f; int found=0; process_lock(); - f = popen("zfs list 2> /dev/null", "r"); + f = lxc_popen("zfs list 2> /dev/null"); process_unlock(); if (f == NULL) { SYSERROR("popen failed"); return 0; } - while (fgets(output, inlen, f)) { + while (fgets(output, inlen, f->f)) { if (strstr(output, path)) { found = 1; break; } } process_lock(); - (void) pclose(f); + (void) lxc_pclose(f); process_unlock(); return found; @@ -813,7 +813,7 @@ static int lvm_umount(struct bdev *bdev) } static int lvm_compare_lv_attr(const char *path, int pos, const char expected) { - FILE *f; + struct lxc_popen_FILE *f; int ret, len, status, start=0; char *cmd, output[12]; const char *lvscmd = "lvs --unbuffered --noheadings -o lv_attr %s 2>/dev/null"; @@ -826,7 +826,7 @@ static int lvm_compare_lv_attr(const char *path, int pos, const char expected) { return -1; process_lock(); - f = popen(cmd, "r"); + f = lxc_popen(cmd); process_unlock(); if (f == NULL) { @@ -834,10 +834,10 @@ static int lvm_compare_lv_attr(const char *path, int pos, const char expected) { return -1; } - ret = fgets(output, 12, f) == NULL; + ret = fgets(output, 12, f->f) == NULL; process_lock(); - status = pclose(f); + status = lxc_pclose(f); process_unlock(); if (ret || WEXITSTATUS(status)) diff --git a/src/lxc/conf.c b/src/lxc/conf.c index daf491f49..fa776188a 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -350,12 +350,12 @@ static char *mkifname(char *template) static int run_buffer(char *buffer) { - FILE *f; + struct lxc_popen_FILE *f; char *output; int ret; process_lock(); - f = popen(buffer, "r"); + f = lxc_popen(buffer); process_unlock(); if (!f) { SYSERROR("popen failed"); @@ -366,18 +366,18 @@ static int run_buffer(char *buffer) if (!output) { ERROR("failed to allocate memory for script output"); process_lock(); - pclose(f); + lxc_pclose(f); process_unlock(); return -1; } - while(fgets(output, LXC_LOG_BUFFER_SIZE, f)) + while(fgets(output, LXC_LOG_BUFFER_SIZE, f->f)) DEBUG("script output: %s", output); free(output); process_lock(); - ret = pclose(f); + ret = lxc_pclose(f); process_unlock(); if (ret == -1) { SYSERROR("Script exited on error"); diff --git a/src/lxc/utils.c b/src/lxc/utils.c index 97c252d51..80addf55f 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -616,6 +616,139 @@ FILE *fopen_cloexec(const char *path, const char *mode) return ret; } +/* must be called with process_lock() held */ +extern struct lxc_popen_FILE *lxc_popen(const char *command) +{ + struct lxc_popen_FILE *fp = NULL; + int parent_end = -1, child_end = -1; + int pipe_fds[2]; + pid_t child_pid; + + int r = pipe2(pipe_fds, O_CLOEXEC); + + if (r < 0) { + ERROR("pipe2 failure"); + return NULL; + } + + parent_end = pipe_fds[0]; + child_end = pipe_fds[1]; + + child_pid = fork(); + + if (child_pid == 0) { + /* child */ + int child_std_end = STDOUT_FILENO; + + if (child_end != child_std_end) { + /* dup2() doesn't dup close-on-exec flag */ + dup2(child_end, child_std_end); + + /* it's safe not to close child_end here + * as it's marked close-on-exec anyway + */ + } else { + /* + * The descriptor is already the one we will use. + * But it must not be marked close-on-exec. + * Undo the effects. + */ + fcntl(child_end, F_SETFD, 0); + } + + /* + * Unblock signals. + * This is the main/only reason + * why we do our lousy popen() emulation. + */ + { + sigset_t mask; + sigfillset(&mask); + sigprocmask(SIG_UNBLOCK, &mask, NULL); + } + + execl("/bin/sh", "sh", "-c", command, (char *) NULL); + exit(127); + } + + /* parent */ + + close(child_end); + child_end = -1; + + if (child_pid < 0) { + ERROR("fork failure"); + goto error; + } + + fp = calloc(1, sizeof(*fp)); + if (!fp) { + ERROR("failed to allocate memory"); + goto error; + } + + fp->f = fdopen(parent_end, "r"); + if (!fp->f) { + ERROR("fdopen failure"); + goto error; + } + + fp->child_pid = child_pid; + + return fp; + +error: + + if (fp) { + if (fp->f) { + fclose(fp->f); + parent_end = -1; /* so we do not close it second time */ + } + + free(fp); + } + + if (child_end != -1) + close(child_end); + if (parent_end != -1) + close(parent_end); + + return NULL; +} + +/* must be called with process_lock() held */ +extern int lxc_pclose(struct lxc_popen_FILE *fp) +{ + FILE *f = NULL; + pid_t child_pid = 0; + int wstatus = 0; + pid_t wait_pid; + + if (fp) { + f = fp->f; + child_pid = fp->child_pid; + /* free memory (we still need to close file stream) */ + free(fp); + fp = NULL; + } + + if (!f || fclose(f)) { + ERROR("fclose failure"); + return -1; + } + + do { + wait_pid = waitpid(child_pid, &wstatus, 0); + } while (wait_pid == -1 && errno == EINTR); + + if (wait_pid == -1) { + ERROR("waitpid failure"); + return -1; + } + + return wstatus; +} + char *lxc_string_replace(const char *needle, const char *replacement, const char *haystack) { ssize_t len = -1, saved_len = -1; diff --git a/src/lxc/utils.h b/src/lxc/utils.h index 714e74c6f..945f1de31 100644 --- a/src/lxc/utils.h +++ b/src/lxc/utils.h @@ -158,6 +158,34 @@ static inline int signalfd(int fd, const sigset_t *mask, int flags) FILE *fopen_cloexec(const char *path, const char *mode); +/* Struct to carry child pid from lxc_popen() to lxc_pclose(). + * Not an opaque struct to allow direct access to the underlying FILE * + * (i.e., struct lxc_popen_FILE *file; fgets(buf, sizeof(buf), file->f)) + * without additional wrappers. + */ +struct lxc_popen_FILE { + FILE *f; + pid_t child_pid; +}; + +/* popen(command, "re") replacement that restores default signal mask + * via sigprocmask(2) (unblocks all signals) after fork(2) but prior to calling exec(3). + * In short, popen(command, "re") does pipe() + fork() + exec() + * while lxc_popen(command) does pipe() + fork() + sigprocmask() + exec(). + * Must be called with process_lock() held. + * Returns pointer to struct lxc_popen_FILE, that should be freed with lxc_pclose(). + * On error returns NULL. + */ +extern struct lxc_popen_FILE *lxc_popen(const char *command); + +/* pclose() replacement to be used on struct lxc_popen_FILE *, + * returned by lxc_popen(). + * Waits for associated process to terminate, returns its exit status and + * frees resources, pointed to by struct lxc_popen_FILE *. + * Must be called with process_lock() held. + */ +extern int lxc_pclose(struct lxc_popen_FILE *fp); + /** * BUILD_BUG_ON - break compile if a condition is true. * @condition: the condition which the compiler should know is false.