Accepting request 693112 from home:Freigeist_A:branches:games:tools

- Update to version 1.3.1
  * Disables screensaver when the game is running.
  * New `gamemoderun` script to run games in GameMode which don't
    support it themselves.
  * Add GPU managment capabilities.
- Removed hardening.patch already included in 1.3.
- Add gpuctl-fixes.patch hardening the new GPU features.

OBS-URL: https://build.opensuse.org/request/show/693112
OBS-URL: https://build.opensuse.org/package/show/games:tools/gamemode?expand=0&rev=12
This commit is contained in:
Matthias Bach 2019-04-18 18:16:22 +00:00 committed by Git OBS Bridge
parent bef96deeab
commit b258109d4f
8 changed files with 665 additions and 173 deletions

View File

@ -2,7 +2,7 @@
<service name="obs_scm" mode="disabled">
<param name="url">https://github.com/FeralInteractive/gamemode.git</param>
<param name="scm">git</param>
<param name="revision">1.2</param>
<param name="revision">1.3.1</param>
<param name="versionformat">@PARENT_TAG@</param>
<param name="changesgenerate">enable</param>
</service>

View File

@ -1,4 +1,4 @@
<servicedata>
<service name="tar_scm">
<param name="url">https://github.com/FeralInteractive/gamemode.git</param>
<param name="changesrevision">ceb476052d10b945d16bc4bcf5381e69c6722c96</param></service></servicedata>
<param name="changesrevision">bc20bef283fb947d7cee23306860af04f723ae9f</param></service></servicedata>

View File

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:6ea1f7709ab8a7a1c8d0a4e6dfa7858e3a530c4bde1edd4728bec5b617f03bbb
size 38336

3
gamemode-1.3.1.tar.xz Normal file
View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:b84ed18d7df4a35ba5a73659b5ee9504c3361e04c1d23ba78bf6507d11224fa4
size 55908

View File

@ -1,3 +1,14 @@
-------------------------------------------------------------------
Tue Apr 9 19:39:24 UTC 2019 - Matthias Bach <marix@marix.org>
- Update to version 1.3.1
* Disables screensaver when the game is running.
* New `gamemoderun` script to run games in GameMode which don't
support it themselves.
* Add GPU managment capabilities.
- Removed hardening.patch already included in 1.3.
- Add gpuctl-fixes.patch hardening the new GPU features.
-------------------------------------------------------------------
Sat Mar 16 19:34:04 UTC 2019 - Matthias Bach <marix@marix.org>

View File

@ -18,7 +18,7 @@
Name: gamemode
Version: 1.2
Version: 1.3.1
Release: 0
Summary: Daemon/library combo for changing Linux system performance on demand
License: BSD-3-Clause
@ -28,7 +28,7 @@ Source0: gamemode-%{version}.tar.xz
Source1: gamemode-rpmlintrc
Source2: README.openSUSE
Source3: baselibs.conf
Patch0: hardening.patch
Patch1: gpuctl-fixes.patch
BuildRequires: meson
BuildRequires: ninja
BuildRequires: pkg-config
@ -109,7 +109,7 @@ built-in GameMode support.
%prep
%setup -q
%patch -P 0 -p1
%patch1 -p1
cp %{SOURCE2} .
%build
@ -136,7 +136,9 @@ export CC=gcc-7 # gcc4.8 does not work because of https://gcc.gnu.org/bugzilla/
%files -n gamemoded
%defattr(-,root,root)
%{_bindir}/gamemoded
%{_bindir}/gamemoderun
%{_libexecdir}/cpugovctl
%{_libexecdir}/gpuclockctl
%{_userunitdir}/gamemoded.service
%{_datadir}/polkit-1/actions/com.feralinteractive.GameMode.policy
%{_datadir}/dbus-1/services/com.feralinteractive.GameMode.service

644
gpuctl-fixes.patch Normal file
View File

