improve: code robustness

This commit improves the robustness of the code by removing debug logs, fixing a memory leak, and adding missing error handling.
This commit is contained in:
ThePedroo
2024-10-06 23:49:20 -03:00
parent 980bf2ab4c
commit ab7de49e4c
5 changed files with 105 additions and 105 deletions

View File

@@ -42,12 +42,8 @@ void *entry_thread(void *arg) {
int fd = args->fd;
zygisk_companion_entry_func module_entry = args->entry;
LOGI("New companion thread (inside the thread!).\n - Client fd: %d\n", fd);
module_entry(fd);
LOGI("Companion thread has been terminated.\n");
close(fd);
free(args);
@@ -63,7 +59,8 @@ void entry(int fd) {
if (ret == -1) {
LOGE("Failed to read module name\n");
write_uint8_t(fd, 2);
ret = write_uint8_t(fd, 2);
ASSURE_SIZE_WRITE("ZygiskdCompanion", "name", ret, sizeof(uint8_t));
exit(0);
}
@@ -74,7 +71,8 @@ void entry(int fd) {
if (library_fd == -1) {
LOGE("Failed to receive library fd\n");
write_uint8_t(fd, 2);
ret = write_uint8_t(fd, 2);
ASSURE_SIZE_WRITE("ZygiskdCompanion", "library_fd", ret, sizeof(uint8_t));
exit(0);
}
@@ -87,11 +85,13 @@ void entry(int fd) {
if (module_entry == NULL) {
LOGI("No companion module entry for module: %.*s\n", (int)ret, name);
write_uint8_t(fd, 0);
ret = write_uint8_t(fd, 0);
ASSURE_SIZE_WRITE("ZygiskdCompanion", "module_entry", ret, sizeof(uint8_t));
exit(0);
} else {
write_uint8_t(fd, 1);
ret = write_uint8_t(fd, 1);
ASSURE_SIZE_WRITE("ZygiskdCompanion", "module_entry", ret, sizeof(uint8_t));
}
while (1) {
@@ -103,18 +103,27 @@ void entry(int fd) {
break;
}
struct companion_module_thread_args *args = malloc(sizeof(struct companion_module_thread_args));
args->entry = module_entry;
if ((args->fd = read_fd(fd)) == -1) {
int fd = read_fd(fd);
if (fd == -1) {
LOGE("Failed to receive client fd\n");
exit(0);
}
struct companion_module_thread_args *args = malloc(sizeof(struct companion_module_thread_args));
if (args == NULL) {
LOGE("Failed to allocate memory for thread args\n");
exit(0);
}
args->fd = fd;
args->entry = module_entry;
LOGI("New companion request.\n - Module name: %.*s\n - Client fd: %d\n", (int)ret, name, args->fd);
write_uint8_t(args->fd, 1);
ret = write_uint8_t(args->fd, 1);
ASSURE_SIZE_WRITE("ZygiskdCompanion", "client_fd", ret, sizeof(uint8_t));
pthread_t thread;
pthread_create(&thread, NULL, entry_thread, args);

View File

@@ -76,7 +76,7 @@ bool _apatch_get_package_config(struct packages_config *restrict config) {
return false;
}
char line[1048 * 2];
char line[1048];
/* INFO: Skip the CSV header */
if (fgets(line, sizeof(line), fp) == NULL) {
LOGE("Failed to read APatch's package_config header: %s\n", strerror(errno));
@@ -166,7 +166,9 @@ bool apatch_uid_should_umount(uid_t uid) {
bool apatch_uid_is_manager(uid_t uid) {
struct stat s;
if (stat("/data/user_de/0/me.bmax.apatch", &s) == -1) {
if (errno != ENOENT) LOGE("Failed to stat APatch manager data directory: %s\n", strerror(errno));
if (errno != ENOENT) {
LOGE("Failed to stat APatch manager data directory: %s\n", strerror(errno));
}
errno = 0;
return false;

View File

@@ -53,7 +53,9 @@ bool ksu_uid_should_umount(uid_t uid) {
bool ksu_uid_is_manager(uid_t uid) {
struct stat s;
if (stat("/data/user_de/0/me.weishu.kernelsu", &s) == -1) {
if (errno != ENOENT) LOGE("Failed to stat KSU manager data directory: %s\n", strerror(errno));
if (errno != ENOENT) {
LOGE("Failed to stat KSU manager data directory: %s\n", strerror(errno));
}
errno = 0;
return false;

View File

@@ -80,8 +80,6 @@ enum RootImplState magisk_get_existence(void) {
return Abnormal;
}
LOGI("Magisk info: %s\n", magisk_info);
for (unsigned long i = 0; i < sizeof(supported_variants) / sizeof(supported_variants[0]); i++) {
if (strstr(magisk_info, supported_variants[i])) variant = (enum magisk_variants)(i + 1);
}
@@ -96,15 +94,11 @@ enum RootImplState magisk_get_existence(void) {
return Abnormal;
}
LOGI("Magisk version: %s\n", magisk_version);
if (atoi(magisk_version) >= MIN_MAGISK_VERSION) return Supported;
else return TooOld;
}
bool magisk_uid_granted_root(uid_t uid) {
LOGI("Checking if UID %d is granted root access\n", uid);
char sqlite_cmd[256];
snprintf(sqlite_cmd, sizeof(sqlite_cmd), "select 1 from policies where uid=%d and policy=2 limit 1", uid);
@@ -122,8 +116,6 @@ bool magisk_uid_granted_root(uid_t uid) {
}
bool magisk_uid_should_umount(uid_t uid) {
LOGI("Checking if UID %d should unmount\n", uid);
struct dirent *entry;
DIR *proc = opendir("/proc");
if (!proc) {
@@ -197,8 +189,6 @@ bool magisk_uid_should_umount(uid_t uid) {
}
bool magisk_uid_is_manager(uid_t uid) {
LOGI("Checking if UID %d is a Magisk manager\n", uid);
char sqlite_cmd[256];
snprintf(sqlite_cmd, sizeof(sqlite_cmd), "select value from strings where key=\"requester\" limit 1");
@@ -212,9 +202,11 @@ bool magisk_uid_is_manager(uid_t uid) {
return false;
}
if (output[0] == '\0') {
char stat_path[PATH_MAX];
if (output[0] == '\0')
snprintf(stat_path, sizeof(stat_path), "/data/user_de/0/%s", magisk_managers[(int)variant]);
else
snprintf(stat_path, sizeof(stat_path), "/data/user_de/0/%s", output + strlen("value="));
struct stat s;
if (stat(stat_path, &s) == -1) {
@@ -225,20 +217,4 @@ bool magisk_uid_is_manager(uid_t uid) {
}
return s.st_uid == uid;
} else {
char stat_path[PATH_MAX];
snprintf(stat_path, sizeof(stat_path), "/data/user_de/0/%s", output + strlen("value=")); /* BUG: ISSUE HERE, OUTPUT IS NOT VALID */
LOGI("Checking |%s|\n", stat_path);
struct stat s;
if (stat(stat_path, &s) == -1) {
LOGE("Failed to stat %s WHAT %s\n", stat_path, strerror(errno));
errno = 0;
return false;
}
return s.st_uid == uid;
}
}

View File

@@ -101,21 +101,10 @@ static enum Architecture get_arch(void) {
}
int create_library_fd(const char *restrict so_path) {
/* INFO: This is required as older implementations of glibc may not
have the memfd_create function call, causing a crash. */
int memfd = syscall(SYS_memfd_create, "jit-cache-zygisk", MFD_ALLOW_SEALING);
if (memfd == -1) {
LOGE("Failed creating memfd: %s\n", strerror(errno));
return -1;
}
int so_fd = open(so_path, O_RDONLY);
if (so_fd == -1) {
LOGE("Failed opening so file: %s\n", strerror(errno));
close(memfd);
return -1;
}
@@ -124,7 +113,6 @@ int create_library_fd(const char *restrict so_path) {
LOGE("Failed getting so file size: %s\n", strerror(errno));
close(so_fd);
close(memfd);
return -1;
}
@@ -133,7 +121,15 @@ int create_library_fd(const char *restrict so_path) {
LOGE("Failed seeking so file: %s\n", strerror(errno));
close(so_fd);
close(memfd);
return -1;
}
/* INFO: This is required as older implementations of glibc may not
have the memfd_create function call, causing a crash. */
int memfd = syscall(SYS_memfd_create, "jit-cache-zygisk", MFD_ALLOW_SEALING);
if (memfd == -1) {
LOGE("Failed creating memfd: %s\n", strerror(errno));
return -1;
}
@@ -163,7 +159,7 @@ int create_library_fd(const char *restrict so_path) {
/* WARNING: Dynamic memory based */
static void load_modules(enum Architecture arch, struct Context *restrict context) {
context->len = 0;
context->modules = malloc(1);
context->modules = NULL;
DIR *dir = opendir(PATH_MODULES_DIR);
if (dir == NULL) {
@@ -212,7 +208,6 @@ static void load_modules(enum Architecture arch, struct Context *restrict contex
errno = 0;
} else continue;
LOGI("Loading module `%s`...\n", name);
int lib_fd = create_library_fd(so_path);
if (lib_fd == -1) {
LOGE("Failed loading module `%s`\n", name);
@@ -220,9 +215,14 @@ static void load_modules(enum Architecture arch, struct Context *restrict contex
continue;
}
LOGI("Loaded module lib fd: %d\n", lib_fd);
context->modules = realloc(context->modules, ((context->len + 1) * sizeof(struct Module)));
if (context->modules == NULL) {
LOGE("Failed reallocating memory for modules.\n");
return;
}
context->modules[context->len].name = strdup(name);
context->modules[context->len].lib_fd = lib_fd;
context->modules[context->len].companion = -1;
@@ -244,8 +244,6 @@ static int create_daemon_socket(void) {
}
static int spawn_companion(char *restrict argv[], char *restrict name, int lib_fd) {
LOGI("Spawning a new companion...\n");
int sockets[2];
if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) {
LOGE("Failed creating socket pair.\n");
@@ -296,8 +294,6 @@ static int spawn_companion(char *restrict argv[], char *restrict name, int lib_f
return -1;
}
LOGI("Companion response: %hhu\n", response);
switch (response) {
/* INFO: Even without any entry, we should still just deal with it */
case 0: { return -2; }
@@ -367,6 +363,12 @@ void zygiskd_start(char *restrict argv[]) {
enum RootImpl impl = get_impl();
if (impl == None) {
struct MsgHead *msg = malloc(sizeof(struct MsgHead) + sizeof("No root implementation found."));
if (msg == NULL) {
LOGE("Failed allocating memory for message.\n");
return;
}
msg->cmd = DAEMON_SET_ERROR_INFO;
msg->length = sizeof("No root implementation found.");
memcpy(msg->data, "No root implementation found.", msg->length);
@@ -376,6 +378,12 @@ void zygiskd_start(char *restrict argv[]) {
free(msg);
} else if (impl == Multiple) {
struct MsgHead *msg = malloc(sizeof(struct MsgHead) + sizeof("Multiple root implementations found. Not supported yet."));
if (msg == NULL) {
LOGE("Failed allocating memory for message.\n");
return;
}
msg->cmd = DAEMON_SET_ERROR_INFO;
msg->length = sizeof("Multiple root implementations found. Not supported yet.");
memcpy(msg->data, "Multiple root implementations found. Not supported yet.", msg->length);
@@ -421,6 +429,12 @@ void zygiskd_start(char *restrict argv[]) {
if (context.len == 0) {
msg = malloc(sizeof(struct MsgHead) + strlen("Root: , Modules: None") + root_impl_len + 1);
if (msg == NULL) {
LOGE("Failed allocating memory for message.\n");
return;
}
msg->cmd = DAEMON_SET_INFO;
msg->length = strlen("Root: , Modules: None") + root_impl_len + 1;
@@ -445,6 +459,12 @@ void zygiskd_start(char *restrict argv[]) {
}
} else {
char *module_list = malloc(1);
if (module_list == NULL) {
LOGE("Failed allocating memory for module list.\n");
return;
}
size_t module_list_len = 0;
for (int i = 0; i < context.len; i++) {
@@ -466,6 +486,12 @@ void zygiskd_start(char *restrict argv[]) {
}
msg = malloc(sizeof(struct MsgHead) + strlen("Root: , Modules: ") + root_impl_len + module_list_len + 1);
if (msg == NULL) {
LOGE("Failed allocating memory for message.\n");
return;
}
msg->cmd = DAEMON_SET_INFO;
msg->length = strlen("Root: , Modules: ") + root_impl_len + module_list_len + 1;
@@ -532,18 +558,12 @@ void zygiskd_start(char *restrict argv[]) {
switch (action) {
case PingHeartbeat: {
LOGI("ZD-- PingHeartbeat\n");
enum DaemonSocketAction msgr = ZYGOTE_INJECTED;
unix_datagram_sendto(CONTROLLER_SOCKET, &msgr, sizeof(enum DaemonSocketAction));
LOGI("ZD++ PingHeartbeat\n");
break;
}
case ZygoteRestart: {
LOGI("ZD-- ZygoteRestart\n");
for (int i = 0; i < context.len; i++) {
if (context.modules[i].companion != -1) {
close(context.modules[i].companion);
@@ -551,18 +571,12 @@ void zygiskd_start(char *restrict argv[]) {
}
}
LOGI("ZD++ ZygoteRestart\n");
break;
}
case SystemServerStarted: {
LOGI("ZD-- SystemServerStarted\n");
enum DaemonSocketAction msgr = SYSTEM_SERVER_STARTED;
unix_datagram_sendto(CONTROLLER_SOCKET, &msgr, sizeof(enum DaemonSocketAction));
LOGI("ZD++ SystemServerStarted\n");
break;
}
case RequestLogcatFd: {
@@ -598,8 +612,6 @@ void zygiskd_start(char *restrict argv[]) {
break;
}
case GetProcessFlags: {
LOGI("ZD-- GetProcessFlags\n");
uint32_t uid = 0;
ssize_t ret = read_uint32_t(client_fd, &uid);
ASSURE_SIZE_READ_BREAK("GetProcessFlags", "uid", ret, sizeof(uid));
@@ -639,13 +651,9 @@ void zygiskd_start(char *restrict argv[]) {
ret = write_int(client_fd, flags);
ASSURE_SIZE_WRITE_BREAK("GetProcessFlags", "flags", ret, sizeof(flags));
LOGI("ZD++ GetProcessFlags\n");
break;
}
case GetInfo: {
LOGI("ZD-- GetInfo\n");
uint32_t flags = 0;
switch (get_impl()) {
@@ -677,34 +685,40 @@ void zygiskd_start(char *restrict argv[]) {
size_t modules_len = context.len;
ret = write_size_t(client_fd, modules_len);
ASSURE_SIZE_WRITE_BREAK("GetInfo", "modules_len", ret, sizeof(modules_len));
for (size_t i = 0; i < modules_len; i++) {
write_string(client_fd, context.modules[i].name);
}
ret = write_string(client_fd, context.modules[i].name);
if (ret == -1) {
LOGE("Failed writing module name.\n");
LOGI("ZD++ GetInfo\n");
break;
}
}
break;
}
case ReadModules: {
LOGI("ZD-- ReadModules\n");
size_t clen = context.len;
ssize_t ret = write_size_t(client_fd, clen);
ASSURE_SIZE_WRITE_BREAK("ReadModules", "len", ret, sizeof(clen));
for (size_t i = 0; i < clen; i++) {
if (write_string(client_fd, context.modules[i].name) == -1) break;
if (write_fd(client_fd, context.modules[i].lib_fd) == -1) break;
}
if (write_string(client_fd, context.modules[i].name) == -1) {
LOGE("Failed writing module name.\n");
LOGI("ZD++ ReadModules\n");
break;
}
if (write_fd(client_fd, context.modules[i].lib_fd) == -1) {
LOGE("Failed writing module fd.\n");
break;
}
}
break;
}
case RequestCompanionSocket: {
LOGI("ZD-- RequestCompanionSocket\n");
size_t index = 0;
ssize_t ret = read_size_t(client_fd, &index);
ASSURE_SIZE_READ_BREAK("RequestCompanionSocket", "index", ret, sizeof(index));
@@ -770,10 +784,9 @@ void zygiskd_start(char *restrict argv[]) {
break;
}
case GetModuleDir: {
LOGI("ZD-- GetModuleDir\n");
size_t index = 0;
read_size_t(client_fd, &index);
ASSURE_SIZE_READ_BREAK("GetModuleDir", "index", ret, sizeof(index));
char module_dir[PATH_MAX];
snprintf(module_dir, PATH_MAX, "%s/%s", PATH_MODULES_DIR, context.modules[index].name);
@@ -802,8 +815,6 @@ void zygiskd_start(char *restrict argv[]) {
break;
}
LOGI("ZD++ GetModuleDir\n");
break;
}
}