fix: memory leak, unitialized memory access, FILE pointer leak bugs

This commit fixes numerous general code bugs, improving reliability and consistency of ReZygisk.
This commit is contained in:
ThePedroo
2025-08-10 17:06:48 -03:00
parent a7917e20fe
commit 08513b17e8
11 changed files with 231 additions and 156 deletions

View File

@@ -153,7 +153,14 @@ void companion_entry(int fd) {
LOGI("New companion request.\n - Module name: %s\n - Client fd: %d\n", name, client_fd);
ret = write_uint8_t(client_fd, 1);
ASSURE_SIZE_WRITE("ZygiskdCompanion", "client_fd", ret, sizeof(uint8_t));
if (ret != sizeof(uint8_t)) {
LOGE("Failed to sent client_fd in ZygiskdCompanion: Expected %zu, got %zd\n", sizeof(uint8_t), ret);
free(args);
close(client_fd);
break;
}
pthread_t thread;
if (pthread_create(&thread, NULL, entry_thread, (void *)args) == 0)

View File

@@ -73,6 +73,14 @@ struct packages_config {
size_t size;
};
void _apatch_free_package_config(struct packages_config *restrict config) {
for (size_t i = 0; i < config->size; i++) {
free(config->configs[i].process);
}
free(config->configs);
}
/* WARNING: Dynamic memory based */
bool _apatch_get_package_config(struct packages_config *restrict config) {
config->configs = NULL;
@@ -96,14 +104,16 @@ bool _apatch_get_package_config(struct packages_config *restrict config) {
}
while (fgets(line, sizeof(line), fp) != NULL) {
config->configs = realloc(config->configs, (config->size + 1) * sizeof(struct package_config));
if (config->configs == NULL) {
struct package_config *tmp_configs = realloc(config->configs, (config->size + 1) * sizeof(struct package_config));
if (tmp_configs == NULL) {
LOGE("Failed to realloc APatch config struct: %s\n", strerror(errno));
_apatch_free_package_config(config);
fclose(fp);
return false;
}
config->configs = tmp_configs;
config->configs[config->size].process = strdup(strtok(line, ","));
@@ -128,21 +138,9 @@ bool _apatch_get_package_config(struct packages_config *restrict config) {
return true;
}
void _apatch_free_package_config(struct packages_config *restrict config) {
for (size_t i = 0; i < config->size; i++) {
free(config->configs[i].process);
}
free(config->configs);
}
bool apatch_uid_granted_root(uid_t uid) {
struct packages_config config;
if (!_apatch_get_package_config(&config)) {
_apatch_free_package_config(&config);
return false;
}
if (!_apatch_get_package_config(&config)) return false;
for (size_t i = 0; i < config.size; i++) {
if (config.configs[i].uid != uid) continue;
@@ -162,11 +160,7 @@ bool apatch_uid_granted_root(uid_t uid) {
bool apatch_uid_should_umount(uid_t uid, const char *const process) {
struct packages_config config;
if (!_apatch_get_package_config(&config)) {
_apatch_free_package_config(&config);
return false;
}
if (!_apatch_get_package_config(&config)) return false;
for (size_t i = 0; i < config.size; i++) {
if (config.configs[i].uid != uid) continue;

View File

@@ -74,8 +74,7 @@ void root_impls_setup(void) {
}
void get_impl(struct root_impl *uimpl) {
uimpl->impl = impl.impl;
uimpl->variant = impl.variant;
*uimpl = impl;
}
bool uid_granted_root(uid_t uid) {

View File

@@ -109,12 +109,9 @@ static void get_current_attr(char *restrict output, size_t size) {
return;
}
if (fread(output, 1, size, current) == 0) {
if (fread(output, 1, size, current) == 0)
LOGE("fread: %s\n", strerror(errno));
return;
}
fclose(current);
}
@@ -543,15 +540,13 @@ bool parse_mountinfo(const char *restrict pid, struct mountinfos *restrict mount
&optional_start, &optional_end, &type_start, &type_end,
&source_start, &source_end, &fs_option_start, &fs_option_end);
mounts->mounts = (struct mountinfo *)realloc(mounts->mounts, (i + 1) * sizeof(struct mountinfo));
if (!mounts->mounts) {
struct mountinfo *tmp_mounts = (struct mountinfo *)realloc(mounts->mounts, (i + 1) * sizeof(struct mountinfo));
if (!tmp_mounts) {
LOGE("Failed to allocate memory for mounts->mounts");
fclose(mountinfo);
free_mounts(mounts);
return false;
goto cleanup_mount_allocs;
}
mounts->mounts = tmp_mounts;
unsigned int shared = 0;
unsigned int master = 0;
@@ -572,16 +567,64 @@ bool parse_mountinfo(const char *restrict pid, struct mountinfos *restrict mount
mounts->mounts[i].parent = parent;
mounts->mounts[i].device = (dev_t)(makedev(maj, min));
mounts->mounts[i].root = strndup(line + root_start, (size_t)(root_end - root_start));
if (mounts->mounts[i].root == NULL) {
LOGE("Failed to allocate memory for root\n");
goto cleanup_mount_allocs;
}
mounts->mounts[i].target = strndup(line + target_start, (size_t)(target_end - target_start));
if (mounts->mounts[i].target == NULL) {
LOGE("Failed to allocate memory for target\n");
goto cleanup_root;
}
mounts->mounts[i].vfs_option = strndup(line + vfs_option_start, (size_t)(vfs_option_end - vfs_option_start));
if (mounts->mounts[i].vfs_option == NULL) {
LOGE("Failed to allocate memory for vfs_option\n");
goto cleanup_target;
}
mounts->mounts[i].optional.shared = shared;
mounts->mounts[i].optional.master = master;
mounts->mounts[i].optional.propagate_from = propagate_from;
mounts->mounts[i].type = strndup(line + type_start, (size_t)(type_end - type_start));
if (mounts->mounts[i].type == NULL) {
LOGE("Failed to allocate memory for type\n");
goto cleanup_vfs_option;
}
mounts->mounts[i].source = strndup(line + source_start, (size_t)(source_end - source_start));
if (mounts->mounts[i].source == NULL) {
LOGE("Failed to allocate memory for source\n");
goto cleanup_type;
}
mounts->mounts[i].fs_option = strndup(line + fs_option_start, (size_t)(fs_option_end - fs_option_start));
if (mounts->mounts[i].fs_option == NULL) {
LOGE("Failed to allocate memory for fs_option\n");
goto cleanup_source;
}
i++;
continue;
cleanup_source:
free((void *)mounts->mounts[i].source);
cleanup_type:
free((void *)mounts->mounts[i].type);
cleanup_vfs_option:
free((void *)mounts->mounts[i].vfs_option);
cleanup_target:
free((void *)mounts->mounts[i].target);
cleanup_root:
free((void *)mounts->mounts[i].root);
cleanup_mount_allocs:
fclose(mountinfo);
free_mounts(mounts);
return false;
}
fclose(mountinfo);

View File

@@ -123,7 +123,6 @@ static void load_modules(enum Architecture arch, struct Context *restrict contex
continue;
}
context->modules = realloc(context->modules, (size_t)((context->len + 1) * sizeof(struct Module)));
if (context->modules == NULL) {
LOGE("Failed reallocating memory for modules.\n");
@@ -143,8 +142,10 @@ static void load_modules(enum Architecture arch, struct Context *restrict contex
static void free_modules(struct Context *restrict context) {
for (size_t i = 0; i < context->len; i++) {
free(context->modules[i].name);
if (context->modules[i].companion != -1) close(context->modules[i].companion);
if (context->modules[i].companion >= 0) close(context->modules[i].companion);
}
free(context->modules);
}
static int create_daemon_socket(void) {
@@ -288,7 +289,7 @@ void zygiskd_start(char *restrict argv[]) {
unix_datagram_sendto(CONTROLLER_SOCKET, &msg, sizeof(struct MsgHead));
unix_datagram_sendto(CONTROLLER_SOCKET, msg_data, (size_t)msg.length);
free(msg_data);
exit(EXIT_FAILURE);
} else {
enum Architecture arch = get_arch();
load_modules(arch, &context);
@@ -301,12 +302,24 @@ void zygiskd_start(char *restrict argv[]) {
} else {
for (size_t i = 0; i < context.len; i++) {
if (i != context.len - 1) {
module_list = realloc(module_list, module_list_len + strlen(context.modules[i].name) + strlen(", ") + 1);
if (module_list == NULL) {
char *tmp_module_list = realloc(module_list, module_list_len + strlen(context.modules[i].name) + strlen(", ") + 1);
if (tmp_module_list == NULL) {
LOGE("Failed reallocating memory for module list.\n");
return;
char *kmsg_failure = "Failed reallocating memory for module list";
struct MsgHead msg = {
.cmd = DAEMON_SET_ERROR_INFO,
.length = (int)strlen(kmsg_failure) + 1
};
unix_datagram_sendto(CONTROLLER_SOCKET, &msg, sizeof(struct MsgHead));
unix_datagram_sendto(CONTROLLER_SOCKET, kmsg_failure, (size_t)msg.length);
free(module_list);
free_modules(&context);
exit(EXIT_FAILURE);
}
module_list = tmp_module_list;
strcpy(module_list + module_list_len, context.modules[i].name);
@@ -316,12 +329,24 @@ void zygiskd_start(char *restrict argv[]) {
module_list_len += strlen(", ");
} else {
module_list = realloc(module_list, module_list_len + strlen(context.modules[i].name) + 1);
if (module_list == NULL) {
char *tmp_module_list = realloc(module_list, module_list_len + strlen(context.modules[i].name) + 1);
if (tmp_module_list == NULL) {
LOGE("Failed reallocating memory for module list.\n");
return;
char *kmsg_failure = "Failed reallocating memory for module list";
struct MsgHead msg = {
.cmd = DAEMON_SET_ERROR_INFO,
.length = (int)strlen(kmsg_failure) + 1
};
unix_datagram_sendto(CONTROLLER_SOCKET, &msg, sizeof(struct MsgHead));
unix_datagram_sendto(CONTROLLER_SOCKET, kmsg_failure, (size_t)msg.length);
free(module_list);
free_modules(&context);
exit(EXIT_FAILURE);
}
module_list = tmp_module_list;
strcpy(module_list + module_list_len, context.modules[i].name);
@@ -344,7 +369,16 @@ void zygiskd_start(char *restrict argv[]) {
if (msg_data == NULL) {
LOGE("Failed allocating memory for message data.\n");
return;
char *kmsg_failure = "Failed allocating memory for message data";
msg.cmd = DAEMON_SET_ERROR_INFO;
msg.length = (int)strlen(kmsg_failure) + 1;
unix_datagram_sendto(CONTROLLER_SOCKET, &msg, sizeof(struct MsgHead));
unix_datagram_sendto(CONTROLLER_SOCKET, kmsg_failure, (size_t)msg.length);
free(module_list);
free_modules(&context);
exit(EXIT_FAILURE);
}
snprintf(msg_data, msg_length, "Root: %s, Modules: %s", impl_name, module_list);
@@ -360,6 +394,8 @@ void zygiskd_start(char *restrict argv[]) {
if (socket_fd == -1) {
LOGE("Failed creating daemon socket\n");
free_modules(&context);
return;
}
@@ -372,7 +408,7 @@ void zygiskd_start(char *restrict argv[]) {
if (client_fd == -1) {
LOGE("accept: %s\n", strerror(errno));
return;
break;
}
uint8_t action8 = 0;
@@ -380,11 +416,11 @@ void zygiskd_start(char *restrict argv[]) {
if (len == -1) {
LOGE("read: %s\n", strerror(errno));
return;
break;
} else if (len == 0) {
LOGI("Client disconnected\n");
return;
break;
}
enum DaemonSocketAction action = (enum DaemonSocketAction)action8;
@@ -402,10 +438,10 @@ void zygiskd_start(char *restrict argv[]) {
}
case ZygoteRestart: {
for (size_t i = 0; i < context.len; i++) {
if (context.modules[i].companion != -1) {
close(context.modules[i].companion);
context.modules[i].companion = -1;
}
if (context.modules[i].companion <= -1) continue;
close(context.modules[i].companion);
context.modules[i].companion = -1;
}
break;
@@ -566,9 +602,19 @@ void zygiskd_start(char *restrict argv[]) {
ssize_t ret = read_size_t(client_fd, &index);
ASSURE_SIZE_READ_BREAK("RequestCompanionSocket", "index", ret, sizeof(index));
struct Module *module = &context.modules[index];
if (index >= context.len) {
LOGE("Invalid module index: %zu\n", index);
if (module->companion != -1) {
ret = write_uint8_t(client_fd, 0);
ASSURE_SIZE_WRITE_BREAK("RequestCompanionSocket", "response", ret, sizeof(int));
close(client_fd);
break;
}
struct Module *module = &context.modules[index];
if (module->companion >= 0) {
if (!check_unix_socket(module->companion, false)) {
LOGE(" - Companion for module \"%s\" crashed\n", module->name);
@@ -577,10 +623,10 @@ void zygiskd_start(char *restrict argv[]) {
}
}
if (module->companion == -1) {
if (module->companion <= -1) {
module->companion = spawn_companion(argv, module->name, module->lib_fd);
if (module->companion > 0) {
if (module->companion >= 0) {
LOGI(" - Spawned companion for \"%s\": %d\n", module->name, module->companion);
} else {
if (module->companion == -2) {
@@ -597,7 +643,7 @@ void zygiskd_start(char *restrict argv[]) {
so just sending the file descriptor of the client is
safe.
*/
if (module->companion != -1) {
if (module->companion >= 0) {
LOGI(" - Sending companion fd socket of module \"%s\"\n", module->name);
if (write_fd(module->companion, client_fd) == -1) {
@@ -629,6 +675,17 @@ void zygiskd_start(char *restrict argv[]) {
ssize_t ret = read_size_t(client_fd, &index);
ASSURE_SIZE_READ_BREAK("GetModuleDir", "index", ret, sizeof(index));
if (index >= context.len) {
LOGE("Invalid module index: %zu\n", index);
ret = write_uint8_t(client_fd, 0);
ASSURE_SIZE_WRITE_BREAK("GetModuleDir", "response", ret, sizeof(int));
close(client_fd);
break;
}
char module_dir[PATH_MAX];
snprintf(module_dir, PATH_MAX, "%s/%s", PATH_MODULES_DIR, context.modules[index].name);