Cleanup file descriptors

jni/daemon/daemon.c

@@ -87,17 +87,18 @@ static void *request_handler(void *args) {
 		late_start(client);
 		break;
 	default:
-		close(client);
+		break;
 	}
+	// Just in case
+	close(client);
 	return NULL;
 }
 
 /* Setup the address and return socket fd */
 static int setup_socket(struct sockaddr_un *sun) {
 	int fd = xsocket(AF_LOCAL, SOCK_STREAM, 0);
-	if (fcntl(fd, F_SETFD, FD_CLOEXEC)) {
+	if (fcntl(fd, F_SETFD, FD_CLOEXEC))
 		PLOGE("fcntl FD_CLOEXEC");
-	}
 
 	memset(sun, 0, sizeof(*sun));
 	sun->sun_family = AF_LOCAL;
@@ -115,12 +116,13 @@ static void *large_sepol_patch(void *args) {
 	return NULL;
 }
 
-void start_daemon() {
+void start_daemon(int client) {
 	// Launch the daemon, create new session, set proper context
 	if (getuid() != UID_ROOT || getgid() != UID_ROOT) {
 		fprintf(stderr, "Starting daemon requires root: %s\n", strerror(errno));
 		PLOGE("start daemon");
 	}
+
 	switch (fork()) {
 	case -1:
 		PLOGE("fork");
@@ -129,10 +131,13 @@ void start_daemon() {
 	default:
 		return;
 	}
+
+	// First close the client, it's useless for us
+	close(client);
 	xsetsid();
 	setcon("u:r:su:s0");
 	umask(022);
-	null_fd = xopen("/dev/null", O_RDWR);
+	null_fd = xopen("/dev/null", O_RDWR | O_CLOEXEC);
 	xdup2(null_fd, STDIN_FILENO);
 	xdup2(null_fd, STDOUT_FILENO);
 	xdup2(null_fd, STDERR_FILENO);
@@ -142,12 +147,12 @@ void start_daemon() {
 	sepol_med_rules();
 	dump_policydb(SELINUX_LOAD);
 
-	// Continue the larger patch in another thread, we will need to join later
+	// Continue the larger patch in another thread, we will join later
 	pthread_create(&sepol_patch, NULL, large_sepol_patch, NULL);
 
 	struct sockaddr_un sun;
 	int fd = setup_socket(&sun);
-	
+
 	xbind(fd, (struct sockaddr*) &sun, sizeof(sun));
 	xlisten(fd, 10);
 
@@ -168,15 +173,17 @@ void start_daemon() {
 	// Setup links under /sbin
 	xmount(NULL, "/", NULL, MS_REMOUNT, NULL);
 	create_links(NULL, "/sbin");
-	chmod("/sbin", 0755);
+	xchmod("/sbin", 0755);
-	mkdir("/magisk", 0755);
+	xmkdir("/magisk", 0755);
-	chmod("/magisk", 0755);
+	xchmod("/magisk", 0755);
 	xmount(NULL, "/", NULL, MS_REMOUNT | MS_RDONLY, NULL);
 
-	// Loop forever to listen to requests
+	// Loop forever to listen for requests
 	while(1) {
 		int *client = xmalloc(sizeof(int));
 		*client = xaccept(fd, NULL, NULL);
+		// Just in case, set to close on exec
+		fcntl(*client, F_SETFD, FD_CLOEXEC);
 		pthread_t thread;
 		xpthread_create(&thread, NULL, request_handler, client);
 		// Detach the thread, we will never join it
@@ -194,7 +201,7 @@ int connect_daemon() {
 		 * since there is no clear entry point when the daemon should be started
 		 */
 		LOGD("client: connect fail, try launching new daemon process\n");
-		start_daemon();
+		start_daemon(fd);
 		do {
 			// Wait for 10ms
 			usleep(10);

jni/daemon/daemon.h

@@ -37,7 +37,7 @@ typedef enum {
 
 // daemon.c
 
-void start_daemon();
+void start_daemon(int client);
 int connect_daemon();
 
 // socket_trans.c

jni/daemon/log_monitor.c

@@ -8,6 +8,7 @@
 #include <stdio.h>
 #include <limits.h>
 #include <pthread.h>
+#include <unistd.h>
 
 #include "magisk.h"
 #include "utils.h"
@@ -17,18 +18,22 @@ static void *logger_thread(void *args) {
 	// Setup error handler
 	err_handler = exit_thread;
 
-	char buffer[PATH_MAX];
+	char *buffer = xmalloc(PATH_MAX);
 	rename(LOGFILE, LASTLOG);
-	FILE *logfile = xfopen(LOGFILE, "w");
+	FILE *logfile = xfdopen(xopen(LOGFILE, O_WRONLY | O_CREAT | O_CLOEXEC | O_TRUNC, 0644), "w");
 	// Disable buffering
 	setbuf(logfile, NULL);
 	// Start logcat
 	char *const command[] = { "logcat", "-s", "Magisk", "-v", "time", NULL };
 	int fd;
 	run_command(&fd, "/system/bin/logcat", command);
-	while (fdgets(buffer, sizeof(buffer), fd)) {
+	while (fdgets(buffer, PATH_MAX, fd)) {
 		fprintf(logfile, "%s", buffer);
 	}
+	// Should never be here, but well...
+	free(buffer);
+	close(fd);
+	fclose(logfile);
 	return NULL;
 }
 

jni/magiskhide/hide_daemon.c

@@ -57,7 +57,6 @@ int hide_daemon() {
 	err_handler = hide_daemon_err;
 	
 	int fd;
-	FILE *fp;
 	char buffer[4096], cache_block[256], *line;
 	struct vector mount_list;
 	

jni/magiskhide/proc_monitor.c

@@ -20,6 +20,7 @@
 static int zygote_num = 0;
 static char init_ns[32], zygote_ns[2][32];
 static int log_pid = 0, log_fd;
+static char *buffer;
 
 static void read_namespace(const int pid, char* target, const size_t size) {
 	char path[32];
@@ -31,8 +32,9 @@ static void read_namespace(const int pid, char* target, const size_t size) {
 static void quit_pthread(int sig) {
 	LOGD("proc_monitor: running cleanup\n");
 	destroy_list();
+	free(buffer);
 	hideEnabled = 0;
-	// Kill the logging if possible
+	// Kill the logging if needed
 	if (log_pid) {
 		kill(log_pid, SIGTERM);
 		waitpid(log_pid, NULL, 0);
@@ -45,6 +47,7 @@ static void quit_pthread(int sig) {
 	close(sv[0]);
 	waitpid(hide_pid, NULL, 0);
 	pthread_mutex_destroy(&hide_lock);
+	pthread_mutex_destroy(&file_lock);
 	LOGD("proc_monitor: terminating...\n");
 	pthread_exit(NULL);
 }
@@ -65,12 +68,16 @@ static void proc_monitor_err() {
 
 void proc_monitor() {
 	// Register the cancel signal
-	signal(SIGUSR1, quit_pthread);
+	struct sigaction act;
+	memset(&act, 0, sizeof(act));
+	act.sa_handler = quit_pthread;
+	sigaction(SIGPIPE, &act, NULL);
+
 	// The error handler should stop magiskhide services
 	err_handler = proc_monitor_err;
 
 	int pid;
-	char buffer[4096];
+	buffer = xmalloc(PATH_MAX);
 
 	// Get the mount namespace of init
 	read_namespace(1, init_ns, 32);
@@ -98,7 +105,7 @@ void proc_monitor() {
 	char *const command[] = { "logcat", "-b", "events", "-v", "raw", "-s", "am_proc_start", NULL };
 	log_pid = run_command(&log_fd, "/system/bin/logcat", command);
 
-	while(fdgets(buffer, sizeof(buffer), log_fd)) {
+	while(fdgets(buffer, PATH_MAX, log_fd)) {
 		int ret, comma = 0;
 		char *pos = buffer, *line, processName[256];
 

jni/su

@@ -1 +1 @@
-Subproject commit 9a1043574a0109e98872bc6e3fe2dcd2f406849a
+Subproject commit 5a41e8882a8762e6dc1815e49002776b634a4ea1

jni/utils/misc.c

@@ -221,6 +221,8 @@ int run_command(int *fd, const char *path, char *const argv[]) {
 		if (socketpair(AF_LOCAL, SOCK_STREAM, 0, sv) == -1)
 			return -1;
 		// We use sv[0], give them sv[1] for communication
+		if (fcntl(sv[1], F_SETFD, FD_CLOEXEC))
+			PLOGE("fcntl FD_CLOEXEC");
 		*fd = sv[1];
 	}
 

jni/utils/utils.h

@@ -23,6 +23,7 @@ extern int quit_signals[];
 // xwrap.c
 
 FILE *xfopen(const char *pathname, const char *mode);
+FILE *xfdopen(int fd, const char *mode);
 #define GET_MACRO(_1, _2, _3, NAME, ...) NAME
 #define xopen(...) GET_MACRO(__VA_ARGS__, xopen3, xopen2)(__VA_ARGS__)
 int xopen2(const char *pathname, int flags);

jni/utils/xwrap.c

@@ -32,6 +32,14 @@ FILE *xfopen(const char *pathname, const char *mode) {
 	return fp;
 }
 
+FILE *xfdopen(int fd, const char *mode) {
+	FILE *fp = fdopen(fd, mode);
+	if (fp == NULL) {
+		PLOGE("fopen");
+	}
+	return fp;
+}
+
 int xopen2(const char *pathname, int flags) {
 	int fd = open(pathname, flags);
 	if (fd < 0) {