Skip to content
Snippets Groups Projects
Commit 4854d072 authored by Simon McVittie's avatar Simon McVittie
Browse files

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: default avatarSimon McVittie <smcv@collabora.com>
parent a50fb11d
No related branches found
No related tags found
1 merge request!353pv-wrap, pv-launcher: Set a reasonable RLIMIT_NOFILE
Pipeline #16519 passed
......@@ -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))
......
......@@ -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;
......
......@@ -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);
......@@ -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;
}
......@@ -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,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment