From ea8cd98361dba2ab1366213de26498f39e2eaab5 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Mon, 8 May 2017 03:11:14 +0800 Subject: [PATCH] Cleanup file descriptors --- jni/daemon/daemon.c | 31 +++++++++++++++++++------------ jni/daemon/daemon.h | 2 +- jni/daemon/log_monitor.c | 11 ++++++++--- jni/magiskhide/hide_daemon.c | 1 - jni/magiskhide/proc_monitor.c | 15 +++++++++++---- jni/su | 2 +- jni/utils/misc.c | 2 ++ jni/utils/utils.h | 1 + jni/utils/xwrap.c | 8 ++++++++ 9 files changed, 51 insertions(+), 22 deletions(-) diff --git a/jni/daemon/daemon.c b/jni/daemon/daemon.c index 98269b3e4..f3660c5dc 100644 --- a/jni/daemon/daemon.c +++ b/jni/daemon/daemon.c @@ -87,17 +87,18 @@ static void *request_handler(void *args) { late_start(client); break; default: - close(client); + break; } + // Just in case + close(client); return NULL; } /* Setup the address and return socket fd */ static int setup_socket(struct sockaddr_un *sun) { int fd = xsocket(AF_LOCAL, SOCK_STREAM, 0); - if (fcntl(fd, F_SETFD, FD_CLOEXEC)) { + if (fcntl(fd, F_SETFD, FD_CLOEXEC)) PLOGE("fcntl FD_CLOEXEC"); - } memset(sun, 0, sizeof(*sun)); sun->sun_family = AF_LOCAL; @@ -115,12 +116,13 @@ static void *large_sepol_patch(void *args) { return NULL; } -void start_daemon() { +void start_daemon(int client) { // Launch the daemon, create new session, set proper context if (getuid() != UID_ROOT || getgid() != UID_ROOT) { fprintf(stderr, "Starting daemon requires root: %s\n", strerror(errno)); PLOGE("start daemon"); } + switch (fork()) { case -1: PLOGE("fork"); @@ -129,10 +131,13 @@ void start_daemon() { default: return; } + + // First close the client, it's useless for us + close(client); xsetsid(); setcon("u:r:su:s0"); umask(022); - null_fd = xopen("/dev/null", O_RDWR); + null_fd = xopen("/dev/null", O_RDWR | O_CLOEXEC); xdup2(null_fd, STDIN_FILENO); xdup2(null_fd, STDOUT_FILENO); xdup2(null_fd, STDERR_FILENO); @@ -142,12 +147,12 @@ void start_daemon() { sepol_med_rules(); dump_policydb(SELINUX_LOAD); - // Continue the larger patch in another thread, we will need to join later + // Continue the larger patch in another thread, we will join later pthread_create(&sepol_patch, NULL, large_sepol_patch, NULL); struct sockaddr_un sun; int fd = setup_socket(&sun); - + xbind(fd, (struct sockaddr*) &sun, sizeof(sun)); xlisten(fd, 10); @@ -168,15 +173,17 @@ void start_daemon() { // Setup links under /sbin xmount(NULL, "/", NULL, MS_REMOUNT, NULL); create_links(NULL, "/sbin"); - chmod("/sbin", 0755); - mkdir("/magisk", 0755); - chmod("/magisk", 0755); + xchmod("/sbin", 0755); + xmkdir("/magisk", 0755); + xchmod("/magisk", 0755); xmount(NULL, "/", NULL, MS_REMOUNT | MS_RDONLY, NULL); - // Loop forever to listen to requests + // Loop forever to listen for requests while(1) { int *client = xmalloc(sizeof(int)); *client = xaccept(fd, NULL, NULL); + // Just in case, set to close on exec + fcntl(*client, F_SETFD, FD_CLOEXEC); pthread_t thread; xpthread_create(&thread, NULL, request_handler, client); // Detach the thread, we will never join it @@ -194,7 +201,7 @@ int connect_daemon() { * since there is no clear entry point when the daemon should be started */ LOGD("client: connect fail, try launching new daemon process\n"); - start_daemon(); + start_daemon(fd); do { // Wait for 10ms usleep(10); diff --git a/jni/daemon/daemon.h b/jni/daemon/daemon.h index 5288d6aed..c2f1cce96 100644 --- a/jni/daemon/daemon.h +++ b/jni/daemon/daemon.h @@ -37,7 +37,7 @@ typedef enum { // daemon.c -void start_daemon(); +void start_daemon(int client); int connect_daemon(); // socket_trans.c diff --git a/jni/daemon/log_monitor.c b/jni/daemon/log_monitor.c index 063a0dc96..475f6208a 100644 --- a/jni/daemon/log_monitor.c +++ b/jni/daemon/log_monitor.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "magisk.h" #include "utils.h" @@ -17,18 +18,22 @@ static void *logger_thread(void *args) { // Setup error handler err_handler = exit_thread; - char buffer[PATH_MAX]; + char *buffer = xmalloc(PATH_MAX); rename(LOGFILE, LASTLOG); - FILE *logfile = xfopen(LOGFILE, "w"); + FILE *logfile = xfdopen(xopen(LOGFILE, O_WRONLY | O_CREAT | O_CLOEXEC | O_TRUNC, 0644), "w"); // Disable buffering setbuf(logfile, NULL); // Start logcat char *const command[] = { "logcat", "-s", "Magisk", "-v", "time", NULL }; int fd; run_command(&fd, "/system/bin/logcat", command); - while (fdgets(buffer, sizeof(buffer), fd)) { + while (fdgets(buffer, PATH_MAX, fd)) { fprintf(logfile, "%s", buffer); } + // Should never be here, but well... + free(buffer); + close(fd); + fclose(logfile); return NULL; } diff --git a/jni/magiskhide/hide_daemon.c b/jni/magiskhide/hide_daemon.c index d19e55b9b..82f1cccb0 100644 --- a/jni/magiskhide/hide_daemon.c +++ b/jni/magiskhide/hide_daemon.c @@ -57,7 +57,6 @@ int hide_daemon() { err_handler = hide_daemon_err; int fd; - FILE *fp; char buffer[4096], cache_block[256], *line; struct vector mount_list; diff --git a/jni/magiskhide/proc_monitor.c b/jni/magiskhide/proc_monitor.c index bb846784f..25c0f6afe 100644 --- a/jni/magiskhide/proc_monitor.c +++ b/jni/magiskhide/proc_monitor.c @@ -20,6 +20,7 @@ static int zygote_num = 0; static char init_ns[32], zygote_ns[2][32]; static int log_pid = 0, log_fd; +static char *buffer; static void read_namespace(const int pid, char* target, const size_t size) { char path[32]; @@ -31,8 +32,9 @@ static void read_namespace(const int pid, char* target, const size_t size) { static void quit_pthread(int sig) { LOGD("proc_monitor: running cleanup\n"); destroy_list(); + free(buffer); hideEnabled = 0; - // Kill the logging if possible + // Kill the logging if needed if (log_pid) { kill(log_pid, SIGTERM); waitpid(log_pid, NULL, 0); @@ -45,6 +47,7 @@ static void quit_pthread(int sig) { close(sv[0]); waitpid(hide_pid, NULL, 0); pthread_mutex_destroy(&hide_lock); + pthread_mutex_destroy(&file_lock); LOGD("proc_monitor: terminating...\n"); pthread_exit(NULL); } @@ -65,12 +68,16 @@ static void proc_monitor_err() { void proc_monitor() { // Register the cancel signal - signal(SIGUSR1, quit_pthread); + struct sigaction act; + memset(&act, 0, sizeof(act)); + act.sa_handler = quit_pthread; + sigaction(SIGPIPE, &act, NULL); + // The error handler should stop magiskhide services err_handler = proc_monitor_err; int pid; - char buffer[4096]; + buffer = xmalloc(PATH_MAX); // Get the mount namespace of init read_namespace(1, init_ns, 32); @@ -98,7 +105,7 @@ void proc_monitor() { char *const command[] = { "logcat", "-b", "events", "-v", "raw", "-s", "am_proc_start", NULL }; log_pid = run_command(&log_fd, "/system/bin/logcat", command); - while(fdgets(buffer, sizeof(buffer), log_fd)) { + while(fdgets(buffer, PATH_MAX, log_fd)) { int ret, comma = 0; char *pos = buffer, *line, processName[256]; diff --git a/jni/su b/jni/su index 9a1043574..5a41e8882 160000 --- a/jni/su +++ b/jni/su @@ -1 +1 @@ -Subproject commit 9a1043574a0109e98872bc6e3fe2dcd2f406849a +Subproject commit 5a41e8882a8762e6dc1815e49002776b634a4ea1 diff --git a/jni/utils/misc.c b/jni/utils/misc.c index 73898f5c7..fcb91019e 100644 --- a/jni/utils/misc.c +++ b/jni/utils/misc.c @@ -221,6 +221,8 @@ int run_command(int *fd, const char *path, char *const argv[]) { if (socketpair(AF_LOCAL, SOCK_STREAM, 0, sv) == -1) return -1; // We use sv[0], give them sv[1] for communication + if (fcntl(sv[1], F_SETFD, FD_CLOEXEC)) + PLOGE("fcntl FD_CLOEXEC"); *fd = sv[1]; } diff --git a/jni/utils/utils.h b/jni/utils/utils.h index c407b4869..4bae70012 100644 --- a/jni/utils/utils.h +++ b/jni/utils/utils.h @@ -23,6 +23,7 @@ extern int quit_signals[]; // xwrap.c FILE *xfopen(const char *pathname, const char *mode); +FILE *xfdopen(int fd, const char *mode); #define GET_MACRO(_1, _2, _3, NAME, ...) NAME #define xopen(...) GET_MACRO(__VA_ARGS__, xopen3, xopen2)(__VA_ARGS__) int xopen2(const char *pathname, int flags); diff --git a/jni/utils/xwrap.c b/jni/utils/xwrap.c index 33e8b5217..5cd1bb794 100644 --- a/jni/utils/xwrap.c +++ b/jni/utils/xwrap.c @@ -32,6 +32,14 @@ FILE *xfopen(const char *pathname, const char *mode) { return fp; } +FILE *xfdopen(int fd, const char *mode) { + FILE *fp = fdopen(fd, mode); + if (fp == NULL) { + PLOGE("fopen"); + } + return fp; +} + int xopen2(const char *pathname, int flags) { int fd = open(pathname, flags); if (fd < 0) {