Fix duplicate process detection

src/daemon.c

@@ -109,12 +109,22 @@ void go_daemon(void)
 	// Closing stdin, stdout and stderr is handled by dnsmasq
 }
 
+/**
+ * @brief Save the current process ID (PID) to a file.
+ *
+ * This function retrieves the PID of the current process and writes it to a
+ * specified file. If the file cannot be opened for writing, an error is logged.
+ * Otherwise, the PID is written to the file and the file is closed. The PID is
+ * also logged for informational purposes.
+ *
+ * @return void
+ */
 void savepid(void)
 {
-	FILE *f;
 	// Get PID of the current process
 	const pid_t pid = getpid();
 	// Open file for writing
+	FILE *f = NULL;
 	if((f = fopen(config.files.pid.v.s, "w+")) == NULL)
 	{
 		// Log error
@@ -129,20 +139,73 @@ void savepid(void)
 	log_info("PID of FTL process: %i", (int)pid);
 }
 
+/**
+ * @brief Reads the process ID (PID) from a file.
+ *
+ * This function attempts to open a file specified by the configuration
+ * and read the PID from it. If the file cannot be opened or the PID
+ * cannot be parsed, appropriate warnings are logged and the function
+ * returns -1.
+ *
+ * @return pid_t The PID read from the file on success, or -1 on failure.
+ */
+pid_t readpid(void)
+{
+	pid_t pid = -1;
+	FILE *f = NULL;
+	// Open file for reading
+	if((f = fopen(config.files.pid.v.s, "r")) == NULL)
+	{
+		// Log error
+		log_warn("Unable to read PID from file: %s", strerror(errno));
+		return -1;
+	}
+
+	// Try to read PID from file if it is not empty
+	if(fscanf(f, "%d", &pid) != 1)
+		log_debug(DEBUG_SHMEM, "Unable to parse PID in PID file");
+
+	// Close file
+	fclose(f);
+
+	return pid;
+}
+
+/**
+ * @brief Empties the PID file and remove it
+ *
+ * This function opens the PID file in write mode, which effectively
+ * empties its contents. If the file cannot be opened, a warning is logged.
+ *
+ * @note This function does not remove the PID file, it only empties it.
+ */
 static void removepid(void)
 {
-	// Note that this function is not really removing the PID file but
-	// rather emptying it
-	FILE *f;
-	// Open file for writing (emptying it)
+	FILE *f = NULL;
+	// Open file for writing to overwrite/empty it
 	if((f = fopen(config.files.pid.v.s, "w")) == NULL)
 	{
 		log_warn("Unable to empty PID file: %s", strerror(errno));
 		return;
 	}
 	fclose(f);
+
+	// Remove PID file
+	if(unlink(config.files.pid.v.s) != 0)
+		log_warn("Unable to remove PID file: %s", strerror(errno));
 }
 
+/**
+ * @brief Retrieves the username of the effective user ID of the calling process.
+ *
+ * This function uses the `geteuid()` function to get the effective user ID (EUID) of the calling process
+ * and then searches the user database for an entry with a matching UID using the `getpwuid()` function.
+ * If a matching entry is found, the username is returned. If no matching entry is found, the UID is
+ * returned as a string. If an error occurs during the lookup, a warning is logged.
+ *
+ * @return A dynamically allocated string containing the username or UID. The caller is responsible for
+ * freeing the allocated memory. Returns NULL if memory allocation fails.
+ */
 char *getUserName(void)
 {
 	char *name;

src/daemon.h

@@ -15,6 +15,7 @@ extern pthread_t threads[THREADS_MAX];
 
 void go_daemon(void);
 void savepid(void);
+pid_t readpid(void);
 char *getUserName(void);
 const char *hostname(void);
 const char *domainname(void);

src/procps.c

@@ -17,6 +17,8 @@
 #include <sys/times.h>
 // config
 #include "config/config.h"
+// readpid()
+#include "daemon.h"
 
 #define PROCESS_NAME   "pihole-FTL"
 
@@ -73,223 +75,49 @@ bool get_process_name(const pid_t pid, char name[PROC_PATH_SIZ])
 	return true;
 }
 
-// This function tries to obtain the parent process ID of a given PID
-// It returns true on success, false otherwise and stores the parent PID in
-// the given pid_t pointer
-static bool get_process_ppid(const pid_t pid, pid_t *ppid)
-{
-	// Try to open status file
-	char filename[sizeof("/proc/%u/task/%u/status") + sizeof(int)*3 * 2];
-	snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
-	FILE *f = fopen(filename, "r");
-	if(f == NULL)
-		return false;
-
-	// Read comm from opened file
-	char buffer[128];
-	while(fgets(buffer, sizeof(buffer), f) != NULL)
-	{
-		if(sscanf(buffer, "PPid: %d\n", ppid) == 1)
-			break;
-	}
-	fclose(f);
-
-	return true;
-}
-
-// This function tries to obtain the process creation time of a given PID
-// It returns true on success, false otherwise and stores the creation time in
-// the given buffer
-static bool get_process_creation_time(const pid_t pid, char timestr[TIMESTR_SIZE])
-{
-	// Try to open comm file
-	char filename[sizeof("/proc/%u/task/%u/comm") + sizeof(int)*3 * 2];
-	snprintf(filename, sizeof(filename), "/proc/%d/comm", pid);
-	struct stat st;
-	if(stat(filename, &st) < 0)
-		return false;
-	get_timestr(timestr, st.st_ctim.tv_sec, false, false);
-
-	return true;
-}
-
-// This function checks if a given PID is running inside a docker container
-static bool is_in_docker(const pid_t pid)
-{
-	char filename[sizeof("/proc/%u/cgroup") + sizeof(int)*3];
-	snprintf(filename, sizeof(filename), "/proc/%d/cgroup", pid);
-
-	FILE *f = fopen(filename, "r");
-	if(f == NULL)
-		return false;
-
-	char buffer[128];
-	while(fgets(buffer, sizeof(buffer), f) != NULL)
-	{
-		if(strstr(buffer, "/docker") != NULL)
-		{
-			fclose(f);
-			return true;
-		}
-	}
-	fclose(f);
-
-	return false;
-}
-
 // This function prints an info message about if another FTL process is already
 // running. It returns true if another FTL process is already running, false
 // otherwise.
 bool another_FTL(void)
 {
-	DIR *dirPos;
-	struct dirent *entry;
 	const pid_t ourselves = getpid();
 	bool already_running = false;
-	pid_t pid = 0;
+	pid_t pid = readpid();
 
-	// First we try to read the PID file and compare the PID in there with
-	// our own PID. If the PID file does not exist or does not contain our
-	// PID, we try to find another FTL process by looking at the process
-	// list further down.
-	if(config.files.pid.v.s != NULL)
+	if(pid == ourselves)
 	{
-		FILE *pidFile = fopen(config.files.pid.v.s, "r");
-		if(pidFile != NULL)
-		{
-			if(fscanf(pidFile, "%d", &pid) == 1)
-			{
-				if(pid == ourselves)
-				{
-					log_debug(DEBUG_SHMEM, "PID file contains our own PID");
-				}
-				else
-				{
-					// Note: kill(pid, 0) does not send a
-					// signal, but merely checks if the
-					// process exists. If the process does
-					// not exist, kill() returns -1 and sets
-					// errno to ESRCH. However, if the
-					// process exists, but security
-					// restrictions tell the system to deny
-					// its existence, we cannot distinguish
-					// between the process not existing and
-					// the process existing but being denied
-					// to us. In that case, our fallback
-					// solution below kicks in and iterates
-					// over /proc instead.
-					already_running = kill(pid, 0) == 0;
-					log_debug(DEBUG_SHMEM, "PID file contains PID %d (%s), we are %d",
-					          pid, already_running ? "running" : "dead", ourselves);
-				}
-			}
-			else
-			{
-				log_debug(DEBUG_SHMEM, "Failed to parse PID in PID file: %s",
-				          strerror(errno));
-			}
-			fclose(pidFile);
-		}
-		else
-		{
-			log_debug(DEBUG_SHMEM, "Failed to open PID file \"%s\": %s",
-			          config.files.pid.v.s, strerror(errno));
-		}
+		log_info("PID file contains our own PID");
 	}
-
-	// If already_running is true, we are done
-	if(already_running)
+	else if(pid < 0)
 	{
-		log_crit("%s is already running (PID %d)!", PROCESS_NAME, pid);
-		return true;
+		log_info("PID file does not exist or not readable");
 	}
-
-	// If the PID file does not contain our own PID, we try to find a running
-	// process with the same name as our own process
-	// Open /proc
-	errno = 0;
-	if ((dirPos = opendir("/proc")) == NULL)
+	else
 	{
-		log_warn("Failed to access /proc: %s", strerror(errno));
-		return false;
+		// Note: kill(pid, 0) does not send a signal, but merely checks
+		// if the process exists. If the process does not exist, kill()
+		// returns -1 and sets errno to ESRCH. However, if the process
+		// exists, but security restrictions tell the system to deny its
+		// existence, we cannot distinguish between the process not
+		// existing and the process existing but being denied to us. In
+		// that case, our fallback solution below kicks in and iterates
+		// over /proc instead.
+		already_running = kill(pid, 0) == 0;
+		log_info("PID file contains PID %d (%s), we are %d",
+		         pid, already_running ? "running" : "dead", ourselves);
 	}
 
-	// Loop over entries in /proc
-	// This is much more efficient than iterating over all possible PIDs
-	pid_t last_pid = 0;
-	size_t last_len = 0u;
-	log_debug(DEBUG_SHMEM, "Reading /proc/[0-9]*");
-	while ((entry = readdir(dirPos)) != NULL)
+	// If already_running is true, we are done
+	if(already_running)
 	{
-		// We are only interested in subdirectories of /proc
-		if(entry->d_type != DT_DIR)
-			continue;
-		// We are only interested in PID subdirectories
-		if(entry->d_name[0] < '0' || entry->d_name[0] > '9')
-			continue;
-
-		// Extract PID
-		pid = strtol(entry->d_name, NULL, 10);
-
-		// Get process name
-		char name[PROC_PATH_SIZ] = { 0 };
-		if(!get_process_name(pid, name))
-			continue;
-
-		log_debug(DEBUG_SHMEM, "PID: %d -> name: %s%s", pid, name, pid == ourselves ? " (us)" : "");
-
-		// Skip our own process
-		if(pid == ourselves)
-			continue;
-
-		// Only process this if this is our own process
-		if(strcasecmp(name, PROCESS_NAME) != 0)
-			continue;
-
-		// Get parent process ID (PPID)
-		pid_t ppid;
-		if(!get_process_ppid(pid, &ppid))
-			continue;
-		char ppid_name[PROC_PATH_SIZ] = { 0 };
-		if(!get_process_name(ppid, ppid_name))
-			continue;
-
-		// Skip if this is an instance running inside a docker container
-		if(is_in_docker(pid))
-			continue;
-
-		log_debug(DEBUG_SHMEM, " └ PPID: %d -> name: %s", ppid, ppid_name);
-
-		char timestr[TIMESTR_SIZE] = { 0 };
-		get_process_creation_time(pid, timestr);
-
-		// If this is the first process we log, add a header
-		if(!already_running)
-		{
-			already_running = true;
-			log_crit("%s is already running!", PROCESS_NAME);
-		}
-
-		if(last_pid != ppid)
-		{
-			// Independent process, may be child of init/systemd
-			log_info("%s (PID %d) ──> %s (PID %d, started %s)",
-			         ppid_name, ppid, name, pid, timestr);
-			last_pid = pid;
-			last_len = snprintf(NULL, 0, "%s (PID %d) ──> ", ppid_name, ppid);
-		}
-		else
-		{
-			// Process parented by the one we analyzed before,
-			// highlight their relationship
-			log_info("%*s └─> %s (PID %d, started %s)",
-			         (int)last_len, "", name, pid, timestr);
-		}
+		log_crit("%s is already running (PID %d)!", PROCESS_NAME, pid);
+		return true;
 	}
-	log_debug(DEBUG_SHMEM, "Done reading /proc/[0-9]*");
 
-	closedir(dirPos);
-	return already_running;
+	// If we did not find another FTL process by looking at the PID file, we assume
+	// no other FTL process is running. We write our own PID to the file later after
+	// we have successfully started up (and possibly forked).
+	return false;
 }
 
 bool getProcessMemory(struct proc_mem *mem, const unsigned long total_memory)