Merge pull request #71 from mgerstner/bugfixes

Bugfixes and Hardening

daemon/cpugovctl.c

@@ -35,15 +35,18 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "logging.h"
 
 #include <ctype.h>
+#include <errno.h>
 #include <sys/types.h>
 
 /**
  * Sets all governors to a value
  */
-static void set_gov_state(const char *value)
+static int set_gov_state(const char *value)
 {
 	char governors[MAX_GOVERNORS][MAX_GOVERNOR_LENGTH] = { { 0 } };
 	int num = fetch_governors(governors);
+	int retval = EXIT_SUCCESS;
+	int res = 0;
 
 	LOG_MSG("Setting governors to %s\n", value);
 	for (int i = 0; i < num; i++) {
@@ -54,9 +57,15 @@ static void set_gov_state(const char *value)
 			continue;
 		}
 
-		fprintf(f, "%s\n", value);
+		res = fprintf(f, "%s\n", value);
+		if (res < 0) {
+			LOG_ERROR("Failed to set governor %s to %s: %s", gov, value, strerror(errno));
+			retval = EXIT_FAILURE;
+		}
 		fclose(f);
 	}
+
+	return retval;
 }
 
 /**
@@ -64,14 +73,9 @@ static void set_gov_state(const char *value)
  */
 int main(int argc, char *argv[])
 {
-	if (argc < 2) {
-		fprintf(stderr, "usage: cpugovctl [get] [set VALUE]\n");
-		return EXIT_FAILURE;
-	}
-
-	if (strncmp(argv[1], "get", 3) == 0) {
+	if (argc == 2 && strncmp(argv[1], "get", 3) == 0) {
 		printf("%s", get_gov_state());
-	} else if (strncmp(argv[1], "set", 3) == 0) {
+	} else if (argc == 3 && strncmp(argv[1], "set", 3) == 0) {
 		const char *value = argv[2];
 
 		/* Must be root to set the state */
@@ -80,8 +84,9 @@ int main(int argc, char *argv[])
 			return EXIT_FAILURE;
 		}
 
-		set_gov_state(value);
+		return set_gov_state(value);
 	} else {
+		fprintf(stderr, "usage: cpugovctl [get] [set VALUE]\n");
 		return EXIT_FAILURE;
 	}
 

daemon/daemonize.c

@@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "daemonize.h"
 #include "logging.h"
 
+#include <fcntl.h>
 #include <stdlib.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -61,14 +62,20 @@ void daemonize(const char *name)
 	}
 
 	/* Now continue execution */
-	umask(0);
+	umask(0022);
 	if (setsid() < 0) {
 		FATAL_ERRORNO("Failed to create process group\n");
 	}
 	if (chdir("/") < 0) {
 		FATAL_ERRORNO("Failed to change to root directory\n");
 	}
-	close(STDIN_FILENO);
-	close(STDOUT_FILENO);
-	close(STDERR_FILENO);
+
+	/* replace standard file descriptors by /dev/null */
+	int devnull_r = open("/dev/null", O_RDONLY);
+	int devnull_w = open("/dev/null", O_WRONLY);
+	dup2(devnull_r, STDIN_FILENO);
+	dup2(devnull_w, STDOUT_FILENO);
+	dup2(devnull_w, STDERR_FILENO);
+	close(devnull_r);
+	close(devnull_w);
 }

daemon/gamemode.c

@@ -381,6 +381,12 @@ bool game_mode_context_register(GameModeContext *self, pid_t client)
 	/* Construct a new client if we can */
 	GameModeClient *cl = NULL;
 
+	/* Cap the total number of active clients */
+	if (game_mode_context_num_clients(self) + 1 > MAX_GAMES) {
+		LOG_ERROR("Max games (%d) reached, not registering %d\n", MAX_GAMES, client);
+		return false;
+	}
+
 	cl = game_mode_client_new(client);
 	if (!cl) {
 		fputs("OOM\n", stderr);
@@ -390,22 +396,16 @@ bool game_mode_context_register(GameModeContext *self, pid_t client)
 
 	if (game_mode_context_has_client(self, client)) {
 		LOG_ERROR("Addition requested for already known process [%d]\n", client);
-		return false;
-	}
-
-	/* Cap the total number of active clients */
-	if (game_mode_context_num_clients(self) + 1 > MAX_GAMES) {
-		LOG_ERROR("Max games (%d) reached, not registering %d\n", MAX_GAMES, client);
-		return false;
+		goto error_cleanup;
 	}
 
 	/* Check our blacklist and whitelist */
 	if (!config_get_client_whitelisted(self->config, cl->executable)) {
 		LOG_MSG("Client [%s] was rejected (not in whitelist)\n", cl->executable);
-		return false;
+		goto error_cleanup;
 	} else if (config_get_client_blacklisted(self->config, cl->executable)) {
 		LOG_MSG("Client [%s] was rejected (in blacklist)\n", cl->executable);
-		return false;
+		goto error_cleanup;
 	}
 
 	/* Begin a write lock now to insert our new client at list start */
@@ -427,6 +427,9 @@ bool game_mode_context_register(GameModeContext *self, pid_t client)
 	game_mode_apply_scheduler(self, client);
 
 	return true;
+error_cleanup:
+	game_mode_client_free(cl);
+	return false;
 }
 
 bool game_mode_context_unregister(GameModeContext *self, pid_t client)