From 9b7dfa7522ee290d7545dc38ada06ef721e6b545 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Fri, 4 Apr 2014 15:30:12 -0600 Subject: [PATCH] Remove calls to log_fatal() in I/O log functions and just pass an error back to the caller. --- plugins/sudoers/defaults.h | 2 +- plugins/sudoers/iolog.c | 119 ++++++++++++------ plugins/sudoers/iolog_path.c | 6 +- .../regress/iolog_path/check_iolog_path.c | 4 +- plugins/sudoers/sudoers.c | 8 +- plugins/sudoers/sudoers.h | 4 +- plugins/sudoers/testsudoers.c | 4 +- 7 files changed, 95 insertions(+), 52 deletions(-) diff --git a/plugins/sudoers/defaults.h b/plugins/sudoers/defaults.h index ed652c227..91535e590 100644 --- a/plugins/sudoers/defaults.h +++ b/plugins/sudoers/defaults.h @@ -51,7 +51,7 @@ struct sudo_defs_types { int type; char *desc; struct def_values *values; - int (*callback)(const char *); + bool (*callback)(const char *); union { int flag; int ival; diff --git a/plugins/sudoers/iolog.c b/plugins/sudoers/iolog.c index 488d23484..fa36cdee8 100644 --- a/plugins/sudoers/iolog.c +++ b/plugins/sudoers/iolog.c @@ -85,55 +85,70 @@ extern __dso_public struct io_plugin sudoers_io; * Create path and any parent directories as needed. * If is_temp is set, use mkdtemp() for the final directory. */ -static void +static bool io_mkdirs(char *path, mode_t mode, bool is_temp) { struct stat sb; gid_t parent_gid = 0; char *slash = path; + bool ok = true; debug_decl(io_mkdirs, SUDO_DEBUG_UTIL) /* Fast path: not a temporary and already exists. */ if (!is_temp && stat(path, &sb) == 0) { if (!S_ISDIR(sb.st_mode)) { - log_fatal(0, N_("%s exists but is not a directory (0%o)"), + log_warning(0, N_("%s exists but is not a directory (0%o)"), path, (unsigned int) sb.st_mode); + ok = false; } - debug_return; + debug_return_bool(ok); } while ((slash = strchr(slash + 1, '/')) != NULL) { *slash = '\0'; if (stat(path, &sb) != 0) { - if (mkdir(path, mode) != 0) - log_fatal(USE_ERRNO, N_("unable to mkdir %s"), path); + if (mkdir(path, mode) != 0) { + log_warning(USE_ERRNO, N_("unable to mkdir %s"), path); + ok = false; + break; + } ignore_result(chown(path, (uid_t)-1, parent_gid)); } else if (!S_ISDIR(sb.st_mode)) { - log_fatal(0, N_("%s exists but is not a directory (0%o)"), + log_warning(0, N_("%s exists but is not a directory (0%o)"), path, (unsigned int) sb.st_mode); + ok = false; + break; } else { /* Inherit gid of parent dir for ownership. */ parent_gid = sb.st_gid; } *slash = '/'; } - /* Create final path component. */ - if (is_temp) { - if (mkdtemp(path) == NULL) - log_fatal(USE_ERRNO, N_("unable to mkdir %s"), path); - ignore_result(chown(path, (uid_t)-1, parent_gid)); - } else { - if (mkdir(path, mode) != 0 && errno != EEXIST) - log_fatal(USE_ERRNO, N_("unable to mkdir %s"), path); - ignore_result(chown(path, (uid_t)-1, parent_gid)); + if (ok) { + /* Create final path component. */ + if (is_temp) { + if (mkdtemp(path) == NULL) { + log_warning(USE_ERRNO, N_("unable to mkdir %s"), path); + ok = false; + } else { + ignore_result(chown(path, (uid_t)-1, parent_gid)); + } + } else { + if (mkdir(path, mode) != 0 && errno != EEXIST) { + log_warning(USE_ERRNO, N_("unable to mkdir %s"), path); + ok = false; + } else { + ignore_result(chown(path, (uid_t)-1, parent_gid)); + } + } } - debug_return; + debug_return_bool(ok); } /* * Set max session ID (aka sequence number) */ -int +bool io_set_max_sessid(const char *maxval) { const char *errstr; @@ -159,7 +174,7 @@ io_set_max_sessid(const char *maxval) * number, and update the on-disk copy. * Uses file locking to avoid sequence number collisions. */ -void +bool io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]) { struct stat sb; @@ -175,7 +190,8 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]) /* * Create I/O log directory if it doesn't already exist. */ - io_mkdirs(iolog_dir, S_IRWXU, false); + if (!io_mkdirs(iolog_dir, S_IRWXU, false)) + debug_return_bool(false); /* * Open sequence file @@ -183,11 +199,14 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]) len = snprintf(pathbuf, sizeof(pathbuf), "%s/seq", iolog_dir); if (len <= 0 || (size_t)len >= sizeof(pathbuf)) { errno = ENAMETOOLONG; - log_fatal(USE_ERRNO, "%s/seq", pathbuf); + log_warning(USE_ERRNO, "%s/seq", pathbuf); + debug_return_bool(false); } fd = open(pathbuf, O_RDWR|O_CREAT, S_IRUSR|S_IWUSR); - if (fd == -1) - log_fatal(USE_ERRNO, N_("unable to open %s"), pathbuf); + if (fd == -1) { + log_warning(USE_ERRNO, N_("unable to open %s"), pathbuf); + debug_return_bool(false); + } lock_file(fd, SUDO_LOCK); /* @@ -225,8 +244,10 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]) if (id == 0) { nread = read(fd, buf, sizeof(buf) - 1); if (nread != 0) { - if (nread == -1) - log_fatal(USE_ERRNO, N_("unable to read %s"), pathbuf); + if (nread == -1) { + log_warning(USE_ERRNO, N_("unable to read %s"), pathbuf); + debug_return_bool(false); + } if (buf[nread - 1] == '\n') nread--; buf[nread] = '\0'; @@ -255,16 +276,19 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]) sessid[6] = '\0'; /* Rewind and overwrite old seq file, including the NUL byte. */ - if (lseek(fd, (off_t)0, SEEK_SET) == (off_t)-1 || write(fd, buf, 7) != 7) - log_fatal(USE_ERRNO, N_("unable to write to %s"), pathbuf); + if (lseek(fd, (off_t)0, SEEK_SET) == (off_t)-1 || write(fd, buf, 7) != 7) { + log_warning(USE_ERRNO, N_("unable to write to %s"), pathbuf); + debug_return_bool(false); + } close(fd); - debug_return; + debug_return_bool(true); } /* * Copy iolog_path to pathbuf and create the directory and any intermediate * directories. If iolog_path ends in 'XXXXXX', use mkdtemp(). + * Returns SIZE_MAX on error. */ static size_t mkdir_iopath(const char *iolog_path, char *pathbuf, size_t pathsize) @@ -276,7 +300,8 @@ mkdir_iopath(const char *iolog_path, char *pathbuf, size_t pathsize) len = strlcpy(pathbuf, iolog_path, pathsize); if (len >= pathsize) { errno = ENAMETOOLONG; - log_fatal(USE_ERRNO, "%s", iolog_path); + log_warning(USE_ERRNO, "%s", iolog_path); + debug_return_size_t((size_t)-1); } /* @@ -285,7 +310,8 @@ mkdir_iopath(const char *iolog_path, char *pathbuf, size_t pathsize) */ if (len >= 6 && strcmp(&pathbuf[len - 6], "XXXXXX") == 0) is_temp = true; - io_mkdirs(pathbuf, S_IRWXU, is_temp); + if (!io_mkdirs(pathbuf, S_IRWXU, is_temp)) + len = (size_t)-1; debug_return_size_t(len); } @@ -296,7 +322,7 @@ mkdir_iopath(const char *iolog_path, char *pathbuf, size_t pathsize) * Uses zlib if docompress is true. * Stores the open file handle which has the close-on-exec flag set. */ -static void +static bool open_io_fd(char *pathbuf, size_t len, struct io_log_file *iol, bool docompress) { int fd; @@ -315,13 +341,15 @@ open_io_fd(char *pathbuf, size_t len, struct io_log_file *iol, bool docompress) #endif iol->fd.f = fdopen(fd, "w"); } - if (fd == -1 || iol->fd.v == NULL) - log_fatal(USE_ERRNO, N_("unable to create %s"), pathbuf); + if (fd == -1 || iol->fd.v == NULL) { + log_warning(USE_ERRNO, N_("unable to create %s"), pathbuf); + debug_return_bool(false); + } } else { /* Remove old log file if we recycled sequence numbers. */ unlink(pathbuf); } - debug_return; + debug_return_bool(true); } /* @@ -427,8 +455,10 @@ iolog_deserialize_info(struct iolog_details *details, char * const user_info[], } break; case 'm': - if (strncmp(*cur, "maxseq=", sizeof("maxseq=") - 1) == 0) + if (strncmp(*cur, "maxseq=", sizeof("maxseq=") - 1) == 0) { io_set_max_sessid(*cur + sizeof("maxseq=") - 1); + continue; + } break; case 'r': if (strncmp(*cur, "runas_gid=", sizeof("runas_gid=") - 1) == 0) { @@ -497,19 +527,22 @@ iolog_deserialize_info(struct iolog_details *details, char * const user_info[], /* * Write the "/log" file that contains the user and command info. */ -void +static bool write_info_log(char *pathbuf, size_t len, struct iolog_details *details, char * const argv[], struct timeval *now) { char * const *av; FILE *fp; int fd; + debug_decl(write_info_log, SUDO_DEBUG_UTIL) pathbuf[len] = '\0'; strlcat(pathbuf, "/log", PATH_MAX); fd = open(pathbuf, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR); - if (fd == -1 || (fp = fdopen(fd, "w")) == NULL) - log_fatal(USE_ERRNO, N_("unable to create %s"), pathbuf); + if (fd == -1 || (fp = fdopen(fd, "w")) == NULL) { + log_warning(USE_ERRNO, N_("unable to create %s"), pathbuf); + debug_return_bool(false); + } fprintf(fp, "%lld:%s:%s:%s:%s:%d:%d\n%s\n%s", (long long)now->tv_sec, details->user ? details->user : "unknown", details->runas_pw->pw_name, @@ -523,6 +556,7 @@ write_info_log(char *pathbuf, size_t len, struct iolog_details *details, } fputc('\n', fp); fclose(fp); + debug_return_bool(ferror(fp) == 0); } static int @@ -583,7 +617,10 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation, /* Get next session ID and convert it into a path. */ tofree = emalloc(sizeof(_PATH_SUDO_IO_LOGDIR) + sizeof(sessid) + 2); memcpy(tofree, _PATH_SUDO_IO_LOGDIR, sizeof(_PATH_SUDO_IO_LOGDIR)); - io_nextid(tofree, NULL, sessid); + if (!io_nextid(tofree, NULL, sessid)) { + rval = false; + goto done; + } snprintf(tofree + sizeof(_PATH_SUDO_IO_LOGDIR), sizeof(sessid) + 2, "%c%c/%c%c/%c%c", sessid[0], sessid[1], sessid[2], sessid[3], sessid[4], sessid[5]); @@ -603,8 +640,10 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation, write_info_log(pathbuf, len, &details, argv, &last_time); /* Create the timing and I/O log files. */ - for (i = 0; i < IOFD_MAX; i++) - open_io_fd(pathbuf, len, &io_log_files[i], iolog_compress); + for (i = 0; i < IOFD_MAX; i++) { + if (!open_io_fd(pathbuf, len, &io_log_files[i], iolog_compress)) + goto done; + } /* * Clear I/O log function pointers for disabled log functions. diff --git a/plugins/sudoers/iolog_path.c b/plugins/sudoers/iolog_path.c index e3f187f8a..d060c3828 100644 --- a/plugins/sudoers/iolog_path.c +++ b/plugins/sudoers/iolog_path.c @@ -57,8 +57,10 @@ fill_seq(char *str, size_t strsize, char *logdir) int len; debug_decl(fill_seq, SUDO_DEBUG_UTIL) - if (sessid[0] == '\0') - io_nextid(logdir, def_iolog_dir, sessid); + if (sessid[0] == '\0') { + if (!io_nextid(logdir, def_iolog_dir, sessid)) + debug_return_size_t((size_t)-1); + } /* Path is of the form /var/log/sudo-io/00/00/01. */ len = snprintf(str, strsize, "%c%c/%c%c/%c%c", sessid[0], diff --git a/plugins/sudoers/regress/iolog_path/check_iolog_path.c b/plugins/sudoers/regress/iolog_path/check_iolog_path.c index 2d2a8f1c3..a26cc0bc1 100644 --- a/plugins/sudoers/regress/iolog_path/check_iolog_path.c +++ b/plugins/sudoers/regress/iolog_path/check_iolog_path.c @@ -203,7 +203,9 @@ main(int argc, char *argv[]) exit(errors); } -void io_nextid(char *iolog_dir, char *fallback, char id[7]) +bool +io_nextid(char *iolog_dir, char *fallback, char id[7]) { memcpy(id, sessid, sizeof(sessid)); + return true; } diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c index a99be6bc8..13084dec8 100644 --- a/plugins/sudoers/sudoers.c +++ b/plugins/sudoers/sudoers.c @@ -82,8 +82,8 @@ * Prototypes */ static char *find_editor(int nfiles, char **files, char ***argv_out); -static int cb_runas_default(const char *); -static int cb_sudoers_locale(const char *); +static bool cb_runas_default(const char *); +static bool cb_sudoers_locale(const char *); static int set_cmnd(void); static void create_admin_success_flag(void); static void init_vars(char * const *); @@ -895,7 +895,7 @@ set_runasgr(const char *group) /* * Callback for runas_default sudoers setting. */ -static int +static bool cb_runas_default(const char *user) { /* Only reset runaspw if user didn't specify one. */ @@ -907,7 +907,7 @@ cb_runas_default(const char *user) /* * Callback for sudoers_locale sudoers setting. */ -static int +static bool cb_sudoers_locale(const char *locale) { sudoers_initlocale(NULL, locale); diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h index 08bb36954..0780422ea 100644 --- a/plugins/sudoers/sudoers.h +++ b/plugins/sudoers/sudoers.h @@ -328,8 +328,8 @@ char *get_timestr(time_t, int); int get_boottime(struct timeval *); /* iolog.c */ -int io_set_max_sessid(const char *sessid); -void io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]); +bool io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7]); +bool io_set_max_sessid(const char *sessid); /* iolog_path.c */ char *expand_iolog_path(const char *prefix, const char *dir, const char *file, diff --git a/plugins/sudoers/testsudoers.c b/plugins/sudoers/testsudoers.c index c7fdaf862..f924f54a2 100644 --- a/plugins/sudoers/testsudoers.c +++ b/plugins/sudoers/testsudoers.c @@ -78,7 +78,7 @@ void print_userspecs(void); void usage(void) __attribute__((__noreturn__)); static void set_runaspw(const char *); static void set_runasgr(const char *); -static int cb_runas_default(const char *); +static bool cb_runas_default(const char *); static int testsudoers_print(const char *msg); extern void setgrfile(const char *); @@ -395,7 +395,7 @@ set_runasgr(const char *group) /* * Callback for runas_default sudoers setting. */ -static int +static bool cb_runas_default(const char *user) { /* Only reset runaspw if user didn't specify one. */ -- 2.40.0