summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephen Hemminger <shemminger@vyatta.com>2012-07-11 09:29:22 -0700
committerStephen Hemminger <shemminger@vyatta.com>2012-07-11 11:52:47 -0700
commit8ebd82645a8503ec1b8464586e5c19c18c009f06 (patch)
tree955485024bcb20545e1c061f83777186baababb7
parent628e456b5fc2c11cd474546461d1458a17d4ab56 (diff)
downloadvyatta-cfg-8ebd82645a8503ec1b8464586e5c19c18c009f06.tar.gz
vyatta-cfg-8ebd82645a8503ec1b8464586e5c19c18c009f06.zip
Don't leak file descriptors to action
Bug 8204 Make sure we don't handle unnecessary file descriptors to child processes. This is done by marking file descriptors as close on exec, and closing pipe before exec.
-rw-r--r--src/cli_new.c85
-rw-r--r--src/cstore/unionfs/cstore-unionfs.cpp7
2 files changed, 66 insertions, 26 deletions
diff --git a/src/cli_new.c b/src/cli_new.c
index 716c06f..a7cd122 100644
--- a/src/cli_new.c
+++ b/src/cli_new.c
@@ -5,9 +5,19 @@
#include <ctype.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <unistd.h>
#include <fcntl.h>
+
+/* Workaround for features present in current kernel but
+ missing from old includes (glibc) in Debian Squeeze */
+#ifndef O_CLOEXEC
+#define O_CLOEXEC 02000000
+#endif
+#ifndef F_DUPFD_CLOEXEC
+#define F_DUPFD_CLOEXEC 0x406
+#endif
+
#include <sys/wait.h>
-#include <unistd.h>
#include <dirent.h>
#include <limits.h>
#include <stdarg.h>
@@ -2043,9 +2053,9 @@ static int set_reference_environment(const char* var_reference,
/*** output ***/
+#define OUT_FD_MIN 16
int out_fd = -1;
FILE *out_stream = NULL;
-
int err_fd = -1;
FILE *err_stream = NULL;
int new_out_fd = -1;
@@ -2054,35 +2064,55 @@ int new_err_fd = -1;
int
initialize_output(const char *op)
{
- if ((out_fd = dup(STDOUT_FILENO)) == -1) {
- return -1;
- }
- if ((out_stream = fdopen(out_fd, "a")) == NULL) {
- return -1;
- }
- if ((err_fd = dup(STDOUT_FILENO)) == -1) {
- return -1;
- }
- if ((err_stream = fdopen(err_fd, "a")) == NULL) {
- return -1;
+ out_fd = fcntl(STDOUT_FILENO, F_DUPFD_CLOEXEC, OUT_FD_MIN);
+ if (out_fd < 0)
+ goto err1;
+
+ out_stream = fdopen(out_fd, "w");
+ if (out_stream == NULL) {
+ close(out_fd);
+ goto err1;
}
- new_out_fd = open(LOGFILE_STDOUT, O_WRONLY | O_CREAT, 0660);
- new_err_fd = open(LOGFILE_STDERR, O_WRONLY | O_CREAT, 0660);
- if (new_out_fd == -1 || new_err_fd == -1) {
- return -1;
- }
- if ((lseek(new_out_fd, 0, SEEK_END) == ((off_t) -1))
- || (lseek(new_err_fd, 0, SEEK_END) == ((off_t) -1))) {
- return -1;
+ err_fd = fcntl(STDERR_FILENO, F_DUPFD_CLOEXEC, OUT_FD_MIN);
+ if (err_fd < 0)
+ goto err2;
+
+ err_stream = fdopen(err_fd, "w");
+ if (err_stream == NULL) {
+ close(err_fd);
+ goto err2;
}
+
+ new_out_fd = open(LOGFILE_STDOUT,
+ O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC, 0660);
+ if (new_out_fd < 0)
+ goto err3;
+
+ new_err_fd = open(LOGFILE_STDERR,
+ O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC, 0660);
+ if (new_err_fd < 0)
+ goto err4;
+
if ((dup2(new_out_fd, STDOUT_FILENO) == -1)
- || (dup2(new_err_fd, STDERR_FILENO) == -1)) {
- return -1;
- }
+ || (dup2(new_err_fd, STDERR_FILENO) == -1))
+ goto err5;
cli_operation_name = op;
return 0;
+
+err5:
+ close(new_err_fd); new_err_fd = -1;
+err4:
+ close(new_out_fd); new_out_fd = -1;
+err3:
+ fclose(err_stream);
+ err_stream = NULL; err_fd = -1;
+err2:
+ fclose(out_stream);
+ out_stream = NULL; out_fd = -1;
+err1:
+ return -1;
}
int
@@ -2139,11 +2169,14 @@ system_out(char *cmd, const char *prepend_msg, boolean eloc)
int waited = 0;
int prepend = 1;
+ close(pfd[1]);
+
if (cpid == -1) {
+ close(pfd[0]);
+ fprintf(stderr, "fork failed\n");
return -1;
}
- close(pfd[1]);
while (1) {
int sret;
fd_set readfds;
@@ -2278,6 +2311,7 @@ system_out(char *cmd, const char *prepend_msg, boolean eloc)
/* XXX XXX XXX END emulating original "error" location handling */
if (out_stream != NULL) {
if (fwrite(out, count, 1, out_stream) != 1) {
+ close(pfd[0]);
return -1;
}
fflush(out_stream);
@@ -2311,6 +2345,7 @@ system_out(char *cmd, const char *prepend_msg, boolean eloc)
|| (ret = dup2(pfd[1], STDERR_FILENO)) < 0) {
exit(1);
}
+ close(pfd[1]);
char *eargs[] = { "sh", "-c", cmd, NULL };
execv("/bin/sh", eargs);
return -1; /* should not get here */
diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp
index f8e54d7..7cfe23d 100644
--- a/src/cstore/unionfs/cstore-unionfs.cpp
+++ b/src/cstore/unionfs/cstore-unionfs.cpp
@@ -20,7 +20,9 @@
#include <fstream>
#include <sstream>
+#include <unistd.h>
#include <errno.h>
+#include <fcntl.h>
#include <sys/mount.h>
#include <cli_cstore.h>
@@ -701,7 +703,10 @@ UnionfsCstore::commitConfig(commit::PrioNode& node)
bool
UnionfsCstore::getCommitLock()
{
- int fd = creat(C_COMMIT_LOCK_FILE.c_str(), 0777);
+ int fd;
+
+ fd = open(C_COMMIT_LOCK_FILE.c_str(),
+ O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666);
if (fd < 0) {
// should not happen since all commit processes should have write access
output_internal("getCommitLock() failed to open lock file\n");