From b91eba438a2f81bbbc4bf5744bb1a15addb4d25c Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 30 Aug 2018 13:38:39 +0200 Subject: [PATCH] Use parsed command line arguments instead of raw string values Change-Id: I28c9479c2d6cb5d754e7ff3ee37f8cab34235456 --- src/crash-manager/crash-manager.c | 100 +++++++++++++++++++------------------- src/crash-stack/crash-stack.c | 5 +- src/shared/util.c | 8 +-- src/shared/util.h | 4 +- 4 files changed, 57 insertions(+), 60 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index a778b97..3603907 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -113,12 +113,14 @@ static int report_type; /* Paths and variables */ static struct crash_info { - char *pid_info; - char *tid_info; - char *sig_info; + pid_t pid_info; + pid_t tid_info; + int uid_info; + int gid_info; + int sig_info; char cmd_line[PATH_MAX]; char cmd_path[PATH_MAX]; - char time_info[80]; + time_t time_info; char temp_dir[PATH_MAX]; char name[FILENAME_MAX]; char result_path[PATH_MAX]; @@ -437,7 +439,7 @@ static int set_prstatus() int ret; char prstatus_name[NAME_MAX+1]; - ret = snprintf(prstatus_name, NAME_MAX, "/%s.prstatus", crash_info.pid_info); + ret = snprintf(prstatus_name, NAME_MAX, "/%d.prstatus", crash_info.pid_info); if (ret < 0) { _E("Failed to snprintf for prstatus path"); goto close_fd; @@ -485,29 +487,18 @@ static int set_crash_info(int argc, char *argv[]) { int ret; char *temp_dir_ret = NULL; - time_t time_val; + char date[80]; struct tm loc_tm; - crash_info.pid_info = argv[1]; - crash_info.sig_info = argv[4]; - if (argc > 6) { - crash_info.tid_info = strdup(argv[6]); - if (crash_info.tid_info == NULL) { - _E("strdup error: %m"); - return -1; - } - } else { - crash_info.tid_info = NULL; - int pid = atoi(crash_info.pid_info); - int tid = find_crash_tid(pid); - if (tid < 0) { + crash_info.pid_info = strtol(argv[1], NULL, 10); + crash_info.sig_info = atoi(argv[4]); + if (argc > 6) + crash_info.tid_info = strtol(argv[6], NULL, 10); + else { + crash_info.tid_info = find_crash_tid(crash_info.pid_info); + if (crash_info.tid_info < 0) { _I("TID not found"); - tid = pid; - } - - if (asprintf(&crash_info.tid_info, "%d", tid) == -1) { - _E("asprintf error: %m"); - return -1; + crash_info.tid_info = crash_info.pid_info; } } @@ -517,10 +508,9 @@ static int set_crash_info(int argc, char *argv[]) return -1; } - time_val = atoll(argv[5]); - localtime_r(&time_val, &loc_tm); - strftime(crash_info.time_info, sizeof(crash_info.time_info), - "%Y%m%d%H%M%S", &loc_tm); + crash_info.time_info = strtol(argv[5], NULL, 10); + localtime_r(&crash_info.time_info, &loc_tm); + strftime(date, sizeof(date), "%Y%m%d%H%M%S", &loc_tm); ret = snprintf(crash_info.temp_dir, sizeof(crash_info.temp_dir), "%s/crash.XXXXXX", crash_temp_path); @@ -534,10 +524,10 @@ static int set_crash_info(int argc, char *argv[]) return -1; } - ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%s_%s", + ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%d_%s", basename(crash_info.cmd_line), crash_info.pid_info, - crash_info.time_info); + date); if (ret < 0) { _E("Failed to snprintf for name"); goto rm_temp; @@ -592,7 +582,7 @@ static int set_crash_info(int argc, char *argv[]) #ifdef SYS_ASSERT ret = snprintf(crash_info.sysassert_cs_path, sizeof(crash_info.sysassert_cs_path), - "/tmp/crash_stack/%s_%s.info", + "/tmp/crash_stack/%s_%d.info", basename(crash_info.cmd_line), crash_info.pid_info); if (ret < 0) { _E("Failed to snprintf for sys-assert callstack path"); @@ -719,7 +709,7 @@ static void copy_maps() { char maps_path[PATH_MAX]; char temp_maps_path[PATH_MAX]; - snprintf(maps_path, sizeof(maps_path), "/proc/%s/maps", + snprintf(maps_path, sizeof(maps_path), "/proc/%d/maps", crash_info.pid_info); snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s", @@ -751,27 +741,36 @@ static void save_so_info() get_and_save_so_info(maps_path, so_info_path); } -static int execute_minicoredump(int argc, char *argv[]) +static int execute_minicoredump() { +#define SNPRINTF_OR_EXIT(name, format) if (snprintf(name##_str, sizeof(name##_str), format, crash_info.name##_info) < 0) goto out; + char *coredump_name = NULL; char *prstatus_fd_str = NULL; - int ret = 0; + int ret = -1; if (asprintf(&coredump_name, "%s.coredump", crash_info.name) == -1 || asprintf(&prstatus_fd_str, "%d", crash_info.prstatus_fd) == -1) { _E("Unable to allocate memory"); - ret = -1; goto out; } + char pid_str[11], uid_str[11], gid_str[11], sig_str[11], time_str[11]; + + SNPRINTF_OR_EXIT(pid, "%d") + SNPRINTF_OR_EXIT(uid, "%d") + SNPRINTF_OR_EXIT(gid, "%d") + SNPRINTF_OR_EXIT(sig, "%d") + SNPRINTF_OR_EXIT(time, "%ld") + /* Execute minicoredumper */ char *args[] = { MINICOREDUMPER_BIN_PATH, // minicoredumper filename path - crash_info.pid_info, // %p - pid - argv[2], // %u - UID - argv[3], // %g - GID - crash_info.sig_info, // %s - number of signal - argv[5], // %t - time of dump + pid_str, // %p - pid + uid_str, // %u - UID + gid_str, // %g - GID + sig_str, // %s - number of signal + time_str, // %t - time of dump "localhost", // %h - hostname "core", // %e - exe name (need for result filename) MINICOREDUMPER_CONFIG_PATH, // config file @@ -791,6 +790,7 @@ static int execute_minicoredump(int argc, char *argv[]) args[12], args[13], args[14]); run_command_timeout(args[0], args, NULL, MINICOREDUMPER_TIMEOUT); + ret = 0; /* Minicoredumper must be executed to dump at least PRSTATUS for other tools, coredump, however, might have been disabled. */ @@ -818,16 +818,17 @@ out: free(prstatus_fd_str); return ret; +#undef SNPRINTF_OR_EXIT } -static void execute_crash_stack(int argc, char *argv[]) +static void execute_crash_stack() { int ret; char command[PATH_MAX]; /* Execute crash-stack */ ret = snprintf(command, sizeof(command), - "%s --pid %s --tid %s --sig %s --prstatus_fd %d > %s", + "%s --pid %d --tid %d --sig %d --prstatus_fd %d > %s", CRASH_STACK_PATH, crash_info.pid_info, crash_info.tid_info, @@ -844,11 +845,11 @@ static void execute_crash_stack(int argc, char *argv[]) system_command(command); } -static int execute_crash_modules(int argc, char *argv[]) +static int execute_crash_modules() { _D("Execute crash module: "); - if (execute_minicoredump(argc, argv) < 0) { + if (execute_minicoredump() < 0) { _E("Failed to run minicoredumper - can not continue"); return -1; } @@ -859,7 +860,7 @@ static int execute_crash_modules(int argc, char *argv[]) if (crash_info.have_sysassert_report) return -1; #endif - execute_crash_stack(argc, argv); + execute_crash_stack(); return 0; } @@ -1242,7 +1243,7 @@ int main(int argc, char *argv[]) copy_maps(); } /* Exec crash modules */ - if (execute_crash_modules(argc, argv) < 0) { + if (execute_crash_modules() < 0) { res = EXIT_FAILURE; goto exit; } @@ -1269,8 +1270,8 @@ int main(int argc, char *argv[]) struct NotifyParams notify_params = { .prstatus_fd = crash_info.prstatus_fd, - .pid = strtol(crash_info.pid_info, NULL, 10), - .tid = strtol(crash_info.tid_info, NULL, 10), + .pid = crash_info.pid_info, + .tid = crash_info.tid_info, .cmd_name = basename(crash_info.cmd_line), .cmd_path = crash_info.cmd_path, .report_path = crash_info.result_path, @@ -1282,7 +1283,6 @@ int main(int argc, char *argv[]) exit: close(crash_info.prstatus_fd); - free(crash_info.tid_info); free(crash_temp_path); free(crash_root_path); free(crash_crash_path); diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index 2f65b7c..35516eb 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -259,11 +259,8 @@ void callstack_destructor(Callstack *callstack) static void __crash_stack_print_exe(FILE* outputfile, pid_t pid) { char file_path[PATH_MAX]; - char pid_str[11]; - snprintf(pid_str, sizeof(pid_str), "%d", pid); - - if (get_exe_path(pid_str, file_path, sizeof(file_path)) == 0) + if (get_exe_path(pid, file_path, sizeof(file_path)) == 0) snprintf(file_path, sizeof(file_path), ""); fprintf(outputfile, "Executable File Path: %s\n", file_path); diff --git a/src/shared/util.c b/src/shared/util.c index 2eb4399..97f3499 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -819,12 +819,12 @@ int find_crash_tid(int pid) return -1; } -int get_cmd_line(char* pid, char *cmd, size_t len) +int get_cmd_line(pid_t pid, char *cmd, size_t len) { char cmdline_path[PATH_MAX]; snprintf(cmdline_path, sizeof(cmdline_path), - "/proc/%s/cmdline", pid); + "/proc/%d/cmdline", pid); int fd = open(cmdline_path, O_RDONLY); if (fd < 0) { @@ -845,12 +845,12 @@ int get_cmd_line(char* pid, char *cmd, size_t len) return 1; } -int get_exe_path(char* pid, char *path, size_t len) +int get_exe_path(pid_t pid, char *path, size_t len) { char exe_link[PATH_MAX]; snprintf(exe_link, sizeof(exe_link), - "/proc/%s/exe", pid); + "/proc/%d/exe", pid); ssize_t ret = readlink(exe_link, path, len - 1); diff --git a/src/shared/util.h b/src/shared/util.h index 0cb35f2..5d40d91 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -63,9 +63,9 @@ int log_kmsg(char *fmt, ...); int find_crash_tid(int pid); -int get_exe_path(char* pid, char *path, size_t len); +int get_exe_path(pid_t pid, char *path, size_t len); -int get_cmd_line(char* pid, char *cmd, size_t len); +int get_cmd_line(pid_t pid, char *cmd, size_t len); /** * @} */ -- 2.7.4