diff --git a/tools/acrn-crashlog/common/cmdutils.c b/tools/acrn-crashlog/common/cmdutils.c index ee4161847..55fd4614d 100644 --- a/tools/acrn-crashlog/common/cmdutils.c +++ b/tools/acrn-crashlog/common/cmdutils.c @@ -47,7 +47,7 @@ int execv_out2file(char *argv[], char *outfile) if (pid == 0) { int res; - int fd; + int fd = -1; if (outfile) { fd = open(outfile, O_WRONLY | O_CREAT | O_TRUNC, 0664); @@ -65,6 +65,8 @@ int execv_out2file(char *argv[], char *outfile) res = execvp(argv[0], argv); if (res == -1) { + if (fd > 0) + close(fd); LOGE("execvp (%s) failed, error (%s)\n", argv[0], strerror(errno)); return -1; @@ -211,8 +213,10 @@ char *exec_out2mem(char *fmt, ...) memsize += 1024; new = realloc(out, memsize); if (!new) { - if (out) + if (out) { free(out); + out = NULL; + } goto end; } else { out = new; diff --git a/tools/acrn-crashlog/common/fsutils.c b/tools/acrn-crashlog/common/fsutils.c index 65bc7747e..700ef2190 100644 --- a/tools/acrn-crashlog/common/fsutils.c +++ b/tools/acrn-crashlog/common/fsutils.c @@ -114,11 +114,12 @@ char *mm_get_line(struct mm_file_t *mfile, int line) if (line <= 0 || line > mm_count_lines(mfile)) return NULL; - else if (line == 1) + + if (line == 1) ret = begin; else { next = begin; - for (i = 2; i <= line; i++) + for (i = 2; i <= line && next; i++) next = strchr(next, '\n') + 1; ret = next; } @@ -820,7 +821,7 @@ int read_full_binary_file(const char *path, unsigned long *size, void **data) close_file(path, f); *data = buf; - *size = (unsigned int)_size; + *size = (unsigned long)_size; return 0; @@ -1112,7 +1113,7 @@ int read_file(const char *path, unsigned long *size, void **data) int len = 0; int fd = 0; int memsize = 1; /* for '\0' */ - size_t result = 0; + ssize_t result = 0; char *out = NULL; char *new; @@ -1129,6 +1130,8 @@ int read_file(const char *path, unsigned long *size, void **data) result = read(fd, (void *)tmp, 1024); if (result == 0) break; + else if (result == -1) + goto free; memsize += result; new = realloc(out, memsize); @@ -1145,7 +1148,7 @@ int read_file(const char *path, unsigned long *size, void **data) close(fd); *data = out; - *size = (unsigned int)len; + *size = (unsigned long)len; return 0; diff --git a/tools/acrn-crashlog/common/log_sys.c b/tools/acrn-crashlog/common/log_sys.c index e4a2c9d55..325172c24 100644 --- a/tools/acrn-crashlog/common/log_sys.c +++ b/tools/acrn-crashlog/common/log_sys.c @@ -19,7 +19,7 @@ void do_log(const int level, char log[MAX_LOG_LEN]; int n = 0; #ifdef DEBUG_ACRN_CRASHLOG - const char header_fmt[] = "<%-20s%d>: "; + const char header_fmt[] = "<%-20s%5d>: "; #endif if (level > LOG_LEVEL) @@ -37,7 +37,7 @@ void do_log(const int level, #ifdef DEBUG_ACRN_CRASHLOG /* header */ n = snprintf(log, sizeof(log), header_fmt, func, line); - if (n < 0 || n >= sizeof(log)) + if (n < 0 || (size_t)n >= sizeof(log)) n = 0; #endif /* msg */ diff --git a/tools/acrn-crashlog/common/strutils.c b/tools/acrn-crashlog/common/strutils.c index e45c3a8de..f67b3945b 100644 --- a/tools/acrn-crashlog/common/strutils.c +++ b/tools/acrn-crashlog/common/strutils.c @@ -74,11 +74,16 @@ static char *strtriml(char *str) static char *strtrimr(char *str) { - char *end = str + strlen(str) - 1; + size_t len; + char *end; - while (*end == ' ' && end >= str) { - *end = 0; - end--; + len = strlen(str); + if (len > 0) { + end = str + strlen(str) - 1; + while (*end == ' ' && end >= str) { + *end = 0; + end--; + } } return str; @@ -86,8 +91,12 @@ static char *strtrimr(char *str) char *strtrim(char *str) { - strtrimr(str); - return strtriml(str); + if (str) { + strtrimr(str); + return strtriml(str); + } + + return NULL; } int strcnt(char *str, char c) diff --git a/tools/acrn-crashlog/usercrash/client.c b/tools/acrn-crashlog/usercrash/client.c index c8e22c0eb..de5dd4819 100644 --- a/tools/acrn-crashlog/usercrash/client.c +++ b/tools/acrn-crashlog/usercrash/client.c @@ -72,8 +72,8 @@ static int set_timeout(int sockfd) * usercrashd_connect is used to connect to server, and get the crash file fd * from server */ -bool usercrashd_connect(int pid, int *usercrashd_socket, - int *output_fd, char *name) +static int usercrashd_connect(int pid, int *usercrashd_socket, + int *output_fd, const char *name) { int sockfd; int tmp_output_fd; @@ -81,15 +81,20 @@ bool usercrashd_connect(int pid, int *usercrashd_socket, int flags; struct crash_packet packet = {0}; + if (name == NULL) { + LOGE("crash process name is NULL\n"); + return -1; + } sockfd = socket_local_client(SOCKET_NAME, SOCK_SEQPACKET); if (sockfd == -1) { LOGE("failed to connect to usercrashd, error (%s)\n", strerror(errno)); - return false; + return -1; } packet.packet_type = kDumpRequest; packet.pid = pid; - strncpy(packet.name, name, COMM_NAME_LEN - 1); + strncpy(packet.name, name, COMM_NAME_LEN); + packet.name[COMM_NAME_LEN - 1] = '\0'; if (set_timeout(sockfd)) { close(sockfd); return -1; @@ -99,7 +104,7 @@ bool usercrashd_connect(int pid, int *usercrashd_socket, LOGE("failed to write DumpRequest packet, error (%s)\n", strerror(errno)); close(sockfd); - return false; + return -1; } rc = recv_fd(sockfd, &packet, sizeof(packet), @@ -108,7 +113,7 @@ bool usercrashd_connect(int pid, int *usercrashd_socket, LOGE("failed to read response to DumpRequest packet, "); LOGE("error (%s)\n", strerror(errno)); close(sockfd); - return false; + return -1; } else if (rc != sizeof(packet)) { LOGE("received DumpRequest response packet of incorrect "); LOGE("length (expected %lu, got %ld)\n", sizeof(packet), rc); @@ -135,11 +140,11 @@ bool usercrashd_connect(int pid, int *usercrashd_socket, *usercrashd_socket = sockfd; *output_fd = tmp_output_fd; - return true; + return 0; fail: close(sockfd); close(tmp_output_fd); - return false; + return -1; } @@ -149,7 +154,7 @@ fail: * dump, the server will pop another crash from the queue and execute the dump * process */ -bool usercrashd_notify_completion(int usercrashd_socket) +static int usercrashd_notify_completion(int usercrashd_socket) { struct crash_packet packet = {0}; @@ -160,14 +165,15 @@ bool usercrashd_notify_completion(int usercrashd_socket) } if (write(usercrashd_socket, &packet, sizeof(packet)) != sizeof(packet)) { - return false; + return -1; } - return true; + return 0; } static void print_usage(void) { - printf("It's the client role of usercrash.\n"); + printf("usercrash - tool to dump crash info for the crashes in the "); + printf("userspace on sos. It's the client role of usercrash.\n"); printf("[Usage]\n"); printf("\t--coredump, usercrash_c "); printf("(root role to run)\n"); @@ -209,14 +215,14 @@ int main(int argc, char *argv[]) pid = atoi(argv[1]); sig = atoi(argv[3]); ret = usercrashd_connect(pid, &sock, &out_fd, argv[2]); - if (!ret) { + if (ret) { LOGE("usercrashd_connect failed, error (%s)\n", strerror(errno)); exit(EXIT_FAILURE); } crash_dump(pid, sig, out_fd); close(out_fd); - if (!usercrashd_notify_completion(sock)) { + if (usercrashd_notify_completion(sock)) { LOGE("failed to notify usercrashd of completion"); close(sock); exit(EXIT_FAILURE); diff --git a/tools/acrn-crashlog/usercrash/crash_dump.c b/tools/acrn-crashlog/usercrash/crash_dump.c index 256211e06..1cfac635a 100644 --- a/tools/acrn-crashlog/usercrash/crash_dump.c +++ b/tools/acrn-crashlog/usercrash/crash_dump.c @@ -79,43 +79,35 @@ static const char *get_signame(int sig) */ static int save_coredump(const char *filename) { - size_t read; + size_t read_len; + size_t write_len; char input_buffer[BUFFER_SIZE]; FILE *dump_file; /* open dump file in write and binary mode */ dump_file = fopen(filename, "wb"); - if (dump_file != NULL) { - /* Read from STDIN and write to dump file */ - while (1) { - read = fread(input_buffer, 1, BUFFER_SIZE, stdin); - if (read > 0) { - if (fwrite(input_buffer, 1, read, dump_file) <= - 0) { - LOGE("fwrite error\n"); - goto fail; - } - continue; - } else if (read == 0) { - break; - } - - LOGE("fread error\n"); - goto fail; + if (dump_file == NULL) { + LOGE("fopen core file failed\n"); + return -1; + } + /* Read from STDIN and write to dump file */ + while (1) { + read_len = fread(input_buffer, 1, BUFFER_SIZE, stdin); + if (read_len == 0) + break; + write_len = fwrite(input_buffer, 1, read_len, dump_file); + if (write_len == 0) { + LOGE("fwrite error\n"); + fclose(dump_file); + return -1; } - fclose(dump_file); - return 0; } - LOGE("fopen core file failed\n"); - return -1; - -fail: fclose(dump_file); - return -1; + return 0; } -static int get_backtrace(int pid, int fd, int sig, char *comm) +static int get_backtrace(int pid, int fd, int sig, const char *comm) { char *membkt; char format[512]; @@ -150,7 +142,7 @@ static int get_backtrace(int pid, int fd, int sig, char *comm) * @name: a string save to file * return 0 on success, or -1 on error */ -static int save_proc_info(int pid, int fd, char *path, char *name) +static int save_proc_info(int pid, int fd, const char *path, const char *name) { int ret; char *data; @@ -171,7 +163,7 @@ static int save_proc_info(int pid, int fd, char *path, char *name) } -static int get_openfiles(int pid, int fd, char *path, char *name) +static int get_openfiles(int pid, int fd, const char *path, const char *name) { int ret; int loop; @@ -205,7 +197,7 @@ static int get_openfiles(int pid, int fd, char *path, char *name) return 0; } -static int save_usercrash_file(int pid, int tgid, char *comm, +static int save_usercrash_file(int pid, int tgid, const char *comm, int sig, int out_fd) { int ret; @@ -242,7 +234,8 @@ static int save_usercrash_file(int pid, int tgid, char *comm, return 0; } -static int get_key_value(int pid, char *path, char *key, char *value) +static int get_key_value(int pid, const char *path, const char *key, + char *value, size_t value_len) { int len; int ret; @@ -256,7 +249,7 @@ static int get_key_value(int pid, char *path, char *key, char *value) memset(format, 0, sizeof(format)); snprintf(format, sizeof(format), path, pid); ret = read_file(format, &size, (void *)&data); - if (ret) { + if (ret || !data) { LOGE("read file failed\n"); return -1; } @@ -271,6 +264,10 @@ static int get_key_value(int pid, char *path, char *key, char *value) end = data + size; start = msg + strlen(key); len = end - start; + if (len >= (int)value_len) { + free(data); + return -1; + } memcpy(value, start, len); *(value + len) = 0; free(data); @@ -301,7 +298,7 @@ void crash_dump(int pid, int sig, int out_fd) } comm[ret] = '\0'; - ret = get_key_value(pid, GET_TID, "Tgid:", result); + ret = get_key_value(pid, GET_TID, "Tgid:", result, sizeof(result)); if (ret) { LOGE("get Tgid error\n"); return; diff --git a/tools/acrn-crashlog/usercrash/debugger.c b/tools/acrn-crashlog/usercrash/debugger.c index 0d89becf3..894ad8a83 100644 --- a/tools/acrn-crashlog/usercrash/debugger.c +++ b/tools/acrn-crashlog/usercrash/debugger.c @@ -19,10 +19,9 @@ */ static void print_usage(void) { - printf("debugger - tool to dump process info of a running process. "); + printf("debugger - tool to dump process info of a running process.\n"); printf("[Usage]\n"); printf("\t--shell cmd, debugger (root role to run)\n"); - printf("(root role to run)\n"); printf("[Option]\n"); printf("\t-h: print this usage message\n"); printf("\t-v: print debugger version\n"); @@ -48,7 +47,7 @@ int main(int argc, char *argv[]) print_usage(); if (getuid() != 0) { - LOGE("failed to execute debugger, root is required\n"); + printf("failed to execute debugger, root is required\n"); exit(EXIT_FAILURE); } diff --git a/tools/acrn-crashlog/usercrash/include/packet.h b/tools/acrn-crashlog/usercrash/include/packet.h index 4c52118c6..0a970bb2b 100644 --- a/tools/acrn-crashlog/usercrash/include/packet.h +++ b/tools/acrn-crashlog/usercrash/include/packet.h @@ -10,7 +10,6 @@ #define SOCKET_NAME "user_crash" #include -#include enum CrashPacketType { /* Initial request from crash_dump */ diff --git a/tools/acrn-crashlog/usercrash/protocol.c b/tools/acrn-crashlog/usercrash/protocol.c index 9fd40a194..8aceeeac8 100644 --- a/tools/acrn-crashlog/usercrash/protocol.c +++ b/tools/acrn-crashlog/usercrash/protocol.c @@ -27,23 +27,28 @@ #include "protocol.h" #include "log_sys.h" +#define SUN_PATH_MAX \ + (sizeof(struct sockaddr_un) - offsetof(struct sockaddr_un, sun_path)) + /* Documented in header file. */ static int socket_make_sockaddr_un(const char *name, struct sockaddr_un *p_addr, socklen_t *alen) { - size_t namelen; + size_t name_len; + size_t socket_len; - memset(p_addr, 0, sizeof(struct sockaddr_un)); - namelen = strlen(name) + strlen(RESERVED_SOCKET_PREFIX); - /* unix_path_max appears to be missing on linux */ - if (namelen > sizeof(*p_addr)- - offsetof(struct sockaddr_un, sun_path) - 1) { + socket_len = strlen(RESERVED_SOCKET_PREFIX); + if (socket_len >= SUN_PATH_MAX) return -1; - } strcpy(p_addr->sun_path, RESERVED_SOCKET_PREFIX); + name_len = strlen(name); + if (name_len >= (SUN_PATH_MAX - socket_len)) + return -1; strcat(p_addr->sun_path, name); + p_addr->sun_family = AF_LOCAL; - *alen = namelen + offsetof(struct sockaddr_un, sun_path) + 1; + *alen = name_len + socket_len + + offsetof(struct sockaddr_un, sun_path) + 1; return 0; } @@ -100,8 +105,12 @@ static int socket_bind(int fd, const char *name) { struct sockaddr_un addr; socklen_t alen; + size_t name_len; addr.sun_family = AF_UNIX; + name_len = strlen(name); + if (name_len >= SUN_PATH_MAX) + return -1; strcpy(addr.sun_path, name); unlink(addr.sun_path); alen = strlen(addr.sun_path) + sizeof(addr.sun_family); diff --git a/tools/acrn-crashlog/usercrash/server.c b/tools/acrn-crashlog/usercrash/server.c index 7c7a38820..903bf02fe 100644 --- a/tools/acrn-crashlog/usercrash/server.c +++ b/tools/acrn-crashlog/usercrash/server.c @@ -185,8 +185,8 @@ static int get_usercrash(struct crash_node *crash) } next_usercrash = (next_usercrash + 1) % usercrash_count; crash->out_fd = result; - strncpy(crash->crash_path, file_name, FILE_PATH_LEN_MAX - 1); - + strncpy(crash->crash_path, file_name, FILE_PATH_LEN_MAX); + crash->crash_path[FILE_PATH_LEN_MAX - 1] = '\0'; return 0; } @@ -295,8 +295,8 @@ static void crash_request_cb(evutil_socket_t sockfd, short ev, void *arg) goto fail; } crash->pid = request.pid; - strncpy(crash->name, request.name, COMM_NAME_LEN - 1); - + strncpy(crash->name, request.name, COMM_NAME_LEN); + crash->name[COMM_NAME_LEN - 1] = '\0'; LOGI("received crash request from pid %d, name: %s\n", crash->pid, crash->name); @@ -452,7 +452,7 @@ int main(int argc, char *argv[]) RESERVED_SOCKET_PREFIX, SOCKET_NAME); if (chmod(socket_path, 0622) == -1) { LOGE("failed to change usercrashd_crash priority\n"); - exit(EXIT_FAILURE); + goto fail; } evutil_make_socket_nonblocking(crash_socket); @@ -460,7 +460,7 @@ int main(int argc, char *argv[]) base = event_base_new(); if (!base) { LOGE("failed to create event_base\n"); - exit(EXIT_FAILURE); + goto fail; } TAILQ_INIT(&queue_t); @@ -468,7 +468,7 @@ int main(int argc, char *argv[]) LEV_OPT_CLOSE_ON_FREE, -1, crash_socket); if (!listener) { LOGE("failed to create evconnlistener: %s\n", strerror(errno)); - exit(EXIT_FAILURE); + goto fail; } LOGI("usercrash_s successfully initialized\n"); @@ -477,5 +477,10 @@ int main(int argc, char *argv[]) evconnlistener_free(listener); event_base_free(base); + close(crash_socket); return 0; + +fail: + close(crash_socket); + exit(EXIT_FAILURE); }