@ -0,0 +1,644 @@
From cbc6ce7e2677f9c3759423bfef5a03ff4ad7575c Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
Date: Wed, 3 Apr 2019 16:18:36 +0200
Subject: [PATCH 1/4] gpuclockctl: fix return value of get_gpu_state_amd()
---
daemon/gpuclockctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index 73c17c5..8598a9f 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -303,7 +303,7 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info)
/* Copy in the value from the file */
strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1);
info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0';
- return info == NULL;
+ return 0;
}
/*
From fd576682a3720a22e12e63014ec9f01621c6ed0f Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
Date: Wed, 3 Apr 2019 16:27:12 +0200
Subject: [PATCH 2/4] daemon: fix file descriptor leaks
---
daemon/daemon_config.c | 1 +
daemon/governors-query.c | 1 +
daemon/gpu-control.c | 5 ++++-
daemon/gpuclockctl.c | 26 +++++++++++++++++---------
4 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c
index 675c195..23e0e76 100644
--- a/daemon/daemon_config.c
+++ b/daemon/daemon_config.c
@@ -362,6 +362,7 @@ static void load_config_files(GameModeConfig *self)
if (error) {
LOG_MSG("Failed to parse config file - error on line %d!\n", error);
}
+ fclose(f);
}
free(path);
}
diff --git a/daemon/governors-query.c b/daemon/governors-query.c
index 9e0af0f..9d483c0 100644
--- a/daemon/governors-query.c
+++ b/daemon/governors-query.c
@@ -133,6 +133,7 @@ const char *get_gov_state(void)
/* Don't handle the mixed case, this shouldn't ever happen
* But it is a clear sign we shouldn't carry on */
LOG_ERROR("Governors malformed: got \"%s\", expected \"%s\"", contents, governor);
+ fclose(f);
return "malformed";
}
diff --git a/daemon/gpu-control.c b/daemon/gpu-control.c
index 147cdd1..a2699c0 100644
--- a/daemon/gpu-control.c
+++ b/daemon/gpu-control.c
@@ -50,7 +50,10 @@ enum GPUVendor gamemode_get_gpu_vendor(long device)
return Vendor_Invalid;
}
char buff[64];
- if (fgets(buff, 64, file) != NULL) {
+ bool got_line = fgets(buff, 64, file) != NULL;
+ fclose(file);
+
+ if (got_line) {
vendor = strtol(buff, NULL, 0);
} else {
LOG_ERROR("Coudn't read contents of file %s, will not apply optimisations!\n", path);
diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index 8598a9f..873ab54 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -289,21 +289,27 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info)
return -1;
}
+ int ret = 0;
+
char buff[CONFIG_VALUE_MAX] = { 0 };
if (!fgets(buff, CONFIG_VALUE_MAX, file)) {
LOG_ERROR("Could not read file %s (%s)!\n", path, strerror(errno));
- return -1;
+ ret = -1;
}
if (fclose(file) != 0) {
LOG_ERROR("Could not close %s after reading (%s)!\n", path, strerror(errno));
- return -1;
+ ret = -1;
}
- /* Copy in the value from the file */
- strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1);
- info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0';
- return 0;
+ if( ret == 0 )
+ {
+ /* Copy in the value from the file */
+ strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1);
+ info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0';
+ }
+
+ return ret;
}
/*
@@ -320,17 +326,19 @@ static int set_gpu_state_amd_file(const char *filename, long device, const char
return -1;
}
+ int ret = 0;
+
if (fprintf(file, "%s", value) < 0) {
LOG_ERROR("Could not write to %s (%s)!\n", path, strerror(errno));
- return -1;
+ ret = -1;
}
if (fclose(file) != 0) {
LOG_ERROR("Could not close %s after writing (%s)!\n", path, strerror(errno));
- return -1;
+ ret = -1;
}
- return 0;
+ return ret;
}
/**
From 54212fa81d80b7a6981faed09777e55ce8e86963 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
Date: Wed, 3 Apr 2019 16:46:38 +0200
Subject: [PATCH 3/4] gpuclockctl: refactor buffer size specification and avoid
redundancies
---
daemon/gpuclockctl.c | 116 ++++++++++++++++++++++++-------------------
1 file changed, 64 insertions(+), 52 deletions(-)
diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index 873ab54..cac49bd 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -36,6 +36,8 @@ POSSIBILITY OF SUCH DAMAGE.
#include "external-helper.h"
#include "gpu-control.h"
+#include <limits.h>
+
/* NV constants */
#define NV_CORE_OFFSET_ATTRIBUTE "GPUGraphicsClockOffset"
#define NV_MEM_OFFSET_ATTRIBUTE "GPUMemoryTransferRateOffset"
@@ -44,6 +46,7 @@ POSSIBILITY OF SUCH DAMAGE.
#define NV_PCIDEVICE_ATTRIBUTE "PCIDevice"
#define NV_ATTRIBUTE_FORMAT "[gpu:%ld]/%s"
#define NV_PERF_LEVEL_FORMAT "[%ld]"
+#define NV_ARG_MAX 128
/* AMD constants */
#define AMD_DRM_PATH "/sys/class/drm/card%ld/device/%s"
@@ -64,6 +67,30 @@ static void print_usage_and_exit(void)
exit(EXIT_FAILURE);
}
+static const char* get_nv_attr(const char *attr)
+{
+ static char out[EXTERNAL_BUFFER_MAX];
+ const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", attr, "-t", NULL };
+ if (run_external_process(exec_args, out, -1) != 0) {
+ LOG_ERROR("Failed to get %s!\n", attr);
+ out[0] = 0;
+ return NULL;
+ }
+
+ return &out[0];
+}
+
+static int set_nv_attr(const char *attr)
+{
+ const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", attr, NULL };
+ if (run_external_process(exec_args_core, NULL, -1) != 0) {
+ LOG_ERROR("Failed to set %s!\n", attr);
+ return -1;
+ }
+
+ return 0;
+}
+
/* Get the nvidia driver index for the current GPU */
static long get_gpu_index_id_nv(struct GameModeGPUInfo *info)
{
@@ -108,23 +135,21 @@ static long get_max_perf_level_nv(struct GameModeGPUInfo *info)
if (!getenv("DISPLAY"))
LOG_ERROR("Getting Nvidia parameters requires DISPLAY to be set - will likely fail!\n");
- char arg[128] = { 0 };
- char buf[EXTERNAL_BUFFER_MAX] = { 0 };
+ char arg[NV_ARG_MAX] = { 0 };
+ const char *attr;
- snprintf(arg, 128, NV_ATTRIBUTE_FORMAT, info->device, NV_PERFMODES_ATTRIBUTE);
- const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
- if (run_external_process(exec_args, buf, -1) != 0) {
- LOG_ERROR("Failed to get %s!\n", arg);
+ snprintf(arg, NV_ARG_MAX, NV_ATTRIBUTE_FORMAT, info->device, NV_PERFMODES_ATTRIBUTE);
+ if ((attr = get_nv_attr(arg)) == NULL) {
return -1;
}
- char *ptr = strrchr(buf, ';');
+ char *ptr = strrchr(attr, ';');
long level = -1;
if (!ptr || sscanf(ptr, "; perf=%ld", &level) != 1) {
LOG_ERROR(
"Output didn't match expected format, couldn't discern highest perf level from "
"nvidia-settings!\n");
- LOG_ERROR("Output:%s\n", buf);
+ LOG_ERROR("Output:%s\n", attr);
return -1;
}
@@ -142,59 +167,53 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
long perf_level = get_max_perf_level_nv(info);
- char arg[128] = { 0 };
- char buf[EXTERNAL_BUFFER_MAX] = { 0 };
+ char arg[NV_ARG_MAX] = { 0 };
+ const char *attr;
char *end;
/* Get the GPUGraphicsClockOffset parameter */
snprintf(arg,
- 128,
+ NV_ARG_MAX,
NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT,
info->device,
NV_CORE_OFFSET_ATTRIBUTE,
perf_level);
- const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
- if (run_external_process(exec_args_core, buf, -1) != 0) {
- LOG_ERROR("Failed to get %s!\n", arg);
+ if ((attr = get_nv_attr(arg)) == NULL) {
return -1;
}
- info->nv_core = strtol(buf, &end, 10);
- if (end == buf) {
- LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
+ info->nv_core = strtol(attr, &end, 10);
+ if (end == attr) {
+ LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr);
return -1;
}
/* Get the GPUMemoryTransferRateOffset parameter */
snprintf(arg,
- 128,
+ NV_ARG_MAX,
NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT,
info->device,
NV_MEM_OFFSET_ATTRIBUTE,
perf_level);
- const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
- if (run_external_process(exec_args_mem, buf, -1) != 0) {
- LOG_ERROR("Failed to get %s!\n", arg);
+ if ((attr = get_nv_attr(arg)) == NULL) {
return -1;
}
- info->nv_mem = strtol(buf, &end, 10);
- if (end == buf) {
- LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
+ info->nv_mem = strtol(attr, &end, 10);
+ if (end == attr) {
+ LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr);
return -1;
}
/* Get the GPUPowerMizerMode parameter */
- snprintf(arg, 128, NV_ATTRIBUTE_FORMAT, info->device, NV_POWERMIZER_MODE_ATTRIBUTE);
- const char *exec_args_pm[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
- if (run_external_process(exec_args_pm, buf, -1) != 0) {
- LOG_ERROR("Failed to get %s!\n", arg);
+ snprintf(arg, NV_ARG_MAX, NV_ATTRIBUTE_FORMAT, info->device, NV_POWERMIZER_MODE_ATTRIBUTE);
+ if ((attr = get_nv_attr(arg)) == NULL) {
return -1;
}
- info->nv_powermizer_mode = strtol(buf, &end, 10);
- if (end == buf) {
- LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
+ info->nv_powermizer_mode = strtol(attr, &end, 10);
+ if (end == attr) {
+ LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr);
return -1;
}
@@ -218,20 +237,18 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info)
long perf_level = get_max_perf_level_nv(info);
- char arg[128] = { 0 };
+ char arg[NV_ARG_MAX] = { 0 };
/* Set the GPUGraphicsClockOffset parameter */
if (info->nv_core != -1) {
snprintf(arg,
- 128,
+ NV_ARG_MAX,
NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT "=%ld",
info->device,
NV_CORE_OFFSET_ATTRIBUTE,
perf_level,
info->nv_core);
- const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL };
- if (run_external_process(exec_args_core, NULL, -1) != 0) {
- LOG_ERROR("Failed to set %s!\n", arg);
+ if (set_nv_attr(arg) != 0) {
status = -1;
}
}
@@ -239,15 +256,13 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info)
/* Set the GPUMemoryTransferRateOffset parameter */
if (info->nv_mem != -1) {
snprintf(arg,
- 128,
+ NV_ARG_MAX,
NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT "=%ld",
info->device,
NV_MEM_OFFSET_ATTRIBUTE,
perf_level,
info->nv_mem);
- const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL };
- if (run_external_process(exec_args_mem, NULL, -1) != 0) {
- LOG_ERROR("Failed to set %s!\n", arg);
+ if (set_nv_attr(arg) != 0) {
status = -1;
}
}
@@ -255,14 +270,12 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info)
/* Set the GPUPowerMizerMode parameter if requested */
if (info->nv_powermizer_mode != -1) {
snprintf(arg,
- 128,
+ NV_ARG_MAX,
NV_ATTRIBUTE_FORMAT "=%ld",
info->device,
NV_POWERMIZER_MODE_ATTRIBUTE,
info->nv_powermizer_mode);
- const char *exec_args_pm[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL };
- if (run_external_process(exec_args_pm, NULL, -1) != 0) {
- LOG_ERROR("Failed to set %s!\n", arg);
+ if (set_nv_attr(arg) != 0) {
status = -1;
}
}
@@ -280,8 +293,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info)
return -1;
/* Get the contents of power_dpm_force_performance_level */
- char path[64];
- snprintf(path, 64, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level");
+ char path[PATH_MAX];
+ snprintf(path, PATH_MAX, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level");
FILE *file = fopen(path, "r");
if (!file) {
@@ -317,8 +330,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info)
*/
static int set_gpu_state_amd_file(const char *filename, long device, const char *value)
{
- char path[64];
- snprintf(path, 64, AMD_DRM_PATH, device, filename);
+ char path[PATH_MAX];
+ snprintf(path, PATH_MAX, AMD_DRM_PATH, device, filename);
FILE *file = fopen(path, "w");
if (!file) {
@@ -398,10 +411,11 @@ static long get_generic_value(const char *val)
*/
int main(int argc, char *argv[])
{
+ struct GameModeGPUInfo info;
+ memset(&info, 0, sizeof(info));
+
if (argc == 3 && strncmp(argv[2], "get", 3) == 0) {
/* Get and verify the vendor and device */
- struct GameModeGPUInfo info;
- memset(&info, 0, sizeof(info));
info.device = get_device(argv[1]);
info.vendor = gamemode_get_gpu_vendor(info.device);
@@ -428,8 +442,6 @@ int main(int argc, char *argv[])
} else if (argc >= 4 && argc <= 7 && strncmp(argv[2], "set", 3) == 0) {
/* Get and verify the vendor and device */
- struct GameModeGPUInfo info;
- memset(&info, 0, sizeof(info));
info.device = get_device(argv[1]);
info.vendor = gamemode_get_gpu_vendor(info.device);
From 9320a94638bcc216104ffe822299def89eaf8945 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
Date: Thu, 4 Apr 2019 11:39:17 +0200
Subject: [PATCH 4/4] external-helper: improve run_external_process()
robustness
run_external_process() contained pipe file descriptors leaks (e.g. one
pipe end was never closed). Also the stdout might have been captured
incomplete, since only a single read() was performed on the pipe.
Furthermore should a child process try to write a larger amount of data
onto the pipe then it will become stuck, because the parent process
isn't consuming the data. Thus the timeout would trigger in these cases
although the child process does nothing wrong.
This commit changes the implementation to follow a select() based
approach that continually reads from the pipe, but discards data that
doesn't fit in the provided buffer.
---
daemon/external-helper.c | 136 ++++++++++++++++++++++++++-------------
daemon/gpuclockctl.c | 5 +-
2 files changed, 93 insertions(+), 48 deletions(-)
diff --git a/daemon/external-helper.c b/daemon/external-helper.c
index 52361f9..62cf29a 100644
--- a/daemon/external-helper.c
+++ b/daemon/external-helper.c
@@ -36,11 +36,75 @@ POSSIBILITY OF SUCH DAMAGE.
#include <linux/limits.h>
#include <stdio.h>
+#include <sys/time.h>
+#include <sys/types.h>
#include <sys/wait.h>
-#include <time.h>
#include <unistd.h>
-static const int default_timeout = 5;
+static const int DEFAULT_TIMEOUT = 5;
+
+static int read_child_stdout(int pipe_fd, char buffer[EXTERNAL_BUFFER_MAX], int tsec)
+{
+ fd_set fds;
+ struct timeval timeout;
+ int num_readable = 0;
+ ssize_t buffer_bytes_read = 0;
+ ssize_t just_read = 0;
+ bool buffer_full = false;
+ char discard_buffer[EXTERNAL_BUFFER_MAX];
+
+ /* Set up the timout */
+ timeout.tv_sec = tsec;
+ timeout.tv_usec = 0;
+
+ FD_ZERO(&fds);
+
+ /* Wait for the child to finish up with a timout */
+ while (true) {
+ FD_SET(pipe_fd, &fds);
+ num_readable = select(pipe_fd + 1, &fds, NULL, NULL, &timeout);
+
+ if (num_readable < 0) {
+ if (errno == EINTR) {
+ continue;
+ } else {
+ LOG_ERROR("sigtimedwait failed: %s\n", strerror(errno));
+ return -1;
+ }
+ } else if (num_readable == 0) {
+ return -2;
+ }
+
+ if (!buffer_full) {
+ just_read = read(pipe_fd,
+ buffer + buffer_bytes_read,
+ EXTERNAL_BUFFER_MAX - (size_t)buffer_bytes_read - 1);
+ } else {
+ just_read = read(pipe_fd, discard_buffer, EXTERNAL_BUFFER_MAX - 1);
+ }
+
+ if (just_read < 0) {
+ return -1;
+ } else if (just_read == 0) {
+ // EOF encountered
+ break;
+ }
+
+ if (!buffer_full) {
+ buffer_bytes_read += just_read;
+
+ if (buffer_bytes_read == EXTERNAL_BUFFER_MAX - 1) {
+ // our buffer is exhausted, discard the rest
+ // of the output
+ buffer_full = true;
+ }
+ }
+ }
+
+ buffer[buffer_bytes_read] = 0;
+
+ return 0;
+}
/**
* Call an external process
@@ -50,6 +114,7 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF
pid_t p;
int status = 0;
int pipes[2];
+ int ret = 0;
char internal[EXTERNAL_BUFFER_MAX] = { 0 };
if (pipe(pipes) == -1) {
@@ -59,20 +124,12 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF
/* Set the default timeout */
if (tsec == -1) {
- tsec = default_timeout;
- }
-
- /* set up our signaling for the child and the timout */
- sigset_t mask;
- sigset_t omask;
- sigemptyset(&mask);
- sigaddset(&mask, SIGCHLD);
- if (sigprocmask(SIG_BLOCK, &mask, &omask) < 0) {
- LOG_ERROR("sigprocmask failed: %s\n", strerror(errno));
- return -1;
+ tsec = DEFAULT_TIMEOUT;
}
if ((p = fork()) < 0) {
+ close(pipes[0]);
+ close(pipes[1]);
LOG_ERROR("Failed to fork(): %s\n", strerror(errno));
return false;
} else if (p == 0) {
@@ -91,39 +148,31 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF
LOG_ERROR("Failed to execute external process: %s %s\n", exec_args[0], strerror(errno));
exit(EXIT_FAILURE);
}
- _exit(EXIT_SUCCESS);
- }
-
- /* Set up the timout */
- struct timespec timeout;
- timeout.tv_sec = tsec;
- timeout.tv_nsec = 0;
-
- /* Wait for the child to finish up with a timout */
- while (true) {
- if (sigtimedwait(&mask, NULL, &timeout) < 0) {
- if (errno == EINTR) {
- continue;
- } else if (errno == EAGAIN) {
- LOG_ERROR("Child process timed out for %s, killing and returning\n", exec_args[0]);
- kill(p, SIGKILL);
- } else {
- LOG_ERROR("sigtimedwait failed: %s\n", strerror(errno));
- return -1;
- }
- }
- break;
+ // should never be reached
+ abort();
}
+ // close the write end of the pipe so we get signaled EOF once the
+ // child exits
close(pipes[1]);
- ssize_t output_size = read(pipes[0], internal, EXTERNAL_BUFFER_MAX - 1);
- if (output_size < 0) {
- LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno));
- return -1;
+ ret = read_child_stdout(pipes[0], internal, tsec);
+ close(pipes[0]);
+
+ if (ret != 0) {
+ if (ret == -2) {
+ LOG_ERROR("Child process timed out for %s, killing and returning\n", exec_args[0]);
+ kill(p, SIGKILL);
+ } else {
+ LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno));
+ }
+ if (buffer) {
+ // make sure the buffer is a terminated empty string on error
+ buffer[0] = 0;
+ }
+ } else if (buffer) {
+ memcpy(buffer, internal, EXTERNAL_BUFFER_MAX);
}
- internal[output_size] = 0;
-
if (waitpid(p, &status, 0) < 0) {
LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno));
return -1;
@@ -134,12 +183,9 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF
LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]);
} else if (WEXITSTATUS(status) != 0) {
LOG_ERROR("External process failed with exit code %d\n", WEXITSTATUS(status));
- LOG_ERROR("Output was: %s\n", buffer ? buffer : internal);
+ LOG_ERROR("Output was: %s\n", internal);
return -1;
}
- if (buffer)
- memcpy(buffer, internal, EXTERNAL_BUFFER_MAX);
-
return 0;
}
diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c
index cac49bd..850bd05 100644
--- a/daemon/gpuclockctl.c
+++ b/daemon/gpuclockctl.c
@@ -67,7 +67,7 @@ static void print_usage_and_exit(void)
exit(EXIT_FAILURE);
}
-static const char* get_nv_attr(const char *attr)
+static const char *get_nv_attr(const char *attr)
{
static char out[EXTERNAL_BUFFER_MAX];
const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", attr, "-t", NULL };
@@ -315,8 +315,7 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info)
ret = -1;
}
- if( ret == 0 )
- {
+ if (ret == 0) {
/* Copy in the value from the file */
strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1);
info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0';

View File

@ -1,165 +0,0 @@
From: Matthias Gerstner <matthias.gerstner@suse.com>
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 <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;
}
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 <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);
}
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)