From bb1e62e40c1272ef681b68323d228cbcb895af9826c324d2b4a8146ef268f84a Mon Sep 17 00:00:00 2001 From: Matthias Bach Date: Mon, 13 Aug 2018 20:51:46 +0000 Subject: [PATCH] - Add hardening.patch: This backports bugfixes and hardenings resulting from the Factory-inclusion security review and already accepted by upstream. - Stop suppressing linter warning for the PolicyKit priviledge escalation. This has been properly handled via boo#1093979 now. - Suppress false positive lintian caused by a % symbol in the description of the libgamemodeauto library. OBS-URL: https://build.opensuse.org/package/show/games:tools/gamemode?expand=0&rev=4 --- gamemode-rpmlintrc | 7 +- gamemode.changes | 11 +++ gamemode.spec | 2 + hardening.patch | 165 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 hardening.patch diff --git a/gamemode-rpmlintrc b/gamemode-rpmlintrc index 82616db..0959082 100644 --- a/gamemode-rpmlintrc +++ b/gamemode-rpmlintrc @@ -1,6 +1,5 @@ -# The PolicyKit priviledge escalation is desired. It does not make sense to open a bug for this while not requesting Factory inclusion, yet. -addFilter('gamemoded.* polkit-unauthorized-privilege.*') -setBadness('polkit-unauthorized-privilege', 0) - # Libgamemodeauto loads the dependency via dlopen, so it cannot be picked up automatically. addFilter('libgamemodeauto.* explicit-lib-dependency libgamemode') + +# Libgamemodeauto actually has a % in its description. +addFilter('libgamemodeauto.* unexpanded-macro %description -l C %command') diff --git a/gamemode.changes b/gamemode.changes index 4edd9bd..c66bfcc 100644 --- a/gamemode.changes +++ b/gamemode.changes @@ -1,3 +1,14 @@ +------------------------------------------------------------------- +Mon Aug 13 20:47:23 UTC 2018 - marix@marix.org + +- Add hardening.patch: This backports bugfixes and hardenings + resulting from the Factory-inclusion security review and already + accepted by upstream. +- Stop suppressing linter warning for the PolicyKit priviledge + escalation. This has been properly handled via boo#1093979 now. +- Suppress false positive lintian caused by a % symbol in the + description of the libgamemodeauto library. + ------------------------------------------------------------------- Tue Jul 24 21:11:08 UTC 2018 - marix@marix.org diff --git a/gamemode.spec b/gamemode.spec index 33f1082..4a30858 100644 --- a/gamemode.spec +++ b/gamemode.spec @@ -25,6 +25,7 @@ Url: https://github.com/FeralInteractive/gamemode Source0: gamemode-%{version}.tar.xz Source1: gamemode-rpmlintrc Source2: README.openSUSE +Patch0: hardening.patch BuildRequires: meson BuildRequires: ninja BuildRequires: pkg-config @@ -89,6 +90,7 @@ This package contains the headers required to compile games with built-in GameMo %prep %setup -q +%patch -P 0 -p1 cp %{SOURCE2} . %build diff --git a/hardening.patch b/hardening.patch new file mode 100644 index 0000000..1956d46 --- /dev/null +++ b/hardening.patch @@ -0,0 +1,165 @@ +From: Matthias Gerstner +Date: 2018-06-30 17:44:00 +0200 +Subject: Bugfixes and hardening from security review for Factory inclusion +References: boo#1093979 +Upstream: submitted + +diff --git a/daemon/cpugovctl.c b/daemon/cpugovctl.c +index 51b2e58..b64b5b4 100644 +--- a/daemon/cpugovctl.c ++++ b/daemon/cpugovctl.c +@@ -35,15 +35,18 @@ POSSIBILITY OF SUCH DAMAGE. + #include "logging.h" + + #include ++#include + #include + + /** + * 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; + } + +diff --git a/daemon/daemonize.c b/daemon/daemonize.c +index 5a68cbf..cc475f3 100644 +--- a/daemon/daemonize.c ++++ b/daemon/daemonize.c +@@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE. + #include "daemonize.h" + #include "logging.h" + ++#include + #include + #include + #include +@@ -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); + } +diff --git a/daemon/gamemode.c b/daemon/gamemode.c +index 9a8380d..63c02f7 100644 +--- a/daemon/gamemode.c ++++ b/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)