From f6220a2d6e2c6ecb348541b28314d1fa7767f52b Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 27 Sep 2019 11:06:03 +0200 Subject: [PATCH 01/10] lib: do flatpak check only once Either we are in a flatpak or not, this doesn't change, so we can just remember the result. --- lib/client_impl.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/client_impl.c b/lib/client_impl.c index 2a47903..4c187f1 100644 --- a/lib/client_impl.c +++ b/lib/client_impl.c @@ -76,12 +76,17 @@ static char error_string[512] = { 0 }; // Helper to check if we are running inside a flatpak static int in_flatpak(void) { - struct stat sb; - int r; + static int status = -1; - r = lstat("/.flatpak-info", &sb); + if (status == -1) { + struct stat sb; + int r; - return r == 0 && sb.st_size > 0; + r = lstat("/.flatpak-info", &sb); + status = r == 0 && sb.st_size > 0; + } + + return status; } static int log_error(const char *fmt, ...) From b513bc65ae0d3fe6c098ac79f7755d7a761fe6f2 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 27 Sep 2019 17:51:11 +0200 Subject: [PATCH 02/10] lib: extract dbus messaging code Separate the D-Bus messaging code from gamemode_request(). --- lib/client_impl.c | 100 ++++++++++++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 34 deletions(-) diff --git a/lib/client_impl.c b/lib/client_impl.c index 4c187f1..9e2db72 100644 --- a/lib/client_impl.c +++ b/lib/client_impl.c @@ -151,31 +151,17 @@ static void cleanup_pending_call(DBusPendingCall **call) } /* internal API */ -static int gamemode_request(const char *method, pid_t for_pid) +static int make_request(DBusConnection *bus, + int native, const char *method, + pid_t *pids, int npids, + DBusError *error) { - _cleanup_bus_ DBusConnection *bus = NULL; _cleanup_msg_ DBusMessage *msg = NULL; _cleanup_dpc_ DBusPendingCall *call = NULL; - DBusMessageIter iter; DBusError err; - dbus_int32_t pid; - int native; + DBusMessageIter iter; int res = -1; - native = !in_flatpak(); - pid = (dbus_int32_t)getpid(); - - TRACE("GM: [%d] request '%s' received (for pid: %d) [portal: %s]\n", - (int)pid, - method, - (int)for_pid, - (native ? "n" : "y")); - - bus = hop_on_the_bus(); - - if (bus == NULL) - return -1; - // If we are inside a flatpak we need to talk to the portal instead const char *dest = native ? DAEMON_DBUS_NAME : PORTAL_DBUS_NAME; const char *path = native ? DAEMON_DBUS_PATH : PORTAL_DBUS_PATH; @@ -183,14 +169,15 @@ static int gamemode_request(const char *method, pid_t for_pid) msg = dbus_message_new_method_call(dest, path, iface, method); - if (!msg) - return log_error("Could not create dbus message"); + if (!msg) { + dbus_set_error_const(error, DBUS_ERROR_FAILED, + "Could not create dbus message"); + return -1; + } dbus_message_iter_init_append(msg, &iter); - dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT32, &pid); - - if (for_pid != 0) { - dbus_int32_t p = (dbus_int32_t)for_pid; + for (int i = 0; i < npids; i++) { + dbus_int32_t p = (dbus_int32_t)pids[i]; dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT32, &p); } @@ -202,20 +189,65 @@ static int gamemode_request(const char *method, pid_t for_pid) dbus_pending_call_block(call); msg = dbus_pending_call_steal_reply(call); - if (msg == NULL) - return log_error("Did not receive a reply"); + if (msg == NULL) { + dbus_set_error_const(error, DBUS_ERROR_FAILED, + "Did not receive a reply"); + return -1; + } dbus_error_init(&err); - if (dbus_set_error_from_message(&err, msg)) - log_error("Could not call method '%s' on '%s': %s", method, dest, err.message); - else if (!dbus_message_iter_init(msg, &iter) || - dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT32) - log_error("Failed to parse response"); - else + if (dbus_set_error_from_message(&err, msg)) { + dbus_set_error(error, err.name, + "Could not call method '%s' on '%s': %s", + method, dest, err.message); + } else if (!dbus_message_iter_init(msg, &iter) || + dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT32) { + dbus_set_error(error, DBUS_ERROR_INVALID_SIGNATURE, + "Failed to parse response"); + } else { dbus_message_iter_get_basic(&iter, &res); + } - TRACE("GM: [%d] request '%s' done: %d\n", (int)pid, method, res); + /* free the local error */ + if (dbus_error_is_set(&err)) + dbus_error_free(&err); + + return res; +} + +static int gamemode_request(const char *method, pid_t for_pid) +{ + _cleanup_bus_ DBusConnection *bus = NULL; + DBusError err; + pid_t pids[2] = {0, for_pid}; + int npids; + int native; + int res = -1; + + native = !in_flatpak(); + pids[0] = getpid(); + + TRACE("GM: [%d] request '%s' received (for pid: %d) [portal: %s]\n", + (int)pids[0], + method, + (int)pids[1], + (native ? "n" : "y")); + + bus = hop_on_the_bus(); + + if (bus == NULL) + return -1; + + npids = for_pid > 0 ? 2 : 1; + + dbus_error_init(&err); + res = make_request(bus, native, method, pids, npids, &err); + + if (res == -1) + log_error("D-Bus error: %s", err.message); + + TRACE("GM: [%d] request '%s' done: %d\n", (int)pids[0], method, res); if (dbus_error_is_set(&err)) dbus_error_free(&err); From b7dc1dc10c69a5c005c53e1086eb9d222a88dd7c Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Sat, 28 Sep 2019 13:29:47 +0200 Subject: [PATCH 03/10] daemon: small fix for code comments Comments were not reflecting the what they were describing. --- daemon/gamemode-dbus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/daemon/gamemode-dbus.c b/daemon/gamemode-dbus.c index 6a9e001..008d919 100644 --- a/daemon/gamemode-dbus.c +++ b/daemon/gamemode-dbus.c @@ -169,7 +169,7 @@ static int method_unregister_game_by_pid(sd_bus_message *m, void *userdata, } /** - * Handles the QueryStatus D-BUS Method + * Handles the QueryStatusByPID D-BUS Method */ static int method_query_status_by_pid(sd_bus_message *m, void *userdata, __attribute__((unused)) sd_bus_error *ret_error) @@ -188,8 +188,9 @@ static int method_query_status_by_pid(sd_bus_message *m, void *userdata, return sd_bus_reply_method_return(m, "i", status); } + /** - * Handles the Active D-BUS Method + * Handles the ClientCount D-BUS Property */ static int property_get_client_count(sd_bus *local_bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, From 35fb7f5bafb1d59f4e903e9e549aa1bd720b42e3 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Sat, 28 Sep 2019 14:06:37 +0200 Subject: [PATCH 04/10] meson: include dir in daemon_common dependency Specify the include directory in the link_daemon_common dependency and thus everything that includes that as a dependency will get the proper include directory automatically. --- common/meson.build | 3 +-- daemon/meson.build | 1 - util/meson.build | 2 -- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/common/meson.build b/common/meson.build index 78d1c03..686dd81 100644 --- a/common/meson.build +++ b/common/meson.build @@ -15,6 +15,5 @@ daemon_common = static_library( link_daemon_common = declare_dependency( link_with: daemon_common, + include_directories: [include_directories('.')] ) - -include_daemon_common = include_directories('.') \ No newline at end of file diff --git a/daemon/meson.build b/daemon/meson.build index 7a3544e..5109db3 100644 --- a/daemon/meson.build +++ b/daemon/meson.build @@ -26,7 +26,6 @@ executable( ], include_directories: [ gamemoded_includes, - include_daemon_common, ], install: true, ) \ No newline at end of file diff --git a/util/meson.build b/util/meson.build index 700f5c6..4714557 100644 --- a/util/meson.build +++ b/util/meson.build @@ -11,7 +11,6 @@ cpugovctl = executable( link_daemon_common, ], install: true, - include_directories: include_daemon_common, install_dir: path_libexecdir, ) @@ -27,6 +26,5 @@ gpuclockctl = executable( link_daemon_common, ], install: true, - include_directories: include_daemon_common, install_dir: path_libexecdir, ) From 4b59818fd4c059b0935f58bf05a50fbec31ecf37 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 4 Oct 2019 15:54:33 +0200 Subject: [PATCH 05/10] common: add autofree helper Much like the auto-closing helper for file descriptors, add a new auto-free helper that is meant to be used with dynamically allocated memory, a la: autofree char *data = NULL; ... data = malloc(size); When data goes out of scope, cleanup_free will be called with &data, i.e. cleanup_free(&data), which in turn will call free(3) data. In order to work with all types, e.g. 'char *' (resulting in char ** being passed to cleanup_free) or 'int *' (resulting in int ** being passed to cleanup_free), cleanup_free is defined to work with void *, hence the odd-looking cast: void *target = *(void **) ptr. --- common/common-helpers.c | 1 + common/common-helpers.h | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/common/common-helpers.c b/common/common-helpers.c index 5cffc82..cfbdd36 100644 --- a/common/common-helpers.c +++ b/common/common-helpers.c @@ -39,3 +39,4 @@ POSSIBILITY OF SUCH DAMAGE. * a specific call to the function the linker will expect an definition. */ extern inline void cleanup_close(int *fd); +extern inline void cleanup_free(void *ptr); diff --git a/common/common-helpers.h b/common/common-helpers.h index 6af4ba2..d079b3f 100644 --- a/common/common-helpers.h +++ b/common/common-helpers.h @@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE. #pragma once +#include #include #include #include @@ -80,3 +81,23 @@ inline void cleanup_close(int *fd_ptr) * like "autoclose_fd int fd = -1;". */ #define autoclose_fd __attribute__((cleanup(cleanup_close))) + +/** + * Helper function for auto-freeing dynamically allocated memory. Does nothing + * if *ptr is NULL (ptr must not be NULL). + */ +inline void cleanup_free(void *ptr) +{ + /* The function is defined to work with 'void *' because + * that will make sure it compiles without warning also + * for all types; what we are getting passed into is a + * pointer to a pointer though, so we need to cast */ + void *target = *(void **)ptr; + free(target); /* free can deal with NULL */ +} + +/** + * Helper macro for auto-freeing dynamically allocated memory: use by + * prefixing the variable, like "autofree char *data = NULL;". + */ +#define autofree __attribute__((cleanup(cleanup_free))) From 5398dd1d19e44973d0016477b04232bcc1157668 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 4 Oct 2019 17:34:53 +0200 Subject: [PATCH 06/10] common: add pidfd related methods Add functions to open pidfds, i.e. file descriptors representing processes, for process ids and vice versa. Both functions work an array of fds/pids, stop on error and return the number of successfully handled items. --- common/common-pidfds.c | 207 +++++++++++++++++++++++++++++++++++++++++ common/common-pidfds.h | 53 +++++++++++ common/meson.build | 2 + meson.build | 4 + 4 files changed, 266 insertions(+) create mode 100644 common/common-pidfds.c create mode 100644 common/common-pidfds.h diff --git a/common/common-pidfds.c b/common/common-pidfds.c new file mode 100644 index 0000000..a01cab7 --- /dev/null +++ b/common/common-pidfds.c @@ -0,0 +1,207 @@ +/* + +Copyright (c) 2017-2019, Feral Interactive +Copyright (c) 2019, Red Hat +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + +*/ +#define _GNU_SOURCE +#include + +#include "common-helpers.h" +#include "common-pidfds.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if !HAVE_FN_PIDFD_OPEN +#include + +#ifndef __NR_pidfd_open +#define __NR_pidfd_open 434 +#endif + +static int pidfd_open(pid_t pid, unsigned int flags) +{ + return (int)syscall(__NR_pidfd_open, pid, flags); +} +#endif + +/* pidfd functions */ +int open_pidfds(pid_t *pids, int *fds, int count) +{ + int i = 0; + + for (i = 0; i < count; i++) { + int pid = pids[i]; + int fd = pidfd_open(pid, 0); + + if (fd < 0) + break; + + fds[i] = fd; + } + + return i; +} + +static int parse_pid(const char *str, pid_t *pid) +{ + unsigned long long int v; + char *end; + pid_t p; + + errno = 0; + v = strtoull(str, &end, 0); + if (end == str) + return -ENOENT; + else if (errno != 0) + return -errno; + + p = (pid_t)v; + + if (p < 1 || (unsigned long long int)p != v) + return -ERANGE; + + if (pid) + *pid = p; + + return 0; +} + +static int parse_status_field_pid(const char *val, pid_t *pid) +{ + const char *t; + + t = strrchr(val, '\t'); + if (t == NULL) + return -ENOENT; + + return parse_pid(t, pid); +} + +static int pidfd_to_pid(int fdinfo, int pidfd, pid_t *pid) +{ + autofree char *key = NULL; + autofree char *val = NULL; + char name[256] = { + 0, + }; + bool found = false; + FILE *f = NULL; + size_t keylen = 0; + size_t vallen = 0; + ssize_t n; + int fd; + int r = 0; + + *pid = 0; + + buffered_snprintf(name, "%d", pidfd); + + fd = openat(fdinfo, name, O_RDONLY | O_CLOEXEC | O_NOCTTY); + + if (fd != -1) + f = fdopen(fd, "r"); + + if (f == NULL) + return -errno; + + do { + n = getdelim(&key, &keylen, ':', f); + if (n == -1) { + r = errno; + break; + } + + n = getdelim(&val, &vallen, '\n', f); + if (n == -1) { + r = errno; + break; + } + + // TODO: strstrip (key); + + if (!strncmp(key, "Pid", 3)) { + r = parse_status_field_pid(val, pid); + found = r > -1; + } + + } while (r == 0 && !found); + + fclose(f); + + if (r < 0) + return r; + else if (!found) + return -ENOENT; + + return 0; +} + +int pidfds_to_pids(int *fds, pid_t *pids, int count) +{ + int fdinfo = -1; + int r = 0; + int i; + + fdinfo = open_fdinfo_dir(); + if (fdinfo == -1) + return -1; + + for (i = 0; i < count && r == 0; i++) + r = pidfd_to_pid(fdinfo, fds[i], &pids[i]); + + (void)close(fdinfo); + + if (r != 0) + errno = -r; + + return i; +} + +/* misc directory helpers */ +int open_fdinfo_dir(void) +{ + int fd; + + fd = open("/proc/self/fdinfo", O_RDONLY | O_NONBLOCK | O_DIRECTORY | O_CLOEXEC | O_NOCTTY); + + if (fd == -1) + return errno; + + return fd; +} diff --git a/common/common-pidfds.h b/common/common-pidfds.h new file mode 100644 index 0000000..b158517 --- /dev/null +++ b/common/common-pidfds.h @@ -0,0 +1,53 @@ +/* + +Copyright (c) 2017-2019, Feral Interactive +Copyright (c) 2019, Red Hat +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Feral Interactive nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. + +*/ + +#include + +/* Open pidfds for up to count process ids specified in pids. The + * pointer fds needs to point to an array with at least count + * entries. Will stop when it encounters an error (and sets errno). + * Returns the number of successfully opened pidfds (or -1 in case + * of other errors. */ +int open_pidfds(pid_t *pids, int *fds, int count); + +/* Translate up to count process ids to the corresponding process ids. + * The pointer pids needs to point to an array with at least count + * entries. Will stop when it encounters an error (and sets errno). + * Returns the number of successfully translated pidfds (or -1 in + * case of other errors. */ +int pidfds_to_pids(int *fds, pid_t *pids, int count); + +/* Helper to open the fdinfo directory for the current process, i.e. + * does open("/proc/self/fdinfo", ...). Returns the file descriptor + * for the directory, ownership is transferred and caller needs to + * call close on it. */ +int open_fdinfo_dir(void); diff --git a/common/meson.build b/common/meson.build index 686dd81..93c9fde 100644 --- a/common/meson.build +++ b/common/meson.build @@ -5,12 +5,14 @@ common_sources = [ 'common-external.c', 'common-helpers.c', 'common-gpu.c', + 'common-pidfds.c', ] daemon_common = static_library( 'daemon-common', sources: common_sources, install: false, + include_directories: [config_h_dir] ) link_daemon_common = declare_dependency( diff --git a/meson.build b/meson.build index 97da208..881d28e 100644 --- a/meson.build +++ b/meson.build @@ -128,9 +128,13 @@ with_examples = get_option('with-examples') with_util = get_option('with-util') # Provide a config.h +pidfd_open = cc.has_function('pidfd_open', args: '-D_GNU_SOURCE') + cdata = configuration_data() cdata.set_quoted('LIBEXECDIR', path_libexecdir) cdata.set_quoted('GAMEMODE_VERSION', meson.project_version()) +cdata.set10('HAVE_FN_PIDFD_OPEN', pidfd_open) + config_h = configure_file( configuration: cdata, output: 'build-config.h', From a6552044cda88988b9ae52d4a1a993bd79f700be Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 4 Oct 2019 17:48:18 +0200 Subject: [PATCH 07/10] daemon: add new pidfd based D-Bus API Provide a new set of APIs with identical semantics as the existing ByPID family of calls but instead of working with process ids, they take pidfds, file descriptors representing processes, instead. The fds can be translated back to pids (in the correct namespace) and also be monitored via select/poll/epoll. The current implementation translates them directly back to pids, but in the future the monitoring code that watches processes (if they are still alive) could be converted be event driven via pidfds. --- daemon/gamemode-dbus.c | 86 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/daemon/gamemode-dbus.c b/daemon/gamemode-dbus.c index 008d919..f3ba4e2 100644 --- a/daemon/gamemode-dbus.c +++ b/daemon/gamemode-dbus.c @@ -32,7 +32,9 @@ POSSIBILITY OF SUCH DAMAGE. #define _GNU_SOURCE #include "gamemode.h" +#include "common-helpers.h" #include "common-logging.h" +#include "common-pidfds.h" #include #include @@ -189,6 +191,84 @@ static int method_query_status_by_pid(sd_bus_message *m, void *userdata, return sd_bus_reply_method_return(m, "i", status); } +/** + * Handles the RegisterGameByPIDFd D-BUS Method + */ +static int method_register_game_by_pidfd(sd_bus_message *m, void *userdata, + __attribute__((unused)) sd_bus_error *ret_error) +{ + int fds[2] = { -1, -1 }; + pid_t pids[2] = { 0, 0 }; + GameModeContext *context = userdata; + + int ret = sd_bus_message_read(m, "hh", &fds[0], &fds[1]); + if (ret < 0) { + LOG_ERROR("Failed to parse input parameters: %s\n", strerror(-ret)); + return ret; + } + + int reply = pidfds_to_pids(fds, pids, 2); + + if (reply == 2) + reply = game_mode_context_register(context, pids[0], pids[1]); + else + reply = -1; + + return sd_bus_reply_method_return(m, "i", reply); +} + +/** + * Handles the UnregisterGameByPIDFd D-BUS Method + */ +static int method_unregister_game_by_pidfd(sd_bus_message *m, void *userdata, + __attribute__((unused)) sd_bus_error *ret_error) +{ + int fds[2] = { -1, -1 }; + pid_t pids[2] = { 0, 0 }; + GameModeContext *context = userdata; + + int ret = sd_bus_message_read(m, "hh", &fds[0], &fds[1]); + if (ret < 0) { + LOG_ERROR("Failed to parse input parameters: %s\n", strerror(-ret)); + return ret; + } + + int reply = pidfds_to_pids(fds, pids, 2); + + if (reply == 2) + reply = game_mode_context_unregister(context, pids[0], pids[1]); + else + reply = -1; + + return sd_bus_reply_method_return(m, "i", reply); +} + +/** + * Handles the QueryStatusByPIDFd D-BUS Method + */ +static int method_query_status_by_pidfd(sd_bus_message *m, void *userdata, + __attribute__((unused)) sd_bus_error *ret_error) +{ + int fds[2] = { -1, -1 }; + pid_t pids[2] = { 0, 0 }; + GameModeContext *context = userdata; + + int ret = sd_bus_message_read(m, "hh", &fds[0], &fds[1]); + if (ret < 0) { + LOG_ERROR("Failed to parse input parameters: %s\n", strerror(-ret)); + return ret; + } + + int reply = pidfds_to_pids(fds, pids, 2); + + if (reply == 2) + reply = game_mode_context_query_status(context, pids[0], pids[1]); + else + reply = -1; + + return sd_bus_reply_method_return(m, "i", reply); +} + /** * Handles the ClientCount D-BUS Property */ @@ -323,6 +403,12 @@ static const sd_bus_vtable gamemode_vtable[] = { SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("QueryStatusByPID", "ii", "i", method_query_status_by_pid, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("RegisterGameByPIDFd", "hh", "i", method_register_game_by_pidfd, + SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("UnregisterGameByPIDFd", "hh", "i", method_unregister_game_by_pidfd, + SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD("QueryStatusByPIDFd", "hh", "i", method_query_status_by_pidfd, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("RefreshConfig", "", "i", method_refresh_config, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ListGames", "", "a(io)", method_list_games, SD_BUS_VTABLE_UNPRIVILEGED), From cd6c2ee612929b880d6db4993c4066b6f072913b Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 4 Oct 2019 18:56:36 +0200 Subject: [PATCH 08/10] common: create another lib tailored for the client Use a small subset of the existing common files to create another static library to be used from the client library. --- common/meson.build | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/common/meson.build b/common/meson.build index 93c9fde..9411900 100644 --- a/common/meson.build +++ b/common/meson.build @@ -19,3 +19,18 @@ link_daemon_common = declare_dependency( link_with: daemon_common, include_directories: [include_directories('.')] ) + +lib_common = static_library( + 'lib-common', + sources: [ + 'common-helpers.c', + 'common-pidfds.c' + ], + install: false, + include_directories: [config_h_dir] +) + +link_lib_common = declare_dependency( + link_with: lib_common, + include_directories: [include_directories('.')] +) From b84d673aaebb3e7eb0e8eb01820b40b8f3bb498b Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 4 Oct 2019 18:57:59 +0200 Subject: [PATCH 09/10] meson: reorder lib and common subdir So that lib can depend on the new library from common. --- meson.build | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 881d28e..80b244a 100644 --- a/meson.build +++ b/meson.build @@ -141,12 +141,12 @@ config_h = configure_file( ) config_h_dir = include_directories('.') -# Library is always required -subdir('lib') - # common lib is always required subdir('common') +# Library is always required +subdir('lib') + # Utilities are always required except when having both 64 and 32 bit versions # of libgamemode installed if with_util == true From 6f7df91b60c4c050824a1e36c8d4e66d0cf3b2d4 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Fri, 4 Oct 2019 19:04:15 +0200 Subject: [PATCH 10/10] lib: support the new pidfd based APIs Try to make API requests using the new pidfd based APIs. If getting the pidfds fails or if the remote (daemon) does not support the new pidfd based D-Bus API, transparently fall back to the old API. --- lib/client_impl.c | 149 ++++++++++++++++++++++++++++++++++++++-------- lib/meson.build | 1 + 2 files changed, 124 insertions(+), 26 deletions(-) diff --git a/lib/client_impl.c b/lib/client_impl.c index 9e2db72..f53d063 100644 --- a/lib/client_impl.c +++ b/lib/client_impl.c @@ -31,8 +31,14 @@ POSSIBILITY OF SUCH DAMAGE. #define _GNU_SOURCE +#include +#include + #include +#include #include +#include +#include #include #include #include @@ -54,6 +60,7 @@ POSSIBILITY OF SUCH DAMAGE. #define _cleanup_bus_ _cleanup_(hop_off_the_bus) #define _cleanup_msg_ _cleanup_(cleanup_msg) #define _cleanup_dpc_ _cleanup_(cleanup_pending_call) +#define _cleanup_fds_ _cleanup_(cleanup_fd_array) #ifdef NDEBUG #define DEBUG(...) @@ -73,6 +80,35 @@ static int log_error(const char *fmt, ...) __attribute__((format(printf, 1, 2))) // Storage for error strings static char error_string[512] = { 0 }; +// memory helpers +static void cleanup_fd_array(int **fdlist) +{ + if (fdlist == NULL || *fdlist == NULL) + return; + + int errsave = errno; + for (int *fd = *fdlist; *fd != -1; fd++) { + TRACE("GM Closing fd %d\n", *fd); + (void)close(*fd); + } + + errno = errsave; + free(*fdlist); +} + +// Allocate a -1 termianted array of ints +static inline int *alloc_fd_array(int n) +{ + int *fds; + + size_t count = (size_t)n + 1; /* -1, terminated */ + fds = (int *)malloc(sizeof(int) * count); + for (size_t i = 0; i < count; i++) + fds[i] = -1; + + return fds; +} + // Helper to check if we are running inside a flatpak static int in_flatpak(void) { @@ -136,7 +172,7 @@ static DBusConnection *hop_on_the_bus(void) /* cleanup functions */ static void cleanup_msg(DBusMessage **msg) { - if (msg == NULL) + if (msg == NULL || *msg == NULL) return; dbus_message_unref(*msg); @@ -144,24 +180,54 @@ static void cleanup_msg(DBusMessage **msg) static void cleanup_pending_call(DBusPendingCall **call) { - if (call == NULL) + if (call == NULL || *call == NULL) return; dbus_pending_call_unref(*call); } /* internal API */ -static int make_request(DBusConnection *bus, - int native, const char *method, - pid_t *pids, int npids, - DBusError *error) +static int make_request(DBusConnection *bus, int native, int use_pidfds, const char *method, + pid_t *pids, int npids, DBusError *error) { _cleanup_msg_ DBusMessage *msg = NULL; _cleanup_dpc_ DBusPendingCall *call = NULL; + _cleanup_fds_ int *fds = NULL; + char action[256] = { + 0, + }; DBusError err; DBusMessageIter iter; int res = -1; + TRACE("GM: Incoming request: %s, npids: %d, native: %d pifds: %d\n", + method, + npids, + native, + use_pidfds); + + if (use_pidfds) { + fds = alloc_fd_array(npids); + + res = open_pidfds(pids, fds, npids); + if (res != npids) { + dbus_set_error(error, DBUS_ERROR_FAILED, "Could not open pidfd for %d", (int)pids[res]); + return -1; + } + + if (strstr(method, "ByPID")) + snprintf(action, sizeof(action), "%sFd", method); + else + snprintf(action, sizeof(action), "%sByPIDFd", method); + method = action; + } + + TRACE("GM: Making request: %s, npids: %d, native: %d pifds: %d\n", + method, + npids, + native, + use_pidfds); + // If we are inside a flatpak we need to talk to the portal instead const char *dest = native ? DAEMON_DBUS_NAME : PORTAL_DBUS_NAME; const char *path = native ? DAEMON_DBUS_PATH : PORTAL_DBUS_PATH; @@ -170,15 +236,24 @@ static int make_request(DBusConnection *bus, msg = dbus_message_new_method_call(dest, path, iface, method); if (!msg) { - dbus_set_error_const(error, DBUS_ERROR_FAILED, - "Could not create dbus message"); + dbus_set_error_const(error, DBUS_ERROR_FAILED, "Could not create dbus message"); return -1; } dbus_message_iter_init_append(msg, &iter); + for (int i = 0; i < npids; i++) { - dbus_int32_t p = (dbus_int32_t)pids[i]; - dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT32, &p); + dbus_int32_t p; + int type; + + if (use_pidfds) { + type = DBUS_TYPE_UNIX_FD; + p = (dbus_int32_t)fds[i]; + } else { + type = DBUS_TYPE_INT32; + p = (dbus_int32_t)pids[i]; + } + dbus_message_iter_append_basic(&iter, type, &p); } dbus_connection_send_with_reply(bus, msg, &call, -1); @@ -190,21 +265,22 @@ static int make_request(DBusConnection *bus, msg = dbus_pending_call_steal_reply(call); if (msg == NULL) { - dbus_set_error_const(error, DBUS_ERROR_FAILED, - "Did not receive a reply"); + dbus_set_error_const(error, DBUS_ERROR_FAILED, "Did not receive a reply"); return -1; } dbus_error_init(&err); - + res = -1; if (dbus_set_error_from_message(&err, msg)) { - dbus_set_error(error, err.name, - "Could not call method '%s' on '%s': %s", - method, dest, err.message); + dbus_set_error(error, + err.name, + "Could not call method '%s' on '%s': %s", + method, + dest, + err.message); } else if (!dbus_message_iter_init(msg, &iter) || - dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT32) { - dbus_set_error(error, DBUS_ERROR_INVALID_SIGNATURE, - "Failed to parse response"); + dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT32) { + dbus_set_error(error, DBUS_ERROR_INVALID_SIGNATURE, "Failed to parse response"); } else { dbus_message_iter_get_basic(&iter, &res); } @@ -219,16 +295,26 @@ static int make_request(DBusConnection *bus, static int gamemode_request(const char *method, pid_t for_pid) { _cleanup_bus_ DBusConnection *bus = NULL; + static int use_pidfs = 1; DBusError err; - pid_t pids[2] = {0, for_pid}; + pid_t pids[2]; int npids; int native; int res = -1; native = !in_flatpak(); - pids[0] = getpid(); - TRACE("GM: [%d] request '%s' received (for pid: %d) [portal: %s]\n", + /* pid[0] is the client, i.e. the game + * pid[1] is the requestor, i.e. this process + * + * we setup the array such that pids[1] will always be a valid + * pid, because if we are going to use the pidfd based API, + * both pids are being sent, even if they are the same + */ + pids[1] = getpid(); + pids[0] = for_pid != 0 ? for_pid : pids[1]; + + TRACE("GM: [%d] request '%s' received (by: %d) [portal: %s]\n", (int)pids[0], method, (int)pids[1], @@ -239,12 +325,23 @@ static int gamemode_request(const char *method, pid_t for_pid) if (bus == NULL) return -1; - npids = for_pid > 0 ? 2 : 1; - dbus_error_init(&err); - res = make_request(bus, native, method, pids, npids, &err); +retry: + if (for_pid != 0 || use_pidfs) + npids = 2; + else + npids = 1; - if (res == -1) + res = make_request(bus, native, use_pidfs, method, pids, npids, &err); + + if (res == -1 && use_pidfs && dbus_error_is_set(&err)) { + TRACE("GM: Request with pidfds failed (%s). Retrying.\n", err.message); + use_pidfs = 0; + dbus_error_free(&err); + goto retry; + } + + if (res == -1 && dbus_error_is_set(&err)) log_error("D-Bus error: %s", err.message); TRACE("GM: [%d] request '%s' done: %d\n", (int)pids[0], method, res); diff --git a/lib/meson.build b/lib/meson.build index 7378e59..f09dc80 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -12,6 +12,7 @@ gamemode = shared_library( 'client_impl.c', ], dependencies: [ + link_lib_common, dep_dbus, ], install: true,