Skip to content

Commit 5b28906

Browse files
committed
fix: munmap wrong region
This commit fixes the issue that due to wrong heuristics logic, getting the "wrong" offset, it would cause the gap size to be unrealistic to the real one, causing issues. Moreover, the gaps was never an issue as "dlclose" would internally call "soinfo_free", freeing those regions, if existing.
1 parent cc2c069 commit 5b28906

4 files changed

Lines changed: 25 additions & 123 deletions

File tree

loader/src/injector/hook.cpp

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -741,18 +741,15 @@ bool load_modules_only() {
741741
zygisk_modules[zygisk_module_length].size = solist_get_size(entry);
742742

743743
zygisk_modules[zygisk_module_length].deconstructors = solist_get_deconstructors(entry);
744-
zygisk_modules[zygisk_module_length].gap = solist_get_gap_info(entry);
745744

746-
LOGD("Loaded module [%s]. Entry: %p, Base: %p, Size: %zu, Deconstructors: fini_func=%p, fini_array=%p (size: %zu), Gap: %p (size: %zu)",
745+
LOGD("Loaded module [%s]. Entry: %p, Base: %p, Size: %zu, Deconstructors: fini_func=%p, fini_array=%p (size: %zu)",
747746
lib_path,
748747
entry,
749748
zygisk_modules[zygisk_module_length].base,
750749
zygisk_modules[zygisk_module_length].size,
751750
zygisk_modules[zygisk_module_length].deconstructors.fini_func,
752751
zygisk_modules[zygisk_module_length].deconstructors.fini_array,
753-
zygisk_modules[zygisk_module_length].deconstructors.fini_array_size,
754-
zygisk_modules[zygisk_module_length].gap.start,
755-
zygisk_modules[zygisk_module_length].gap.size);
752+
zygisk_modules[zygisk_module_length].deconstructors.fini_array_size);
756753

757754
zygisk_modules[zygisk_module_length].unload = false;
758755

@@ -791,24 +788,28 @@ void ZygiskContext::run_modules_post() {
791788

792789
/* INFO: If module is unloaded by dlclose, there's no need to
793790
hide it from soinfo manually. */
794-
if (m->unload) {
795-
/* INFO: Deconstructors are called in the inverted order, and following fini array then fini
796-
function order. It must not change. */
797-
for (size_t j = m->deconstructors.fini_array_size; j > 0; j--) {
798-
void (*destructor)(void) = m->deconstructors.fini_array[j - 1];
799-
if (destructor) {
800-
LOGD("Calling destructor %p for module %p", (void *)destructor, (void *)m->zygisk_module_entry);
801-
802-
destructor();
803-
}
791+
if (!m->unload) continue;
792+
793+
/* INFO: Deconstructors are called in the inverted order, and following fini array then fini
794+
function order. It must not change. */
795+
for (size_t j = m->deconstructors.fini_array_size; j > 0; j--) {
796+
void (*destructor)(void) = m->deconstructors.fini_array[j - 1];
797+
if (destructor) {
798+
LOGD("Calling destructor %p for module %p", (void *)destructor, (void *)m->zygisk_module_entry);
799+
800+
destructor();
804801
}
802+
}
805803

806-
if (m->deconstructors.fini_func) m->deconstructors.fini_func();
804+
if (m->deconstructors.fini_func) m->deconstructors.fini_func();
807805

808-
solist_unload_lib(&m->gap, m->base, m->size);
806+
if (munmap(m->base, m->size) == -1) {
807+
PLOGE("Failed to unmap module library at %p with size %zu", m->base, m->size);
809808

810-
modules_unloaded++;
809+
continue;
811810
}
811+
812+
modules_unloaded++;
812813
}
813814

814815
if (zygisk_module_length > 0)

loader/src/injector/module.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ struct rezygisk_module {
153153
size_t size;
154154

155155
struct soinfo_deconstructor deconstructors;
156-
struct soinfo_gap gap;
157156

158157
bool unload;
159158
};

loader/src/injector/solist.c

Lines changed: 6 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
#include <stdbool.h>
33
#include <string.h>
44
#include <dlfcn.h>
5-
#include <sys/mman.h>
65

76
#include <linux/limits.h>
87

@@ -28,25 +27,17 @@
2827
size_t solist_fini_array_offset = 0xa8;
2928
size_t solist_fini_array_size_offset = 0xb0;
3029
size_t solist_fini_offset = 0xc0;
31-
32-
size_t solist_version_offset = 0x10c;
33-
size_t solist_gap_start_offset = 0x248;
34-
size_t solist_gap_size_offset = 0x250;
3530
#else
3631
size_t solist_base_offset = 0x8;
3732
size_t solist_size_offset = 0xc;
3833

3934
size_t solist_fini_array_offset = 0x58;
4035
size_t solist_fini_array_size_offset = 0x5c;
4136
size_t solist_fini_offset = 0x64;
42-
43-
size_t solist_version_offset = 0x94;
44-
size_t solist_gap_start_offset = 0x140;
45-
size_t solist_gap_size_offset = 0x144;
4637
#endif
4738

4839
bool base_size_found = false;
49-
bool fini_version_gap_found = false;
40+
bool fini_found = false;
5041

5142
static const char *(*get_realpath)(SoInfo *) = NULL;
5243
static SoInfo *(*find_containing_library)(const void *) = NULL;
@@ -177,7 +168,7 @@ static bool solist_init() {
177168

178169
ElfImg_destroy(linker);
179170

180-
for (size_t i = 0; i < 1024 / sizeof(void *); i++) {
171+
for (size_t i = 0; i < 680 / sizeof(void *); i++) {
181172
size_t possible_size_of_somain = *(size_t *)((uintptr_t)somain + i * sizeof(void *));
182173

183174
if (!base_size_found && possible_size_of_somain < 0x100000 && possible_size_of_somain > 0x100) {
@@ -191,7 +182,7 @@ static bool solist_init() {
191182
}
192183

193184
struct link_map *possible_link_map_head = (struct link_map *)((uintptr_t)solinker + i * sizeof(void *));
194-
if (!fini_version_gap_found && possible_link_map_head->l_name == solinker_map->l_name) {
185+
if (!fini_found && possible_link_map_head->l_name == solinker_map->l_name) {
195186
#ifdef __arm__
196187
/* INFO: For arm32, ARM_exidx and ARM_exidx_count is defined between them. */
197188
solist_fini_array_offset = (i - 7) * sizeof(void *);
@@ -209,29 +200,10 @@ static bool solist_init() {
209200
LOGD("solist_fini_offset is %zu * %zu = %p", (i - 2), sizeof(void *), (void *)solist_fini_offset);
210201
#endif
211202

212-
/* INFO: Offsets are hardcoded. They will not change. They have been retrieved through the same
213-
ways as the ones in the top of the file.
214-
*/
215-
#ifndef __LP64__
216-
solist_version_offset = (i * sizeof(void *)) + 0x20;
217-
solist_gap_start_offset = solist_version_offset + 0xAC;
218-
solist_gap_size_offset = solist_gap_start_offset + sizeof(ElfW(Addr));
219-
220-
LOGD("solist_version_offset is %zu * %zu + 0x20 = %p", i, sizeof(void *), (void *)solist_version_offset);
221-
LOGD("solist_gap_start_offset is %zu + 0xAC = %p", solist_version_offset, (void *)solist_gap_start_offset);
222-
LOGD("solist_gap_size_offset is %zu + %zu = %p", solist_gap_start_offset, sizeof(ElfW(Addr)), (void *)solist_gap_size_offset);
223-
#else
224-
solist_version_offset = (i * sizeof(void *)) + 0x3C;
225-
solist_gap_start_offset = solist_version_offset + 0x13C;
226-
solist_gap_size_offset = solist_gap_start_offset + sizeof(ElfW(Addr));
227-
228-
LOGD("solist_version_offset is %zu * %zu + 0x3C = %p", i, sizeof(void *), (void *)solist_version_offset);
229-
LOGD("solist_gap_start_offset is %zu + 0x13C = %p", solist_version_offset, (void *)solist_gap_start_offset);
230-
LOGD("solist_gap_size_offset is %zu + %zu = %p", solist_gap_start_offset, sizeof(ElfW(Addr)), (void *)solist_gap_size_offset);
231-
#endif
232-
233-
fini_version_gap_found = true;
203+
fini_found = true;
234204
}
205+
206+
if (base_size_found && fini_found) break;
235207
}
236208

237209
return true;
@@ -368,18 +340,6 @@ void solist_reset_counters(size_t load, size_t unload) {
368340
}
369341
}
370342

371-
void solist_unload_lib(struct soinfo_gap *gap, void *base, size_t size) {
372-
if (munmap(base, size) == -1) {
373-
PLOGE("Failed to unmap library from %p (size %zu)", base, size);
374-
}
375-
376-
if (gap->size) {
377-
if (munmap(gap->start, gap->size) == -1) {
378-
PLOGE("Failed to unmap gap from %p (size %zu)", gap->start, gap->size);
379-
}
380-
}
381-
}
382-
383343
ssize_t solist_get_size(void *lib_memory) {
384344
if (somain == NULL && !solist_init()) {
385345
LOGE("Failed to initialize solist");
@@ -438,33 +398,3 @@ struct soinfo_deconstructor solist_get_deconstructors(void *lib_memory) {
438398

439399
return result;
440400
}
441-
442-
struct soinfo_gap solist_get_gap_info(void *lib_memory) {
443-
struct soinfo_gap result = { 0 };
444-
445-
if (somain == NULL && !solist_init()) {
446-
LOGE("Failed to initialize solist");
447-
448-
return result;
449-
}
450-
451-
SoInfo *found = (*find_containing_library)(lib_memory);
452-
if (found == NULL) {
453-
LOGD("Could not find containing library for %p", lib_memory);
454-
455-
return result;
456-
}
457-
458-
if (*(uint32_t *)((uintptr_t)found + solist_version_offset) < 6) {
459-
LOGD("soinfo for %p has no gap info (version %d)",
460-
lib_memory,
461-
*(uint32_t *)((uintptr_t)found + solist_version_offset));
462-
463-
return result;
464-
}
465-
466-
result.start = *(void **)((uintptr_t)found + solist_gap_start_offset);
467-
result.size = *(size_t *)((uintptr_t)found + solist_gap_size_offset);
468-
469-
return result;
470-
}

loader/src/injector/solist.h

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ struct soinfo_deconstructor {
2020
void (**fini_array)();
2121
};
2222

23-
struct soinfo_gap {
24-
void *start;
25-
size_t size;
26-
};
27-
2823
/*
2924
INFO: When dlopen'ing a library, the system will save information of the
3025
opened library so a structure called soinfo, which contains another
@@ -71,20 +66,6 @@ bool solist_drop_so_path(void *lib_memory, bool unload);
7166
*/
7267
void solist_reset_counters(size_t load, size_t unload);
7368

74-
/*
75-
INFO: Loaded libraries have several mappings to load the different parts of their ELF
76-
in the memory. Some libraries might include a padding (gap) in case it is extended.
77-
78-
This function deals with both those jobs, and is purely here for organization, as it
79-
is simple.
80-
81-
SOURCES:
82-
- https://android.googlesource.com/platform/bionic/+/refs/heads/android15-release/linker/linker.cpp#333
83-
- https://android.googlesource.com/platform/bionic/+/refs/heads/android15-release/linker/linker_phdr.cpp#584
84-
*/
85-
void solist_unload_lib(struct soinfo_gap *gap, void *base, size_t size);
86-
87-
8869
/*
8970
INFO: Helper function to get the size of the mappings of a loaded library.
9071
@@ -112,15 +93,6 @@ void *solist_get_base(void *lib_memory);
11293
*/
11394
struct soinfo_deconstructor solist_get_deconstructors(void *lib_memory);
11495

115-
/*
116-
INFO: Helper function to get the information of the padding/gap.
117-
118-
SOURCES:
119-
- https://android.googlesource.com/platform/bionic/+/refs/heads/android15-release/linker/linker_soinfo.h#450
120-
- https://android.googlesource.com/platform/bionic/+/refs/heads/android15-release/linker/linker_soinfo.h#451
121-
*/
122-
struct soinfo_gap solist_get_gap_info(void *lib_memory);
123-
12496
#ifdef __cplusplus
12597
}
12698
#endif /* __cplusplus */

0 commit comments

Comments
 (0)