Simplify su_info cache

The previous implementation is great if multiple different requesters call su rapidly in a very short period of time, however in the real world this is nearly impossible to happen. This comes with quite a big overhead, since it requires two lists and also an everlasting background thread to constantly maintain the lists.

The new implementation will spawn a collector thread for each cache miss, and the thread will terminate itself once the data is invalidated.

su.h

@@ -28,7 +28,7 @@ struct su_info {
 	struct db_settings dbs;
 	struct db_strings str;
 	struct su_access access;
-	struct stat st;
+	struct stat manager_stat;
 
 	/* These should be guarded with global list lock */
 	struct list_head pos;

su_daemon.c

@@ -33,12 +33,9 @@
 #define UNLOCK_LIST()  pthread_mutex_unlock(&list_lock)
 #define UNLOCK_UID()   pthread_mutex_unlock(&ctx.info->lock)
 
-static struct list_head active_list, waiting_list;
+static struct list_head info_cache = { .prev = &info_cache, .next = &info_cache };
-static pthread_t su_collector = 0;
 static pthread_mutex_t list_lock = PTHREAD_MUTEX_INITIALIZER;
 
-int pipefd[2];
-
 static void sighandler(int sig) {
 	restore_stdin();
 
@@ -62,29 +59,20 @@ static void sighandler(int sig) {
 	}
 }
 
-// Maintain the lists periodically
+static void *info_collector(void *node) {
-static void *collector(void *args) {
+	struct su_info *info = node;
-	LOGD("su: collector started\n");
+	while (1) {
-	struct su_info *node;
-	while(1) {
 		sleep(1);
-		LOCK_LIST();
+		if (info->clock && --info->clock == 0) {
-		list_for_each(node, &active_list, struct su_info, pos) {
+			LOCK_LIST();
-			if (--node->clock == 0) {
+			list_pop(&info->pos);
-				// Timeout, move to waiting list
+			UNLOCK_LIST();
-				__ = list_pop(&node->pos);
-				list_insert_end(&waiting_list, &node->pos);
-			}
 		}
-		list_for_each(node, &waiting_list, struct su_info, pos) {
+		if (!info->clock && !info->ref) {
-			if (node->ref == 0) {
+			pthread_mutex_destroy(&info->lock);
-				// Nothing is using the info, remove it
+			free(info);
-				__ = list_pop(&node->pos);
+			return NULL;
-				pthread_mutex_destroy(&node->lock);
-				free(node);
-			}
 		}
-		UNLOCK_LIST();
 	}
 }
 
@@ -118,7 +106,7 @@ static void database_check(struct su_info *info) {
 
 	// We need to check our manager
 	if (info->access.log || info->access.notify)
-		validate_manager(info->str.s[SU_REQUESTER], uid / 100000, &info->st);
+		validate_manager(info->str.s[SU_REQUESTER], uid / 100000, &info->manager_stat);
 }
 
 static struct su_info *get_su_info(unsigned uid) {
@@ -126,22 +114,18 @@ static struct su_info *get_su_info(unsigned uid) {
 
 	LOCK_LIST();
 
-	if (!su_collector) {
+	// Search for existing info in cache
-		init_list_head(&active_list);
+	list_for_each(node, &info_cache, struct su_info, pos) {
-		init_list_head(&waiting_list);
-		xpthread_create(&su_collector, NULL, collector, NULL);
-	}
-
-	// Search for existing in the active list
-	list_for_each(node, &active_list, struct su_info, pos) {
 		if (node->uid == uid) {
 			info = node;
 			break;
 		}
 	}
 
-	// If no exist, create a new request
+	int cache_miss = info == NULL;
-	if (info == NULL) {
+
+	if (cache_miss) {
+		// If cache miss, create a new one and push to cache
 		info = malloc(sizeof(*info));
 		info->uid = uid;
 		info->dbs = DEFAULT_DB_SETTINGS;
@@ -150,10 +134,19 @@ static struct su_info *get_su_info(unsigned uid) {
 		info->ref = 0;
 		info->count = 0;
 		pthread_mutex_init(&info->lock, NULL);
-		list_insert_end(&active_list, &info->pos);
+		list_insert_end(&info_cache, &info->pos);
+	}
+
+	// Update the cache status
+	info->clock = TIMEOUT;
+	++info->ref;
+
+	// Start a thread to maintain the info cache
+	if (cache_miss) {
+		pthread_t thread;
+		xpthread_create(&thread, NULL, info_collector, info);
+		pthread_detach(thread);
 	}
-	info->clock = TIMEOUT;  /* Reset timer */
-	++info->ref;  /* Increment reference count */
 
 	UNLOCK_LIST();
 
@@ -190,7 +183,7 @@ static struct su_info *get_su_info(unsigned uid) {
 		}
 
 		// If it's the manager, allow it silently
-		if ((info->uid % 100000) == (info->st.st_uid % 100000))
+		if ((info->uid % 100000) == (info->manager_stat.st_uid % 100000))
 			info->access = SILENT_SU_ACCESS;
 
 		// Allow if it's root
@@ -249,6 +242,9 @@ void su_daemon_receiver(int client, struct ucred *credential) {
 		// The policy is determined, unlock
 		UNLOCK_UID();
 
+		// Info is now useless to us, decrement reference count
+		--ctx.info->ref;
+
 		// Wait result
 		LOGD("su: waiting child: [%d]\n", child);
 		int status, code;
@@ -276,16 +272,10 @@ void su_daemon_receiver(int client, struct ucred *credential) {
 		act.sa_handler = SIG_DFL;
 		sigaction(SIGPIPE, &act, NULL);
 
-		// Decrement reference count
-		LOCK_LIST();
-		--ctx.info->ref;
-		UNLOCK_LIST();
-
 		return;
 	}
 
 	LOGD("su: child process started\n");
-	UNLOCK_UID();
 
 	// ack
 	write_int(client, 0);

su_socket.c

@@ -44,7 +44,7 @@ int socket_create_temp(char *path, size_t len) {
 
     // Set attributes so requester can access it
     setfilecon(path, "u:object_r:"SEPOL_FILE_DOMAIN":s0");
-    chown(path, su_ctx->info->st.st_uid, su_ctx->info->st.st_gid);
+    chown(path, su_ctx->info->manager_stat.st_uid, su_ctx->info->manager_stat.st_gid);
 
     return fd;
 }