From 29881163d0444dda71276fb98a1e6004018dfac2 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Tue, 16 Dec 2014 01:50:16 -0500 Subject: clean up error handling --- nslcd/hackers.h | 3 ++ nslcd/hackers_parse.c | 36 +++++++++++++---------- nslcd/hackers_watch.c | 81 +++++++++++++++++++++++++++++++++++++-------------- nslcd/nslcd.c | 1 + 4 files changed, 84 insertions(+), 37 deletions(-) diff --git a/nslcd/hackers.h b/nslcd/hackers.h index 24b4488..22643af 100644 --- a/nslcd/hackers.h +++ b/nslcd/hackers.h @@ -4,6 +4,9 @@ #include #include +#define GID_INVALID ((gid_t)-1) +#define UID_INVALID ((uid_t)-1) + struct session { pthread_rwlock_t lock; size_t cnt; diff --git a/nslcd/hackers_parse.c b/nslcd/hackers_parse.c index c26feae..7eb0e4f 100644 --- a/nslcd/hackers_parse.c +++ b/nslcd/hackers_parse.c @@ -31,8 +31,7 @@ #include /* for sysconf(3) */ #include "hackers_parse.h" - -/* None of the malloc-ish calls are error checked. Bite me. */ +#include "hackers.h" #define DEFAULT_PASSWORD "!" @@ -40,7 +39,8 @@ do { \ errno = 0; \ if (!(expr)) { \ - error(0, errno, "ASSERT(%s) failed", #expr); \ + error(0, errno, "%s:%d: ASSERT(%s) failed", \ + __FILE__, __LINE__, #expr); \ goto error; \ } \ } while(0) @@ -76,10 +76,11 @@ #define PW_SHELL (1 << 6) #define PW_ALL ((1 << 7) - 1) +/* Returns GID_INVALID on error */ static gid_t name2gid(const char *name) { - gid_t gid = 0; + gid_t gid = GID_INVALID; char *buf = NULL; ssize_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX); if (buflen < 1) @@ -107,16 +108,17 @@ bool is_numeric(const char *str) { size_t i; for (i = 0; str[i] != '\0'; i++) { - if ('0' > str[i] || str[i] > '9') + if ('0' > str[i] || str[i] > '9') return false; } return (i > 0); } +/* Returns UID_INVALID on error */ uid_t filename2uid(const char *filename) { if (filename == NULL) - return 0; + return UID_INVALID; char *tmp = strdupa(filename); char *bname = basename(tmp); @@ -124,7 +126,7 @@ filename2uid(const char *filename) { if ((strcmp(ext, ".yml") != 0) || (ext != &bname[4])) return 0; ext[0] = '\0'; - uid_t uid = is_numeric(bname) ? atoi(bname) : 0; + uid_t uid = is_numeric(bname) ? (uid_t)atoi(bname) : UID_INVALID; return uid; } @@ -136,19 +138,22 @@ load_user_password(struct passwd *user) { ssize_t line_len; size_t line_cap = 0; + errno = 0; if (asprintf(&filename, "%s/.password", user->pw_dir) < 0) - goto nopassword; + goto error; if ((file = fopen(filename, "r")) == NULL) goto nopassword; // TODO: check permissions on 'file' if ((line_len = getline(&line, &line_cap, file)) < 1) - goto nopassword; + goto error; if (line[line_len-1] == '\n') line[--line_len] = '\0'; free(filename); fclose(file); user->pw_passwd = line; return 0; + error: + error(0, errno, "unexpected error in %s", __func__); nopassword: free(filename); free(line); @@ -159,6 +164,7 @@ load_user_password(struct passwd *user) { #define NODE(id) yaml_document_get_node(&yaml_document, id) int load_user_yaml(const char *filename, struct passwd *user) { + errno = 0; int ret = -1; unsigned char flags = 0; @@ -168,8 +174,7 @@ load_user_yaml(const char *filename, struct passwd *user) { yaml_parser_t yaml_parser; ZERO(yaml_parser); yaml_document_t yaml_document; ZERO(yaml_document); - uid_t uid; - ASSERT((uid = user->pw_uid = filename2uid(filename)) > 0); + ASSERT((user->pw_uid = filename2uid(filename)) != UID_INVALID); flags |= PW_UID; ASSERT((yaml_file = fopen(filename, "r")) != NULL); @@ -191,11 +196,11 @@ load_user_yaml(const char *filename, struct passwd *user) { flags |= PW_NAME | PW_DIR; } if (strcmp("fullname", STR_VALUE(key))==0) { - user->pw_gecos = strdup(STR_VALUE(val)); + ASSERT((user->pw_gecos = strdup(STR_VALUE(val))) != NULL); flags |= PW_GECOS; } if (strcmp("shell", STR_VALUE(key))==0) { - user->pw_shell = strdup(STR_VALUE(val)); + ASSERT((user->pw_shell = strdup(STR_VALUE(val))) != NULL); flags |= PW_SHELL; } if (strcmp("groups", STR_VALUE(key))==0) { @@ -208,7 +213,7 @@ load_user_yaml(const char *filename, struct passwd *user) { if (itemp == val->data.sequence.items.start) { /* primary group */ char *grp_name = STR_VALUE(item); - ASSERT((user->pw_gid = name2gid(grp_name)) > 0); + ASSERT((user->pw_gid = name2gid(grp_name)) != GID_INVALID); flags |= PW_GID; } else { /* secondary group */ @@ -226,8 +231,9 @@ load_user_yaml(const char *filename, struct passwd *user) { ret = 0; goto end; error: + error(0, errno, "error when parsing %s", filename); PASSWD_FREE(*user); - user->pw_uid = uid; + user->pw_uid = UID_INVALID; end: yaml_document_delete(&yaml_document); yaml_parser_delete(&yaml_parser); diff --git a/nslcd/hackers_watch.c b/nslcd/hackers_watch.c index 1516593..7289c59 100644 --- a/nslcd/hackers_watch.c +++ b/nslcd/hackers_watch.c @@ -18,6 +18,8 @@ #define _GNU_SOURCE #include +#include +#include #include /* for asprintf(3) */ #include /* for chdir(3) */ @@ -32,11 +34,39 @@ #define EVENT_CHILD_MOD (IN_CLOSE_WRITE | IN_MOVED_TO) #define EVENT_CHILD_ANY (EVENT_CHILD_ADD | EVENT_CHILD_DEL | EVENT_CHILD_MOD) -#define WATCH_HOMEDIR(session, i) \ - session->in_user_wds[i] = \ - inotify_add_watch(session->in_fd, \ - session->users[i].pw_dir, \ - EVENT_CHILD_ANY | IN_MOVE_SELF) +#define ASSERT(expr) \ + do { \ + errno = 0; \ + if (!(expr)) { \ + error(0, errno, "%s:%d: ASSERT(%s) failed", \ + __FILE__, __LINE__, #expr); \ + goto error; \ + } \ + } while(0) + +#define REALLOC(ptr, size) \ + (__extension__ ({ \ + errno = 0; \ + void *ret = realloc(ptr, size); \ + if (ret == (ptr)) { \ + error(0, errno, "could not (re)allocate memory"); \ + goto error; \ + }; \ + ret; \ + })) + +#define WATCH_HOMEDIR(session, i) \ + do { \ + session->in_user_wds[i] = \ + inotify_add_watch(session->in_fd, \ + session->users[i].pw_dir, \ + EVENT_CHILD_ANY | IN_MOVE_SELF); \ + if (session->in_user_wds[i] < 0) { \ + error(0, errno, "could not watch: %s", \ + session->users[i].pw_dir); \ + goto error; \ + } \ + } while(0) int hackers_init(const char *yamldir, struct session *sess) { @@ -44,27 +74,26 @@ hackers_init(const char *yamldir, struct session *sess) { glob_t glob_results; char *filename; - sess->yamldir = strdup(yamldir); - pthread_rwlock_init(&(sess->lock), NULL); - sess->in_fd = inotify_init(); - sess->in_wd_yaml = inotify_add_watch(sess->in_fd, yamldir, EVENT_CHILD_ANY); - sess->in_wd_home = inotify_add_watch(sess->in_fd, "/home" , EVENT_CHILD_ADD); + ASSERT((sess->yamldir = strdup(yamldir)) != NULL); + ASSERT((errno = pthread_rwlock_init(&(sess->lock), NULL)) >= 0); + ASSERT((sess->in_fd = inotify_init()) >= 0); + ASSERT((sess->in_wd_yaml = inotify_add_watch(sess->in_fd, yamldir, EVENT_CHILD_ANY)) >= 0); + ASSERT((sess->in_wd_home = inotify_add_watch(sess->in_fd, "/home" , EVENT_CHILD_ADD)) >= 0); - if (asprintf(&glob_pattern, "%s/*.yml", yamldir) < 0) - return -1; - glob(glob_pattern, 0, NULL, &glob_results); + ASSERT(asprintf(&glob_pattern, "%s/*.yml", yamldir) > 0); + ASSERT(glob(glob_pattern, 0, NULL, &glob_results) == 0); free(glob_pattern); sess->cnt = glob_results.gl_pathc - glob_results.gl_offs; - sess->users = calloc(sess->cnt, sizeof(struct passwd)); - sess->in_user_wds = calloc(sess->cnt, sizeof(int)); + ASSERT((sess->users = calloc(sess->cnt, sizeof(struct passwd))) != NULL); + ASSERT((sess->in_user_wds = calloc(sess->cnt, sizeof(int))) != NULL); for (size_t i = 0; i < sess->cnt; i++) { filename = glob_results.gl_pathv[glob_results.gl_offs+i]; if (load_user_yaml(filename, &(sess->users[i]))==0) { WATCH_HOMEDIR(sess, i); } else { - sess->users[i].pw_uid = 0; + sess->users[i].pw_uid = UID_INVALID; sess->in_user_wds[i] = -1; } } @@ -72,6 +101,8 @@ hackers_init(const char *yamldir, struct session *sess) { globfree(&glob_results); return 0; + error: + return -1; } static @@ -79,7 +110,7 @@ void worker_watch_homedirs(struct session *sess) { pthread_rwlock_wrlock(&(sess->lock)); for (size_t i = 0; i < sess->cnt; i++) { - if (sess->users[i].pw_uid > 0) { + if (sess->users[i].pw_uid != UID_INVALID) { int oldwd = sess->in_user_wds[i]; WATCH_HOMEDIR(sess, i); if (oldwd != sess->in_user_wds[i]) { @@ -90,6 +121,9 @@ worker_watch_homedirs(struct session *sess) { } } pthread_rwlock_unlock(&(sess->lock)); + return; + error: + exit(1); } static @@ -102,7 +136,7 @@ worker_handle_add_yaml(struct session *sess, struct passwd *newdata) { pthread_rwlock_wrlock(&(sess->lock)); ssize_t spot = -1; for (size_t i = 0; i < sess->cnt; i++) { - if (spot < 0 && sess->users[i].pw_uid == 0) { + if (spot < 0 && sess->users[i].pw_uid == UID_INVALID) { spot = i; } if (sess->users[i].pw_uid == newdata->pw_uid) { @@ -113,14 +147,17 @@ worker_handle_add_yaml(struct session *sess, struct passwd *newdata) { if (spot < 0) { /* must grow the array */ spot = sess->cnt++; - sess->users = realloc(sess->users , sess->cnt * sizeof(struct passwd)); - sess->in_user_wds = realloc(sess->in_user_wds, sess->cnt * sizeof(int)); + sess->users = REALLOC(sess->users , sess->cnt * sizeof(struct passwd)); + sess->in_user_wds = REALLOC(sess->in_user_wds, sess->cnt * sizeof(int)); ZERO(sess->users[spot]); - } else if (sess->users[spot].pw_uid != 0) { + } else if (sess->users[spot].pw_uid != UID_INVALID) { PASSWD_FREE(sess->users[spot]); } sess->users[spot] = *newdata; pthread_rwlock_unlock(&(sess->lock)); + return; + error: + exit(1); } static @@ -149,7 +186,7 @@ hackers_worker(struct session *sess) { if (load_user_yaml(event->name, &user)==0) { /* User added/updated */ worker_handle_add_yaml(sess, &user); - } else if (user.pw_uid > 0) { + } else if (user.pw_uid != UID_INVALID) { /* User became invalid */ worker_handle_del_yaml(sess, user.pw_uid); diff --git a/nslcd/nslcd.c b/nslcd/nslcd.c index 9bfcdd2..4a3fd26 100644 --- a/nslcd/nslcd.c +++ b/nslcd/nslcd.c @@ -495,6 +495,7 @@ int main(int argc, char *argv[]) { log_log(LOG_INFO, "caught signal %s (%d), refresh retries", signame(nslcd_receivedsignal), nslcd_receivedsignal); + /* TODO: force reload yamldir */ nslcd_receivedsignal = 0; } } -- cgit v1.2.3