From 4d267aa719bf3fa2f46475dc6f99be183d190320 Mon Sep 17 00:00:00 2001 From: ThePedroo Date: Fri, 4 Oct 2024 03:24:37 -0300 Subject: [PATCH] fix: sending 32-bit int instead of 8-bit; fix: fd leak This commit fixes the issue where a 32-bit (int) number was sent instead of a 8-bit (uint8_t) number. Also fixes a fd leak when connecting to the companion. --- zygiskd/src/companion.c | 30 ++++++++---------------------- zygiskd/src/utils.c | 4 ++-- zygiskd/src/utils.h | 5 ++--- zygiskd/src/zygiskd.c | 24 ++++++++++++++++++------ 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/zygiskd/src/companion.c b/zygiskd/src/companion.c index 9f12720..87e499f 100644 --- a/zygiskd/src/companion.c +++ b/zygiskd/src/companion.c @@ -42,28 +42,16 @@ void *entry_thread(void *arg) { int fd = args->fd; zygisk_companion_entry_func module_entry = args->entry; - struct stat st0; - fstat(fd, &st0); - LOGI("New companion thread (inside the thread!).\n - Client fd: %d\n", fd); module_entry(fd); - /* TODO: Is this even necessary? */ - struct stat st1; - if (fstat(fd, &st1) != -1) { - if (st0.st_dev != st1.st_dev || st0.st_ino != st1.st_ino) { - close(fd); - - LOGI("Client fd has been replaced. Bye!\n"); - } - } + LOGI("Companion thread has been terminated.\n"); + close(fd); free(args); pthread_exit(NULL); - - return NULL; } void entry(int fd) { @@ -74,20 +62,18 @@ void entry(int fd) { if (ret == -1) { LOGE("Failed to read module name\n"); - uint8_t response = 2; - write(fd, &response, sizeof(response)); + write_uint8_t(fd, 2); exit(0); } LOGI(" - Module name: `%.*s`\n", (int)ret, name); - int library_fd = gread_fd(fd); + int library_fd = read_fd(fd); if (library_fd == -1) { LOGE("Failed to receive library fd\n"); - uint8_t response = 2; - write(fd, &response, sizeof(response)); + write_uint8_t(fd, 2); exit(0); } @@ -100,11 +86,11 @@ void entry(int fd) { if (module_entry == NULL) { LOGI("No companion module entry for module: %.*s\n", (int)ret, name); - write_int(fd, 0); + write_uint8_t(fd, 0); exit(0); } else { - write_int(fd, 1); + write_uint8_t(fd, 1); } while (1) { @@ -119,7 +105,7 @@ void entry(int fd) { struct companion_module_thread_args *args = malloc(sizeof(struct companion_module_thread_args)); args->entry = module_entry; - if ((args->fd = gread_fd(fd)) == -1) { + if ((args->fd = read_fd(fd)) == -1) { LOGE("Failed to receive client fd\n"); exit(0); diff --git a/zygiskd/src/utils.c b/zygiskd/src/utils.c index 97b3832..b695d29 100644 --- a/zygiskd/src/utils.c +++ b/zygiskd/src/utils.c @@ -192,7 +192,7 @@ int unix_listener_from_path(char *restrict path) { return socket_fd; } -ssize_t gwrite_fd(int fd, int sendfd) { +ssize_t write_fd(int fd, int sendfd) { char cmsgbuf[CMSG_SPACE(sizeof(int))]; char buf[1] = { 0 }; @@ -225,7 +225,7 @@ ssize_t gwrite_fd(int fd, int sendfd) { return ret; } -int gread_fd(int fd) { +int read_fd(int fd) { char cmsgbuf[CMSG_SPACE(sizeof(int))]; char buf[1] = { 0 }; diff --git a/zygiskd/src/utils.h b/zygiskd/src/utils.h index 3da8c6d..4c13406 100644 --- a/zygiskd/src/utils.h +++ b/zygiskd/src/utils.h @@ -36,9 +36,8 @@ int chcon(const char *path, const char *restrict context); int unix_listener_from_path(char *path); -ssize_t gwrite_fd(int fd, int sendfd); - -int gread_fd(int fd); +ssize_t write_fd(int fd, int sendfd); +int read_fd(int fd); write_func_def(int); read_func_def(int); diff --git a/zygiskd/src/zygiskd.c b/zygiskd/src/zygiskd.c index ec7a00c..6cf4796 100644 --- a/zygiskd/src/zygiskd.c +++ b/zygiskd/src/zygiskd.c @@ -278,7 +278,7 @@ static int spawn_companion(char *restrict argv[], char *restrict name, int lib_f return -1; } - if (gwrite_fd(daemon_fd, lib_fd) == -1) { + if (write_fd(daemon_fd, lib_fd) == -1) { LOGE("Failed sending library fd.\n"); close(daemon_fd); @@ -694,7 +694,7 @@ void zygiskd_start(char *restrict argv[]) { for (size_t i = 0; i < clen; i++) { if (write_string(client_fd, context.modules[i].name) == -1) break; - if (gwrite_fd(client_fd, context.modules[i].lib_fd) == -1) break; + if (write_fd(client_fd, context.modules[i].lib_fd) == -1) break; } LOGI("ZD++ ReadModules\n"); @@ -711,6 +711,8 @@ void zygiskd_start(char *restrict argv[]) { struct Module *module = &context.modules[index]; if (module->companion != -1) { + LOGI(" Polling companion for module \"%s\"\n", module->name); + if (!check_unix_socket(module->companion, false)) { LOGE(" Poll companion for module \"%s\" crashed\n", module->name); @@ -733,22 +735,32 @@ void zygiskd_start(char *restrict argv[]) { } } + /* + INFO: Companion already exists or was created. In any way, + it should be in the while loop to receive fds now, + so just sending the file descriptor of the client is + safe. + */ if (module->companion != -1) { - if (gwrite_fd(module->companion, client_fd) == -1) { + LOGI(" Sending companion fd socket of module \"%s\"\n", module->name); + + if (write_fd(module->companion, client_fd) == -1) { LOGE("Failed to send companion fd socket of module \"%s\"\n", module->name); - ret = write_int(client_fd, 0); + ret = write_uint8_t(client_fd, 0); ASSURE_SIZE_WRITE_BREAK("RequestCompanionSocket", "response", ret, sizeof(int)); close(module->companion); module->companion = -1; + /* INFO: RequestCompanionSocket by defailt doesn't close the client_fd */ close(client_fd); } } else { - ret = write_int(client_fd, 0); + ret = write_uint8_t(client_fd, 0); ASSURE_SIZE_WRITE_BREAK("RequestCompanionSocket", "response", ret, sizeof(int)); + /* INFO: RequestCompanionSocket by defailt doesn't close the client_fd */ close(client_fd); } @@ -781,7 +793,7 @@ void zygiskd_start(char *restrict argv[]) { break; } - if (gwrite_fd(client_fd, fd) == -1) { + if (write_fd(client_fd, fd) == -1) { LOGE("Failed sending module directory \"%s\" fd: %s\n", module_dir, strerror(errno)); close(fd);