From b570b363d94dc806d77b45980bc2236a45368976 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Sat, 8 Jul 2017 23:51:58 +0800 Subject: [PATCH] Cleanup file descriptors and add more info --- jni/daemon/daemon.c | 26 +++++++++----------------- jni/magisk.h | 1 - jni/magiskboot/main.c | 2 +- jni/magiskhide/hide_daemon.c | 1 + jni/magiskhide/magiskhide.c | 34 ++++++++-------------------------- jni/magiskhide/proc_monitor.c | 16 +++++++++++++++- jni/magiskpolicy | 2 +- jni/main.c | 2 +- jni/resetprop/resetprop.cpp | 2 +- jni/su | 2 +- jni/utils/utils.h | 2 +- jni/utils/xwrap.c | 4 ++-- 12 files changed, 41 insertions(+), 53 deletions(-) diff --git a/jni/daemon/daemon.c b/jni/daemon/daemon.c index 49f60a924..700771cf5 100644 --- a/jni/daemon/daemon.c +++ b/jni/daemon/daemon.c @@ -23,7 +23,6 @@ #include "magiskpolicy.h" pthread_t sepol_patch; -int null_fd; static void *request_handler(void *args) { // Setup the default error handler for threads @@ -89,17 +88,12 @@ static void *request_handler(void *args) { default: 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)) - PLOGE("fcntl FD_CLOEXEC"); - + int fd = xsocket(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0); memset(sun, 0, sizeof(*sun)); sun->sun_family = AF_LOCAL; memcpy(sun->sun_path, REQUESTOR_DAEMON_PATH, REQUESTOR_DAEMON_PATH_LEN); @@ -137,10 +131,11 @@ void start_daemon(int client) { xsetsid(); setcon("u:r:su:s0"); umask(022); - null_fd = xopen("/dev/null", O_RDWR | O_CLOEXEC); - xdup2(null_fd, STDIN_FILENO); - xdup2(null_fd, STDOUT_FILENO); - xdup2(null_fd, STDERR_FILENO); + int fd = xopen("/dev/null", O_RDWR | O_CLOEXEC); + xdup2(fd, STDIN_FILENO); + xdup2(fd, STDOUT_FILENO); + xdup2(fd, STDERR_FILENO); + close(fd); // Patch selinux with medium patch before we do anything load_policydb(SELINUX_POLICY); @@ -151,7 +146,7 @@ void start_daemon(int client) { pthread_create(&sepol_patch, NULL, large_sepol_patch, NULL); struct sockaddr_un sun; - int fd = setup_socket(&sun); + fd = setup_socket(&sun); xbind(fd, (struct sockaddr*) &sun, sizeof(sun)); xlisten(fd, 10); @@ -162,7 +157,7 @@ void start_daemon(int client) { // It should stay intact under any circumstances err_handler = do_nothing; - LOGI("Magisk v" xstr(MAGISK_VERSION) " daemon started\n"); + LOGI("Magisk v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") daemon started\n"); // Unlock all blocks for rw unlock_blocks(); @@ -178,9 +173,7 @@ void start_daemon(int client) { // 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); + *client = xaccept4(fd, NULL, NULL, SOCK_CLOEXEC); pthread_t thread; xpthread_create(&thread, NULL, request_handler, client); // Detach the thread, we will never join it @@ -192,7 +185,6 @@ void start_daemon(int client) { int connect_daemon() { struct sockaddr_un sun; int fd = setup_socket(&sun); - // LOGD("client: trying to connect socket\n"); if (connect(fd, (struct sockaddr*) &sun, sizeof(sun))) { /* If we cannot access the daemon, we start the daemon * since there is no clear entry point when the daemon should be started diff --git a/jni/magisk.h b/jni/magisk.h index 02e4e46aa..9931becb1 100644 --- a/jni/magisk.h +++ b/jni/magisk.h @@ -75,7 +75,6 @@ extern char *argv0; /* For changing process name */ extern char *applet[]; extern int (*applet_main[]) (int, char *[]); -extern int null_fd; // Multi-call entrypoints int magiskhide_main(int argc, char *argv[]); diff --git a/jni/magiskboot/main.c b/jni/magiskboot/main.c index ad10188a0..0653bae7e 100644 --- a/jni/magiskboot/main.c +++ b/jni/magiskboot/main.c @@ -58,7 +58,7 @@ static void usage(char *arg0) { } int main(int argc, char *argv[]) { - printf("MagiskBoot v" xstr(MAGISK_VERSION) " (by topjohnwu) - Boot Image Modification Tool\n\n"); + printf("MagiskBoot v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") (by topjohnwu) - Boot Image Modification Tool\n\n"); if (argc > 1 && strcmp(argv[1], "--cleanup") == 0) { cleanup(); diff --git a/jni/magiskhide/hide_daemon.c b/jni/magiskhide/hide_daemon.c index 9a1187eb5..96956f2ee 100644 --- a/jni/magiskhide/hide_daemon.c +++ b/jni/magiskhide/hide_daemon.c @@ -53,6 +53,7 @@ int hide_daemon() { // Set the process name strcpy(argv0, "magiskhide_daemon"); + LOGD("hide_daemon: listening for hide requests"); // When an error occurs, report its failure to main process err_handler = hide_daemon_err; diff --git a/jni/magiskhide/magiskhide.c b/jni/magiskhide/magiskhide.c index 2697060b3..8da13ce32 100644 --- a/jni/magiskhide/magiskhide.c +++ b/jni/magiskhide/magiskhide.c @@ -16,7 +16,6 @@ #include "daemon.h" #include "resetprop.h" -int sv[2], hide_pid = -1; struct vector *hide_list = NULL; int hideEnabled = 0; @@ -29,7 +28,7 @@ void kill_proc(int pid) { static void usage(char *arg0) { fprintf(stderr, - "MagiskHide v" xstr(MAGISK_VERSION) " (by topjohnwu) - Hide Magisk!\n\n" + "MagiskHide v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") (by topjohnwu) - Hide Magisk!\n\n" "%s [--options [arguments...] ]\n\n" "Options:\n" " --enable: Start the magiskhide daemon\n" @@ -63,19 +62,6 @@ void launch_magiskhide(int client) { hide_sensitive_props(); - if (socketpair(AF_LOCAL, SOCK_STREAM, 0, sv) == -1) - goto error; - - /* - * The setns system call do not support multithread processes - * We have to fork a new process, and communicate with sockets - */ - - if (hide_daemon()) - goto error; - - close(sv[1]); - // Initialize the mutex lock pthread_mutex_init(&hide_lock, NULL); pthread_mutex_init(&file_lock, NULL); @@ -87,8 +73,10 @@ void launch_magiskhide(int client) { // Add SafetyNet by default add_list(strdup("com.google.android.gms.unstable")); - write_int(client, DAEMON_SUCCESS); - close(client); + if (client > 0) { + write_int(client, DAEMON_SUCCESS); + close(client); + } // Get thread reference proc_monitor_thread = pthread_self(); @@ -98,15 +86,9 @@ void launch_magiskhide(int client) { error: hideEnabled = 0; - write_int(client, DAEMON_ERROR); - close(client); - if (hide_pid != -1) { - int kill = -1; - // Kill hide daemon - write(sv[0], &kill, sizeof(kill)); - close(sv[0]); - waitpid(hide_pid, NULL, 0); - hide_pid = -1; + if (client > 0) { + write_int(client, DAEMON_ERROR); + close(client); } return; } diff --git a/jni/magiskhide/proc_monitor.c b/jni/magiskhide/proc_monitor.c index f8fbeafd0..e628d935e 100644 --- a/jni/magiskhide/proc_monitor.c +++ b/jni/magiskhide/proc_monitor.c @@ -22,6 +22,8 @@ static char init_ns[32], zygote_ns[2][32]; static int log_pid, log_fd; static char *buffer; +int sv[2], hide_pid = -1; + // Workaround for the lack of pthread_cancel static void quit_pthread(int sig) { err_handler = do_nothing; @@ -103,10 +105,22 @@ void proc_monitor() { LOGI("proc_monitor: zygote ns=%s\n", zygote_ns[0]); break; case 2: - LOGI("proc_monitor: zygote (32-bit) ns=%s (64-bit) ns=%s\n", zygote_ns[0], zygote_ns[1]); + LOGI("proc_monitor: zygote ns=%s zygote64 ns=%s\n", zygote_ns[0], zygote_ns[1]); break; } + if (socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, sv) == -1) + quit_pthread(SIGUSR1); + + /* + * The setns system call do not support multithread processes + * We have to fork a new process, and communicate with sockets + */ + if (hide_daemon()) + quit_pthread(SIGUSR1); + + close(sv[1]); + while (1) { // Clear previous buffer system("logcat -b events -c"); diff --git a/jni/magiskpolicy b/jni/magiskpolicy index 193d160be..5529dab84 160000 --- a/jni/magiskpolicy +++ b/jni/magiskpolicy @@ -1 +1 @@ -Subproject commit 193d160bed24f75d2dd440063bfc00d7920cf789 +Subproject commit 5529dab84e44856679406edd6560d26c61e1e715 diff --git a/jni/main.c b/jni/main.c index 8c97ea5e5..7d7c5f771 100644 --- a/jni/main.c +++ b/jni/main.c @@ -22,7 +22,7 @@ __thread void (*err_handler)(void); static void usage() { fprintf(stderr, - "Magisk v" xstr(MAGISK_VERSION) " multi-call binary\n" + "Magisk v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") (by topjohnwu) multi-call binary\n" "\n" "Usage: %s [applet [arguments]...]\n" " or: %s --install [SOURCE] \n" diff --git a/jni/resetprop/resetprop.cpp b/jni/resetprop/resetprop.cpp index 966730890..40c278b86 100644 --- a/jni/resetprop/resetprop.cpp +++ b/jni/resetprop/resetprop.cpp @@ -108,7 +108,7 @@ static bool is_legal_property_name(const char* name, size_t namelen) { static int usage(char* arg0) { fprintf(stderr, - "resetprop v" xstr(MAGISK_VERSION) " (by topjohnwu & nkk71) - System Props Modification Tool\n\n" + "resetprop v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") (by topjohnwu & nkk71) - System Props Modification Tool\n\n" "Usage: %s [options] [args...]\n" "%s : Set property entry with \n" "%s --file : Load props from \n" diff --git a/jni/su b/jni/su index 91ea68016..e2821025e 160000 --- a/jni/su +++ b/jni/su @@ -1 +1 @@ -Subproject commit 91ea6801679ddd0b05c893065520acb29f713e6f +Subproject commit e2821025ef7348085958ac8f76e51b08b0e9647f diff --git a/jni/utils/utils.h b/jni/utils/utils.h index 19ed33e3d..ad95b8c69 100644 --- a/jni/utils/utils.h +++ b/jni/utils/utils.h @@ -40,7 +40,7 @@ int xsocket(int domain, int type, int protocol); int xbind(int sockfd, const struct sockaddr *addr, socklen_t addrlen); int xconnect(int sockfd, const struct sockaddr *addr, socklen_t addrlen); int xlisten(int sockfd, int backlog); -int xaccept(int sockfd, struct sockaddr *addr, socklen_t *addrlen); +int xaccept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags); void *xmalloc(size_t size); void *xcalloc(size_t nmemb, size_t size); void *xrealloc(void *ptr, size_t size); diff --git a/jni/utils/xwrap.c b/jni/utils/xwrap.c index 2764c89d8..cd5e6e7d2 100644 --- a/jni/utils/xwrap.c +++ b/jni/utils/xwrap.c @@ -155,8 +155,8 @@ int xlisten(int sockfd, int backlog) { return ret; } -int xaccept(int sockfd, struct sockaddr *addr, socklen_t *addrlen) { - int fd = accept(sockfd, addr, addrlen); +int xaccept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags) { + int fd = accept4(sockfd, addr, addrlen, flags); if (fd == -1) { PLOGE("accept"); }