Skip to content

Commit 08513b1

Browse files
ThePedrooThePedroo
committed
fix: memory leak, unitialized memory access, FILE pointer leak bugs
This commit fixes numerous general code bugs, improving reliability and consistency of ReZygisk.
1 parent a7917e2 commit 08513b1

11 files changed

Lines changed: 231 additions & 156 deletions

File tree

‎loader/src/common/daemon.c‎

Lines changed: 36 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -108,43 +108,32 @@ void rezygiskd_get_info(struct rezygisk_info *info) {
108108

109109
read_uint32_t(fd, (uint32_t *)&info->pid);
110110

111-
read_size_t(fd, &info->modules->modules_count);
112-
if (info->modules->modules_count == 0) {
113-
info->modules->modules = NULL;
111+
read_size_t(fd, &info->modules.modules_count);
112+
if (info->modules.modules_count == 0) {
113+
info->modules.modules = NULL;
114114

115115
close(fd);
116116

117117
return;
118118
}
119119

120-
info->modules->modules = (char **)malloc(sizeof(char *) * info->modules->modules_count);
121-
if (info->modules->modules == NULL) {
120+
info->modules.modules = (char **)malloc(sizeof(char *) * info->modules.modules_count);
121+
if (!info->modules.modules) {
122122
PLOGE("allocating modules name memory");
123123

124-
free(info->modules);
125-
info->modules = NULL;
126-
info->modules->modules_count = 0;
124+
info->modules.modules_count = 0;
127125

128126
close(fd);
129127

130128
return;
131129
}
132130

133-
for (size_t i = 0; i < info->modules->modules_count; i++) {
131+
for (size_t i = 0; i < info->modules.modules_count; i++) {
134132
char *module_name = read_string(fd);
135133
if (module_name == NULL) {
136134
PLOGE("reading module name");
137135

138-
info->modules->modules_count = i;
139-
140-
free_rezygisk_info(info);
141-
142-
info->modules = NULL;
143-
info->modules->modules_count = 0;
144-
145-
close(fd);
146-
147-
return;
136+
goto info_cleanup;
148137
}
149138

150139
char module_path[PATH_MAX];
@@ -156,43 +145,49 @@ void rezygiskd_get_info(struct rezygisk_info *info) {
156145
if (!module_prop) {
157146
PLOGE("failed to open module prop file %s", module_path);
158147

159-
info->modules->modules_count = i;
160-
161-
free_rezygisk_info(info);
162-
163-
info->modules = NULL;
164-
info->modules->modules_count = 0;
165-
166-
close(fd);
167-
168-
return;
148+
goto info_cleanup;
169149
}
170150

151+
info->modules.modules[i] = NULL;
152+
171153
char line[1024];
172154
while (fgets(line, sizeof(line), module_prop) != NULL) {
173155
if (strncmp(line, "name=", strlen("name=")) != 0) continue;
174156

175-
info->modules->modules[i] = strndup(line + 5, strlen(line) - 6);
157+
info->modules.modules[i] = strndup(line + 5, strlen(line) - 6);
176158

177159
break;
178160
}
179161

162+
if (info->modules.modules[i] == NULL) {
163+
PLOGE("failed to read module name from %s", module_path);
164+
165+
fclose(module_prop);
166+
167+
goto info_cleanup;
168+
}
169+
180170
fclose(module_prop);
171+
172+
continue;
173+
174+
info_cleanup:
175+
info->modules.modules_count = i;
176+
free_rezygisk_info(info);
177+
178+
break;
181179
}
182180

183181
close(fd);
184182
}
185183

186184
void free_rezygisk_info(struct rezygisk_info *info) {
187-
if (info->modules->modules) {
188-
for (size_t i = 0; i < info->modules->modules_count; i++) {
189-
free(info->modules->modules[i]);
190-
}
191-
192-
free(info->modules->modules);
185+
for (size_t i = 0; i < info->modules.modules_count; i++) {
186+
free(info->modules.modules[i]);
193187
}
194188

195-
free(info->modules);
189+
free(info->modules.modules);
190+
info->modules.modules = NULL;
196191
}
197192

198193
bool rezygiskd_read_modules(struct zygisk_modules *modules) {
@@ -237,13 +232,11 @@ bool rezygiskd_read_modules(struct zygisk_modules *modules) {
237232
}
238233

239234
void free_modules(struct zygisk_modules *modules) {
240-
if (modules->modules) {
241-
for (size_t i = 0; i < modules->modules_count; i++) {
242-
free(modules->modules[i]);
243-
}
244-
245-
free(modules->modules);
235+
for (size_t i = 0; i < modules->modules_count; i++) {
236+
free(modules->modules[i]);
246237
}
238+
239+
free(modules->modules);
247240
}
248241

249242
int rezygiskd_connect_companion(size_t index) {

‎loader/src/common/socket_utils.c‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ int read_fd(int fd) {
2525
.msg_controllen = sizeof(cmsgbuf)
2626
};
2727

28-
ssize_t ret = recvmsg(fd, &msg, MSG_WAITALL);
28+
ssize_t ret = TEMP_FAILURE_RETRY(recvmsg(fd, &msg, MSG_WAITALL));
2929
if (ret == -1) {
3030
PLOGE("recvmsg");
3131

@@ -47,14 +47,14 @@ int read_fd(int fd) {
4747

4848
ssize_t write_string(int fd, const char *str) {
4949
size_t str_len = strlen(str);
50-
ssize_t write_bytes = write(fd, &str_len, sizeof(size_t));
50+
ssize_t write_bytes = TEMP_FAILURE_RETRY(write(fd, &str_len, sizeof(size_t)));
5151
if (write_bytes != (ssize_t)sizeof(size_t)) {
5252
LOGE("Failed to write string length: Not all bytes were written (%zd != %zu).\n", write_bytes, sizeof(size_t));
5353

5454
return -1;
5555
}
5656

57-
write_bytes = write(fd, str, str_len);
57+
write_bytes = TEMP_FAILURE_RETRY(write(fd, str, str_len));
5858
if (write_bytes != (ssize_t)str_len) {
5959
LOGE("Failed to write string: Promised bytes doesn't exist (%zd != %zu).\n", write_bytes, str_len);
6060

@@ -66,7 +66,7 @@ ssize_t write_string(int fd, const char *str) {
6666

6767
char *read_string(int fd) {
6868
size_t str_len = 0;
69-
ssize_t read_bytes = read(fd, &str_len, sizeof(size_t));
69+
ssize_t read_bytes = TEMP_FAILURE_RETRY(read(fd, &str_len, sizeof(size_t)));
7070
if (read_bytes != (ssize_t)sizeof(size_t)) {
7171
LOGE("Failed to read string length: Not all bytes were read (%zd != %zu).\n", read_bytes, sizeof(size_t));
7272

@@ -80,7 +80,7 @@ char *read_string(int fd) {
8080
return NULL;
8181
}
8282

83-
read_bytes = read(fd, buf, str_len);
83+
read_bytes = TEMP_FAILURE_RETRY(read(fd, buf, str_len));
8484
if (read_bytes != (ssize_t)str_len) {
8585
LOGE("Failed to read string: Promised bytes doesn't exist (%zd != %zu).\n", read_bytes, str_len);
8686

@@ -94,14 +94,14 @@ char *read_string(int fd) {
9494
return buf;
9595
}
9696

97-
#define write_func(type) \
98-
ssize_t write_## type(int fd, type val) { \
99-
return write(fd, &val, sizeof(type)); \
97+
#define write_func(type) \
98+
ssize_t write_## type(int fd, type val) { \
99+
return TEMP_FAILURE_RETRY(write(fd, &val, sizeof(type))); \
100100
}
101101

102-
#define read_func(type) \
103-
ssize_t read_## type(int fd, type *val) { \
104-
return read(fd, val, sizeof(type)); \
102+
#define read_func(type) \
103+
ssize_t read_## type(int fd, type *val) { \
104+
return TEMP_FAILURE_RETRY(read(fd, val, sizeof(type))); \
105105
}
106106

107107
write_func(uint8_t)

‎loader/src/include/daemon.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ enum root_impl {
4242
};
4343

4444
struct rezygisk_info {
45-
struct zygisk_modules *modules;
45+
struct zygisk_modules modules;
4646
enum root_impl root_impl;
4747
pid_t pid;
4848
bool running;

‎loader/src/ptracer/main.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,11 @@ int main(int argc, char **argv) {
7777
}
7878
}
7979

80-
if (info.modules->modules_count != 0) {
81-
printf("Modules: %zu\n", info.modules->modules_count);
80+
if (info.modules.modules_count != 0) {
81+
printf("Modules: %zu\n", info.modules.modules_count);
8282

83-
for (size_t i = 0; i < info.modules->modules_count; i++) {
84-
printf(" - %s\n", info.modules->modules[i]);
83+
for (size_t i = 0; i < info.modules.modules_count; i++) {
84+
printf(" - %s\n", info.modules.modules[i]);
8585
}
8686
} else {
8787
printf("Modules: N/A\n");

‎loader/src/ptracer/monitor.c‎

Lines changed: 21 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ struct rezygiskd_status status32 = {
6666

6767
int monitor_epoll_fd;
6868
bool monitor_events_running = true;
69+
typedef void (*monitor_event_callback_t)();
6970

7071
bool monitor_events_init() {
7172
monitor_epoll_fd = epoll_create(1);
@@ -78,14 +79,9 @@ bool monitor_events_init() {
7879
return true;
7980
}
8081

81-
struct monitor_event_cbs {
82-
void (*callback)();
83-
void (*stop_callback)();
84-
};
85-
86-
bool monitor_events_register_event(struct monitor_event_cbs *event_cbs, int fd, uint32_t events) {
82+
bool monitor_events_register_event(monitor_event_callback_t event_cb, int fd, uint32_t events) {
8783
struct epoll_event ev = {
88-
.data.ptr = event_cbs,
84+
.data.ptr = (void *)event_cb,
8985
.events = events
9086
};
9187

@@ -116,27 +112,23 @@ void monitor_events_loop() {
116112
struct epoll_event events[2];
117113
while (monitor_events_running) {
118114
int nfds = epoll_wait(monitor_epoll_fd, events, 2, -1);
119-
if (nfds == -1) {
120-
if (errno != EINTR) PLOGE("epoll_wait");
115+
if (nfds == -1 && errno != EINTR) {
116+
PLOGE("epoll_wait");
121117

122-
continue;
118+
monitor_events_running = false;
119+
120+
break;
123121
}
124122

125-
for (int i = 0; i < nfds; i++) {
126-
struct monitor_event_cbs *event_cbs = (struct monitor_event_cbs *)events[i].data.ptr;
127-
event_cbs->callback();
123+
for (int i = 0; i < nfds; i++) {
124+
((monitor_event_callback_t)events[i].data.ptr)();
128125

129126
if (!monitor_events_running) break;
130127
}
131128
}
132129

133130
if (monitor_epoll_fd >= 0) close(monitor_epoll_fd);
134131
monitor_epoll_fd = -1;
135-
136-
for (int i = 0; i < (int)(sizeof(events) / sizeof(events[0])); i++) {
137-
struct monitor_event_cbs *event_cbs = (struct monitor_event_cbs *)events[i].data.ptr;
138-
event_cbs->stop_callback();
139-
}
140132
}
141133

142134
int monitor_sock_fd;
@@ -272,15 +264,13 @@ void rezygiskd_listener_callback() {
272264
status64.daemon_info = NULL;
273265
}
274266

275-
status64.daemon_info = (char *)malloc(msg.length);
267+
status64.daemon_info = strdup(msg_data);
276268
if (!status64.daemon_info) {
277269
PLOGE("malloc daemon64 info");
278270

279271
break;
280272
}
281273

282-
strcpy(status64.daemon_info, msg_data);
283-
284274
update_status(NULL);
285275

286276
break;
@@ -293,15 +283,13 @@ void rezygiskd_listener_callback() {
293283
status32.daemon_info = NULL;
294284
}
295285

296-
status32.daemon_info = (char *)malloc(msg.length);
286+
status32.daemon_info = strdup(msg_data);
297287
if (!status32.daemon_info) {
298288
PLOGE("malloc daemon32 info");
299289

300290
break;
301291
}
302292

303-
strcpy(status32.daemon_info, msg_data);
304-
305293
update_status(NULL);
306294

307295
break;
@@ -316,15 +304,13 @@ void rezygiskd_listener_callback() {
316304
status64.daemon_error_info = NULL;
317305
}
318306

319-
status64.daemon_error_info = (char *)malloc(msg.length);
307+
status64.daemon_error_info = strdup(msg_data);
320308
if (!status64.daemon_error_info) {
321309
PLOGE("malloc daemon64 error info");
322310

323311
break;
324312
}
325313

326-
strcpy(status64.daemon_error_info, msg_data);
327-
328314
update_status(NULL);
329315

330316
break;
@@ -339,15 +325,13 @@ void rezygiskd_listener_callback() {
339325
status32.daemon_error_info = NULL;
340326
}
341327

342-
status32.daemon_error_info = (char *)malloc(msg.length);
328+
status32.daemon_error_info = strdup(msg_data);
343329
if (!status32.daemon_error_info) {
344330
PLOGE("malloc daemon32 error info");
345331

346332
break;
347333
}
348334

349-
strcpy(status32.daemon_error_info, msg_data);
350-
351335
update_status(NULL);
352336

353337
break;
@@ -847,10 +831,6 @@ void init_monitor() {
847831

848832
monitor_events_init();
849833

850-
struct monitor_event_cbs listener_cbs = {
851-
.callback = rezygiskd_listener_callback,
852-
.stop_callback = rezygiskd_listener_stop
853-
};
854834
if (!rezygiskd_listener_init()) {
855835
LOGE("failed to create socket");
856836

@@ -859,12 +839,8 @@ void init_monitor() {
859839
exit(1);
860840
}
861841

862-
monitor_events_register_event(&listener_cbs, monitor_sock_fd, EPOLLIN | EPOLLET);
842+
monitor_events_register_event(rezygiskd_listener_callback, monitor_sock_fd, EPOLLIN | EPOLLET);
863843

864-
struct monitor_event_cbs sigchld_cbs = {
865-
.callback = sigchld_listener_callback,
866-
.stop_callback = sigchld_listener_stop
867-
};
868844
if (sigchld_listener_init() == false) {
869845
LOGE("failed to create signalfd");
870846

@@ -874,10 +850,15 @@ void init_monitor() {
874850
exit(1);
875851
}
876852

877-
monitor_events_register_event(&sigchld_cbs, sigchld_signal_fd, EPOLLIN | EPOLLET);
853+
monitor_events_register_event(sigchld_listener_callback, sigchld_signal_fd, EPOLLIN | EPOLLET);
878854

879855
monitor_events_loop();
880856

857+
/* INFO: Once it stops the loop, we cannot access the epool data, so we
858+
either manually call the stops or save to a structure. */
859+
rezygiskd_listener_stop();
860+
sigchld_listener_stop();
861+
881862
if (status64.daemon_info) free(status64.daemon_info);
882863
if (status64.daemon_error_info) free(status64.daemon_error_info);
883864
if (status32.daemon_info) free(status32.daemon_info);

0 commit comments

Comments
 (0)