From 4854d072481d1196de711e6019e5258fd2453140 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Thu, 5 Aug 2021 12:56:16 +0100 Subject: [PATCH] pv-wrap, pv-launcher: Set a reasonable RLIMIT_NOFILE We could inherit any RLIMIT_NOFILE from Steam or another caller, but we want to give games a predictable execution environment. If a game uses select(), allocates memory proportional to the soft limit on file descriptors, or loops for a number of iterations proportional to the soft limit on file descriptors (as we did prior to commit 97b5a8f6 "pressure-vessel: Let short-term subprocesses inherit non-CLOEXEC fds"), then file descriptors numerically greater than 1023 are going to be a problem. If the soft limit is more than 1024 (= FD_SETSIZE), reduce it to 1024 to avoid this. Conversely, if we're launched with a soft limit strictly less than 1024, let's try to raise it to 1024 if the hard limit allows that. Related to <https://github.com/ValveSoftware/steam-for-linux/issues/7970>. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/launcher.c | 5 +++ pressure-vessel/wrap.c | 5 +++ steam-runtime-tools/utils-internal.h | 3 ++ steam-runtime-tools/utils.c | 58 ++++++++++++++++++++++++++++ tests/utils.c | 55 ++++++++++++++++++++++++++ 5 files changed, 126 insertions(+) diff --git a/pressure-vessel/launcher.c b/pressure-vessel/launcher.c index ea080400f..7cb70fbc1 100644 --- a/pressure-vessel/launcher.c +++ b/pressure-vessel/launcher.c @@ -845,6 +845,7 @@ main (int argc, GError **error = &local_error; int ret = EX_USAGE; GBusNameOwnerFlags flags; + int result; global_listener = pv_portal_listener_new (); @@ -882,6 +883,10 @@ main (int argc, if (opt_verbose) pv_set_up_logging (opt_verbose); + if ((result = _srt_set_compatible_resource_limits (0)) < 0) + g_warning ("Unable to set normal resource limits: %s", + g_strerror (-result)); + if (!pv_portal_listener_set_up_info_fd (global_listener, opt_info_fd, error)) diff --git a/pressure-vessel/wrap.c b/pressure-vessel/wrap.c index b4c70c10e..e7fd9b9e5 100644 --- a/pressure-vessel/wrap.c +++ b/pressure-vessel/wrap.c @@ -1331,6 +1331,7 @@ main (int argc, g_autoptr(GArray) pass_fds_through_adverb = g_array_new (FALSE, FALSE, sizeof (int)); const char *steam_app_id; g_autoptr(GPtrArray) adverb_preload_argv = NULL; + int result; setlocale (LC_ALL, ""); @@ -1668,6 +1669,10 @@ main (int argc, * us exit 1. */ ret = 1; + if ((result = _srt_set_compatible_resource_limits (0)) < 0) + g_warning ("Unable to set normal resource limits: %s", + g_strerror (-result)); + if (opt_terminal != PV_TERMINAL_TTY) { int fd; diff --git a/steam-runtime-tools/utils-internal.h b/steam-runtime-tools/utils-internal.h index 916033486..a5ddd0a67 100644 --- a/steam-runtime-tools/utils-internal.h +++ b/steam-runtime-tools/utils-internal.h @@ -29,6 +29,7 @@ #include <fcntl.h> #include <stdio.h> #include <sys/stat.h> +#include <sys/types.h> #include <glib.h> @@ -151,3 +152,5 @@ _srt_is_same_file (const gchar *a, return _srt_fstatat_is_same_file (AT_FDCWD, a, AT_FDCWD, b); } + +int _srt_set_compatible_resource_limits (pid_t pid); diff --git a/steam-runtime-tools/utils.c b/steam-runtime-tools/utils.c index 7e2d0b837..c090beb5f 100644 --- a/steam-runtime-tools/utils.c +++ b/steam-runtime-tools/utils.c @@ -34,6 +34,8 @@ #include <signal.h> #include <stdio.h> #include <string.h> +#include <sys/resource.h> +#include <sys/time.h> #include <sys/wait.h> #include <unistd.h> @@ -1416,3 +1418,59 @@ _srt_get_path_after (const char *str, return NULL; } } + +/* + * _srt_set_compatible_resource_limits: + * @pid: A process ID, or 0 to act on the current process + * + * Attempt to set resource limits (see `getrlimit(3)`) for the given + * process to have reasonable values that are compatible with the + * maximum number of programs and libraries. + * + * `RLIMIT_NOFILE`, the limit for file descriptor numbers that can be + * returned by a successful `open(2)`, `pipe(2)`, `dup(2)`, etc., + * is set to 1024 or to the hard limit, whichever is lower, to avoid + * incompatibility with two classes of programs: + * + * - programs that call `select(2)`, which cannot represent file + * descriptors higher than 1023 in the `fd_set` bitmask; + * - programs that allocate an amount of memory or perform a number + * of operations proportional to `RLIMIT_NOFILE`, such as some + * Java interpreters + * + * See <http://0pointer.net/blog/file-descriptor-limits.html> for more + * information on `RLIMIT_NOFILE` best practices. + * + * Other resource limits are not currently altered. + * + * This function is technically not async-signal-safe + * (see `signal-safety(7)`), but in practice can probably be called + * safely after `fork()` on glibc systems. + * + * Returns: 0 on success, or a negative errno value such as `-EINVAL` + * on failure, with errno set. + */ +int +_srt_set_compatible_resource_limits (pid_t pid) +{ + struct rlimit rlim = { 0, 0 }; + int ret; + + ret = prlimit (pid, RLIMIT_NOFILE, NULL, &rlim); + + if (ret < 0) + return -errno; + + if (rlim.rlim_cur != FD_SETSIZE + && (rlim.rlim_max >= FD_SETSIZE + || rlim.rlim_max == RLIM_INFINITY)) + { + rlim.rlim_cur = FD_SETSIZE; + ret = prlimit (pid, RLIMIT_NOFILE, &rlim, NULL); + + if (ret < 0) + return -errno; + } + + return 0; +} diff --git a/tests/utils.c b/tests/utils.c index b297c0745..5d7ce6591 100644 --- a/tests/utils.c +++ b/tests/utils.c @@ -25,6 +25,9 @@ #include <steam-runtime-tools/steam-runtime-tools.h> +#include <sys/resource.h> +#include <sys/time.h> + #include <glib.h> #include <glib/gstdio.h> #include <glib-object.h> @@ -372,6 +375,56 @@ test_gstring_replace (Fixture *f, } } +G_STATIC_ASSERT (FD_SETSIZE == 1024); + +static void +test_rlimit (Fixture *f, + gconstpointer context) +{ + struct rlimit original; + struct rlimit adjusted; + + if (getrlimit (RLIMIT_NOFILE, &original) < 0) + { + int saved_errno = errno; + g_autofree gchar *message = NULL; + + message = g_strdup_printf ("getrlimit: %s", g_strerror (saved_errno)); + g_test_skip (message); + return; + } + + if (original.rlim_max < 2048) + { + g_test_skip ("RLIMIT_NOFILE rlim_max is too small"); + return; + } + + adjusted = original; + adjusted.rlim_cur = 2048; + g_assert_no_errno (setrlimit (RLIMIT_NOFILE, &adjusted)); + g_assert_cmpint (_srt_set_compatible_resource_limits (0), ==, 0); + g_assert_no_errno (getrlimit (RLIMIT_NOFILE, &adjusted)); + g_assert_cmpint (adjusted.rlim_cur, ==, 1024); + g_assert_cmpint (adjusted.rlim_max, ==, original.rlim_max); + + adjusted = original; + adjusted.rlim_cur = 512; + g_assert_no_errno (setrlimit (RLIMIT_NOFILE, &adjusted)); + g_assert_cmpint (_srt_set_compatible_resource_limits (getpid ()), ==, 0); + g_assert_no_errno (getrlimit (RLIMIT_NOFILE, &adjusted)); + g_assert_cmpint (adjusted.rlim_cur, ==, 1024); + g_assert_cmpint (adjusted.rlim_max, ==, original.rlim_max); + + adjusted = original; + adjusted.rlim_cur = 1024; + g_assert_no_errno (setrlimit (RLIMIT_NOFILE, &adjusted)); + g_assert_cmpint (_srt_set_compatible_resource_limits (0), ==, 0); + g_assert_no_errno (getrlimit (RLIMIT_NOFILE, &adjusted)); + g_assert_cmpint (adjusted.rlim_cur, ==, 1024); + g_assert_cmpint (adjusted.rlim_max, ==, original.rlim_max); +} + static void test_same_file (Fixture *f, gconstpointer context) @@ -507,6 +560,8 @@ main (int argc, setup, test_get_path_after, teardown); g_test_add ("/utils/gstring-replace", Fixture, NULL, setup, test_gstring_replace, teardown); + g_test_add ("/utils/rlimit", Fixture, NULL, + setup, test_rlimit, teardown); g_test_add ("/utils/same-file", Fixture, NULL, setup, test_same_file, teardown); g_test_add ("/utils/str_is_integer", Fixture, NULL, -- GitLab