From: Timo Sirainen Date: Mon, 4 May 2009 22:28:17 +0000 (-0400) Subject: Require each service to have a unique name. Log service errors using service_error(). X-Git-Tag: 2.0.alpha1~839 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=55bc6a7a0940ec48a68558ef70838991c5d301d2;p=thirdparty%2Fdovecot%2Fcore.git Require each service to have a unique name. Log service errors using service_error(). --HG-- branch : HEAD --- diff --git a/src/master/master-settings.c b/src/master/master-settings.c index b97b96cd3c..46a8bf2728 100644 --- a/src/master/master-settings.c +++ b/src/master/master-settings.c @@ -82,6 +82,7 @@ static struct setting_parser_info inet_listener_setting_parser_info = { static struct setting_define service_setting_defines[] = { DEF(SET_INTERNAL, master_set), + DEF(SET_STR, name), DEF(SET_STR, type), DEF(SET_STR, executable), DEF(SET_STR, user), @@ -110,6 +111,7 @@ static struct setting_define service_setting_defines[] = { static struct service_settings service_default_settings = { MEMBER(master_set) NULL, + MEMBER(name) "", MEMBER(type) "", MEMBER(executable) "", MEMBER(user) "", @@ -138,7 +140,7 @@ struct setting_parser_info service_setting_parser_info = { MEMBER(dynamic_parsers) NULL, MEMBER(parent_offset) offsetof(struct service_settings, master_set), - MEMBER(type_offset) (size_t)-1, + MEMBER(type_offset) offsetof(struct service_settings, name), MEMBER(struct_size) sizeof(struct service_settings) }; @@ -217,7 +219,7 @@ master_settings_verify(void *_set, pool_t pool, const char **error_r) { const struct master_settings *set = _set; struct service_settings *const *services; - unsigned int i, count; + unsigned int i, j, count; if (set->last_valid_uid != 0 && set->first_valid_uid > set->last_valid_uid) { @@ -237,6 +239,21 @@ master_settings_verify(void *_set, pool_t pool, const char **error_r) *error_r = "No services defined"; return FALSE; } + for (i = 0; i < count; i++) { + if (*services[i]->name == '\0') { + *error_r = t_strdup_printf( + "Service #%d is missing name", i); + return FALSE; + } + for (j = 0; j < i; j++) { + if (strcmp(services[i]->name, services[j]->name) == 0) { + *error_r = t_strdup_printf( + "Duplicate service name: %s", + services[i]->name); + return FALSE; + } + } + } for (i = 0; i < count; i++) { if (*services[i]->executable != '/') { services[i]->executable = @@ -253,7 +270,7 @@ master_settings_verify(void *_set, pool_t pool, const char **error_r) *services[i]->chroot != '\0') { *error_r = t_strdup_printf("service(%s): " "drop_priv_before_exec=yes can't be " - "used with chroot", services[i]->executable); + "used with chroot", services[i]->name); return FALSE; } fix_file_listener_paths(&services[i]->unix_listeners, diff --git a/src/master/master-settings.h b/src/master/master-settings.h index 7a1133bcec..ead97d6d56 100644 --- a/src/master/master-settings.h +++ b/src/master/master-settings.h @@ -17,6 +17,7 @@ struct inet_listener_settings { struct service_settings { struct master_settings *master_set; + const char *name; const char *type; const char *executable; const char *user; diff --git a/src/master/service-auth-server.c b/src/master/service-auth-server.c index 8c3541827a..0996bc9a8b 100644 --- a/src/master/service-auth-server.c +++ b/src/master/service-auth-server.c @@ -66,10 +66,10 @@ auth_process_lookup_request(struct service_process_auth_server *process, request = hash_table_lookup(process->auth_requests, POINTER_CAST(id)); if (request == NULL) { - i_error("service(%s): authentication service %s " - "sent reply with unknown ID %u", - process->process.service->name, - dec2str(process->process.pid), id); + service_error(process->process.service, + "authentication service %s " + "sent reply with unknown ID %u", + dec2str(process->process.pid), id); return NULL; } @@ -181,9 +181,10 @@ service_process_auth_server_input(struct service_process_auth_server *process) return; case -2: /* buffer full */ - i_error("service(%s): authentication server process %s " - "sent us too long line", process->process.service->name, - dec2str(process->process.pid)); + service_error(process->process.service, + "authentication server process %s " + "sent us too long line", + dec2str(process->process.pid)); service_process_auth_server_close(process); return; } @@ -197,11 +198,11 @@ service_process_auth_server_input(struct service_process_auth_server *process) if (strncmp(line, "VERSION\t", 8) != 0 || atoi(t_strcut(line + 8, '\t')) != AUTH_MASTER_PROTOCOL_MAJOR_VERSION) { - i_error("service(%s): authentication server process %s " - "not compatible with master process " - "(mixed old and new binaries?)", - process->process.service->name, - dec2str(process->process.pid)); + service_error(process->process.service, + "authentication server process %s " + "not compatible with master process " + "(mixed old and new binaries?)", + dec2str(process->process.pid)); service_process_auth_server_close(process); return; } diff --git a/src/master/service-auth-source.c b/src/master/service-auth-source.c index 057056f437..28798f89ba 100644 --- a/src/master/service-auth-source.c +++ b/src/master/service-auth-source.c @@ -128,22 +128,20 @@ auth_read_request(struct service_process_auth_source *process, /* disconnected */ } else if (ret > 0) { /* request wasn't fully read */ - i_error("service(%s): fd_read() partial input (%d/%d)", - service->name, (int)ret, (int)sizeof(*req)); + service_error(service, "fd_read() partial input (%d/%d)", + (int)ret, (int)sizeof(*req)); } else { if (errno == EAGAIN) return 0; - i_error("service(%s): fd_read() failed: %m", - service->name); + service_error(service, "fd_read() failed: %m"); } return -1; } if (req->data_size != 0) { if (req->data_size > MASTER_AUTH_MAX_DATA_SIZE) { - i_error("service(%s): Too large auth data_size sent", - service->name); + service_error(service, "Too large auth data_size sent"); return -1; } /* @UNSAFE */ @@ -153,30 +151,28 @@ auth_read_request(struct service_process_auth_source *process, /* disconnected */ } else if (ret > 0) { /* request wasn't fully read */ - i_error("service(%s): Data read partially %d/%u", - service->name, (int)ret, req->data_size); + service_error(service, + "Data read partially %d/%u", + (int)ret, req->data_size); } else { - i_error("service(%s): read(data) failed: %m", - service->name); + service_error(service, "read(data) failed: %m"); } return -1; } } if (*client_fd_r == -1) { - i_error("service(%s): Auth request missing a file descriptor", - service->name); + service_error(service, "Auth request missing a file descriptor"); return -1; } if (fstat(*client_fd_r, &st) < 0) { - i_error("service(%s): fstat(auth dest fd) failed: %m", - service->name); + service_error(service, "fstat(auth dest fd) failed: %m"); return -1; } if (st.st_ino != req->ino) { - i_error("service(%s): Auth request inode mismatch: %s != %s", - service->name, dec2str(st.st_ino), dec2str(req->ino)); + service_error(service, "Auth request inode mismatch: %s != %s", + dec2str(st.st_ino), dec2str(req->ino)); return -1; } return 1; @@ -210,9 +206,8 @@ service_process_auth_source_input(struct service_process_auth_source *process) /* we have a request. check its validity. */ auth_process = hash_table_lookup(service->list->pids, &req.auth_pid); if (auth_process == NULL) { - i_error("service(%s): authentication request for unknown " - "auth server PID %s", service->name, - dec2str(req.auth_pid)); + service_error(service, "authentication request for unknown " + "auth server PID %s", dec2str(req.auth_pid)); service_process_auth_source_send_reply(process, req.tag, FALSE); (void)close(client_fd); return; @@ -224,7 +219,7 @@ service_process_auth_source_input(struct service_process_auth_source *process) ioloop_time - AUTH_BUSY_LOG_INTERVAL) { i_warning("service(%s): authentication server PID " "%s too busy", - auth_process->process.service->name, + auth_process->process.service->set->name, dec2str(req.auth_pid)); auth_process->auth_busy_stamp = ioloop_time; } diff --git a/src/master/service-listen.c b/src/master/service-listen.c index 2a07710084..4345134f15 100644 --- a/src/master/service-listen.c +++ b/src/master/service-listen.c @@ -31,8 +31,8 @@ static int service_unix_listener_listen(struct service_listener *l) } if (errno != EADDRINUSE) { - i_error("service(%s): net_listen_unix(%s) failed: %m", - service->name, set->path); + service_error(service, "net_listen_unix(%s) failed: %m", + set->path); return -1; } @@ -42,15 +42,15 @@ static int service_unix_listener_listen(struct service_listener *l) if (fd != -1 || errno != ECONNREFUSED || i >= 3) { if (fd != -1) (void)close(fd); - i_error("service(%s): Socket already exists: %s", - service->name, set->path); + service_error(service, "Socket already exists: %s", + set->path); return 0; } /* delete and try again */ if (unlink(set->path) < 0 && errno != ENOENT) { - i_error("service(%s): unlink(%s) failed: %m", - service->name, set->path); + service_error(service, "unlink(%s) failed: %m", + set->path); return -1; } } @@ -88,15 +88,13 @@ static int service_fifo_listener_listen(struct service_listener *l) umask(old_umask); if (ret < 0 && errno != EEXIST) { - i_error("service(%s): mkfifo(%s) failed: %m", - service->name, set->path); + service_error(service, "mkfifo(%s) failed: %m", set->path); return -1; } fd = open(set->path, O_RDONLY); if (fd == -1) { - i_error("service(%s): open(%s) failed: %m", - service->name, set->path); + service_error(service, "open(%s) failed: %m", set->path); return -1; } @@ -127,8 +125,8 @@ static int service_inet_listener_listen(struct service_listener *l) fd = net_listen(&l->set.inetset.ip, &port, service->process_limit); if (fd < 0) { - i_error("service(%s): listen(%s, %u) failed: %m", - service->name, set->address, set->port); + service_error(service, "listen(%s, %u) failed: %m", + set->address, set->port); return errno == EADDRINUSE ? 0 : -1; } net_set_nonblock(fd, TRUE); diff --git a/src/master/service-log.c b/src/master/service-log.c index 7566a52b63..c1dc3fbfe4 100644 --- a/src/master/service-log.c +++ b/src/master/service-log.c @@ -37,12 +37,12 @@ int services_log_init(struct service_list *service_list) fd_close_on_exec(services[i]->log_fd[0], TRUE); fd_close_on_exec(services[i]->log_fd[1], TRUE); - handshake.prefix_len = strlen(services[i]->name) + 2; + handshake.prefix_len = strlen(services[i]->set->name) + 2; buffer_set_used_size(handshake_buf, 0); buffer_append(handshake_buf, &handshake, sizeof(handshake)); - buffer_append(handshake_buf, services[i]->name, - strlen(services[i]->name)); + buffer_append(handshake_buf, services[i]->set->name, + strlen(services[i]->set->name)); buffer_append(handshake_buf, ": ", 2); ret = write(services[i]->log_fd[1], @@ -89,12 +89,12 @@ void services_log_deinit(struct service_list *service_list) for (i = 0; i < count; i++) { if (services[i]->log_fd[0] != -1) { if (close(services[i]->log_fd[0]) < 0) { - i_error("service(%s): close(log_fd) failed: %m", - services[i]->name); + service_error(services[i], + "close(log_fd) failed: %m"); } if (close(services[i]->log_fd[1]) < 0) { - i_error("service(%s): close(log_fd) failed: %m", - services[i]->name); + service_error(services[i], + "close(log_fd) failed: %m"); } services[i]->log_fd[0] = -1; services[i]->log_fd[1] = -1; diff --git a/src/master/service-monitor.c b/src/master/service-monitor.c index 0ede21dd89..d92db983bb 100644 --- a/src/master/service-monitor.c +++ b/src/master/service-monitor.c @@ -29,17 +29,16 @@ static void service_status_input(struct service *service) ret = read(service->status_fd[0], &status, sizeof(status)); switch (ret) { case 0: - i_error("service(%s): read(status) failed: EOF", service->name); + service_error(service, "read(status) failed: EOF"); service_monitor_stop(service); return; case -1: - i_error("service(%s): read(status) failed: %m", service->name); + service_error(service, "read(status) failed: %m"); service_monitor_stop(service); return; default: - i_error("service(%s): child %s sent partial status update " - "(%d bytes)", service->name, - dec2str(status.pid), (int)ret); + service_error(service, "child %s sent partial status update " + "(%d bytes)", dec2str(status.pid), (int)ret); return; case sizeof(status): @@ -62,9 +61,8 @@ static void service_status_input(struct service *service) randomness here, but the worst they can do is DoS and there are already more serious problems if someone is able to do this.. */ - i_error("service(%s): Ignoring invalid update from child %s " - "(UID=%u)", service->name, dec2str(status.pid), - status.uid); + service_error(service, "Ignoring invalid update from child %s " + "(UID=%u)", dec2str(status.pid), status.uid); return; } @@ -111,8 +109,7 @@ static void service_monitor_throttle(struct service *service) if (service->to_throttle != NULL) return; - i_error("service(%s): command startup failed, throttling", - service->name); + service_error(service, "command startup failed, throttling"); service_monitor_listen_stop(service); service->to_throttle = timeout_add(THROTTLE_TIMEOUT, @@ -180,8 +177,7 @@ void services_monitor_start(struct service_list *service_list) if (services[i]->status_fd[0] == -1) { /* we haven't yet created status pipe */ if (pipe(services[i]->status_fd) < 0) { - i_error("service(%s): pipe() failed: %m", - services[i]->name); + service_error(services[i], "pipe() failed: %m"); continue; } @@ -215,10 +211,10 @@ static void service_monitor_stop(struct service *service) if (service->status_fd[0] != -1) { for (i = 0; i < 2; i++) { if (close(service->status_fd[i]) < 0) { - i_error("service(%s): close(%d) failed: %m", - service->name, i); + service_error(service, + "close(status fd) failed: %m"); } - service->status_fd[i] = -1; + service->status_fd[i] = -1; } } service_monitor_listen_stop(service); diff --git a/src/master/service-process.c b/src/master/service-process.c index 1286b9fe90..eb78edbba2 100644 --- a/src/master/service-process.c +++ b/src/master/service-process.c @@ -99,7 +99,7 @@ service_dup_fds(struct service *service, int auth_fd, int std_fd) closelog(); if (dup2_array(&dups) < 0) - i_fatal("service(%s): dup2s failed", service->name); + service_error(service, "dup2s failed"); #ifdef DEBUG env_put(t_strdup_printf("SOCKET_COUNT=%d", socket_listener_count)); @@ -290,12 +290,13 @@ service_process_setup_environment(struct service *service, unsigned int uid) static void service_process_status_timeout(struct service_process *process) { - i_error("service(%s): Initial status notification not received in %d " - "seconds, killing the process", process->service->name, - SERVICE_FIRST_STATUS_TIMEOUT_SECS); + service_error(process->service, + "Initial status notification not received in %d " + "seconds, killing the process", + SERVICE_FIRST_STATUS_TIMEOUT_SECS); if (kill(process->pid, SIGKILL) < 0 && errno != ESRCH) { - i_error("service(%s): kill(%s, SIGKILL) failed: %m", - process->service->name, dec2str(process->pid)); + service_error(process->service, "kill(%s, SIGKILL) failed: %m", + dec2str(process->pid)); } timeout_remove(&process->to_status); } @@ -315,8 +316,7 @@ service_process_create(struct service *service, const char *const *auth_args, case SERVICE_TYPE_AUTH_SOURCE: case SERVICE_TYPE_AUTH_SERVER: if (socketpair(AF_UNIX, SOCK_STREAM, 0, fd) < 0) { - i_error("service(%s): socketpair() failed: %m", - service->name); + service_error(service, "socketpair() failed: %m"); return NULL; } fd_close_on_exec(fd[0], TRUE); @@ -329,7 +329,7 @@ service_process_create(struct service *service, const char *const *auth_args, pid = fork(); if (pid < 0) { - i_error("service(%s): fork() failed: %m", service->name); + service_error(service, "fork() failed: %m"); if (fd[0] != -1) { (void)close(fd[0]); (void)close(fd[1]); @@ -543,7 +543,7 @@ service_process_get_status_error(string_t *str, struct service_process *process, *type_r = LOG_TYPE_ERROR; - str_printfa(str, "service(%s): child %s ", service->name, + str_printfa(str, "service(%s): child %s ", service->set->name, dec2str(process->pid)); if (WIFSIGNALED(status)) { str_printfa(str, "killed with signal %d", WTERMSIG(status)); diff --git a/src/master/service.c b/src/master/service.c index f6b0656158..b46f9792cf 100644 --- a/src/master/service.c +++ b/src/master/service.c @@ -13,6 +13,16 @@ #include #include +void service_error(struct service *service, const char *format, ...) +{ + va_list args; + + va_start(args, format); + i_error("service(%s): %s", service->set->name, + t_strdup_vprintf(format, args)); + va_end(args); +} + static int get_uid(const char *user, uid_t *uid_r, const char **error_r) { struct passwd *pw; @@ -148,7 +158,7 @@ service_create(pool_t pool, const struct service_settings *set, struct inet_listener_settings *const *inet_listeners; struct service *service; struct service_listener *l; - const char *p, *const *tmp; + const char *const *tmp; string_t *str; unsigned int i, unix_count, fifo_count, inet_count; @@ -158,15 +168,12 @@ service_create(pool_t pool, const struct service_settings *set, service->type = SERVICE_TYPE_UNKNOWN; if (*set->type != '\0') { - service->name = set->type; if (strcmp(set->type, "log") == 0) service->type = SERVICE_TYPE_LOG; else if (strcmp(set->type, "config") == 0) service->type = SERVICE_TYPE_CONFIG; else if (strcmp(set->type, "auth") == 0) service->type = SERVICE_TYPE_AUTH_SERVER; - else - service->name = NULL; } if (*set->auth_dest_service != '\0') @@ -188,17 +195,6 @@ service_create(pool_t pool, const struct service_settings *set, return NULL; } - /* get service name from some of the unique types, fallback to - executable name without path and parameters */ - if (service->name == NULL) { - p = strrchr(set->executable, '/'); - if (p == NULL) - service->name = t_strcut(set->executable, ' '); - else - service->name = t_strcut(p + 1, ' '); - service->name = p_strdup(pool, service->name); - } - if (get_uid(set->user, &service->uid, error_r) < 0) return NULL; if (get_gid(set->group, &service->gid, error_r) < 0) @@ -318,7 +314,7 @@ service_lookup(struct service_list *service_list, const char *name) services = array_get(&service_list->services, &count); for (i = 0; i < count; i++) { - if (strcmp(services[i]->name, name) == 0) + if (strcmp(services[i]->set->name, name) == 0) return services[i]; } return NULL; @@ -349,8 +345,7 @@ services_create(const struct master_settings *set, service_list, &error); if (service == NULL) { *error_r = t_strdup_printf("service(%s) %s", - service_settings[i]->type == NULL ? "unknown" : - service_settings[i]->type, error); + service_settings[i]->name, error); return NULL; } @@ -421,8 +416,8 @@ void service_signal(struct service *service, int signo) continue; if (kill(process->pid, signo) < 0 && errno != ESRCH) { - i_error("service(%s): kill(%s, %d) failed: %m", - service->name, dec2str(process->pid), signo); + service_error(service, "kill(%s, %d) failed: %m", + dec2str(process->pid), signo); } } hash_table_iterate_deinit(&iter); diff --git a/src/master/service.h b/src/master/service.h index 1dd71b95cd..bf4bd5794f 100644 --- a/src/master/service.h +++ b/src/master/service.h @@ -45,7 +45,6 @@ struct service { struct service_list *list; enum service_type type; - const char *name; const struct service_settings *set; const char *config_file_path; @@ -107,4 +106,7 @@ void services_destroy(struct service_list *service_list); /* Send a signal to all processes in a given service */ void service_signal(struct service *service, int signo); +void service_error(struct service *service, const char *format, ...) + ATTR_FORMAT(2, 3); + #endif