From 93be2c23164f89def3afdc3d049bf6dd3e872401 Mon Sep 17 00:00:00 2001 From: Dmitrii Kuvaiskii Date: Mon, 19 Aug 2024 03:22:13 -0700 Subject: [PATCH] [LibOS] Fix bug of `RLIMIT_STACK` being overwritten in child Previously, the resource limit `RLIMIT_STACK` (maximum size of the process stack, in bytes) was overwritten in the child process, after fork. Thus, if the parent used `setrlimit(RLIMIT_STACK)`, the effect of this was lost in the child. This commit fixes this bug, and adds a LibOS test to verify the correct behavior. Signed-off-by: Dmitrii Kuvaiskii --- libos/src/libos_init.c | 26 ++++++---- libos/test/regression/meson.build | 1 + libos/test/regression/rlimit_stack.c | 51 +++++++++++++++++++ .../regression/rlimit_stack.manifest.template | 20 ++++++++ libos/test/regression/test_libos.py | 9 ++++ libos/test/regression/tests.toml | 1 + libos/test/regression/tests_musl.toml | 1 + 7 files changed, 99 insertions(+), 10 deletions(-) create mode 100644 libos/test/regression/rlimit_stack.c create mode 100644 libos/test/regression/rlimit_stack.manifest.template diff --git a/libos/src/libos_init.c b/libos/src/libos_init.c index 31f9bf0846..9c0d4ccc8a 100644 --- a/libos/src/libos_init.c +++ b/libos/src/libos_init.c @@ -278,23 +278,29 @@ static int populate_stack(void* stack, size_t stack_size, const char* const* arg int init_stack(const char* const* argv, const char* const* envp, char*** out_argp, elf_auxv_t** out_auxv) { int ret; - - assert(g_manifest_root); uint64_t stack_size; - ret = toml_sizestring_in(g_manifest_root, "sys.stack.size", get_rlimit_cur(RLIMIT_STACK), - &stack_size); - if (ret < 0) { - log_error("Cannot parse 'sys.stack.size'"); - return -EINVAL; - } - stack_size = ALLOC_ALIGN_UP(stack_size); - set_rlimit_cur(RLIMIT_STACK, stack_size); + assert(g_pal_public_state); + if (g_pal_public_state->parent_process) { + /* after fork, in the new child process, `libos_init` is run, hence this function too - but + * forked process will get its RLIMIT_STACK from the checkpoint */ + stack_size = get_rlimit_cur(RLIMIT_STACK); + } else { + assert(g_manifest_root); + ret = toml_sizestring_in(g_manifest_root, "sys.stack.size", get_rlimit_cur(RLIMIT_STACK), + &stack_size); + if (ret < 0) { + log_error("Cannot parse 'sys.stack.size'"); + return -EINVAL; + } + set_rlimit_cur(RLIMIT_STACK, stack_size); + } struct libos_thread* cur_thread = get_cur_thread(); if (!cur_thread || cur_thread->stack) return 0; + stack_size = ALLOC_ALIGN_UP(stack_size); void* stack = allocate_stack(stack_size, ALLOC_ALIGNMENT, /*user=*/true); if (!stack) return -ENOMEM; diff --git a/libos/test/regression/meson.build b/libos/test/regression/meson.build index 8442aa7845..dc66f0e19f 100644 --- a/libos/test/regression/meson.build +++ b/libos/test/regression/meson.build @@ -104,6 +104,7 @@ tests = { 'rename_unlink': {}, 'rename_unlink_fchown': {}, 'rlimit_nofile': {}, + 'rlimit_stack': {}, 'run_test': { 'include_directories': include_directories( # for `gramine_entry_api.h` diff --git a/libos/test/regression/rlimit_stack.c b/libos/test/regression/rlimit_stack.c new file mode 100644 index 0000000000..9954af4279 --- /dev/null +++ b/libos/test/regression/rlimit_stack.c @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: LGPL-3.0-or-later */ +/* Copyright (C) 2024 Intel Corporation */ + +#include +#include +#include +#include +#include +#include + +#include "common.h" + +int main(void) { + struct rlimit rlim; + + CHECK(getrlimit(RLIMIT_STACK, &rlim)); + printf("old RLIMIT_STACK soft limit: %lu\n", (uint64_t)rlim.rlim_cur); + + /* make sure we can increase the current soft limit */ + if (rlim.rlim_cur >= rlim.rlim_max) + CHECK(-1); + + rlim.rlim_cur++; + CHECK(setrlimit(RLIMIT_STACK, &rlim)); + printf("new RLIMIT_STACK soft limit: %lu\n", (uint64_t)rlim.rlim_cur); + + fflush(stdout); + + int pid = CHECK(fork()); + if (pid == 0) { + /* verify that STACK limit is correctly migrated to the child process */ + CHECK(getrlimit(RLIMIT_STACK, &rlim)); + printf("(in child, after setrlimit) RLIMIT_STACK soft limit: %lu\n", + (uint64_t)rlim.rlim_cur); + + /* NOTE: we currently don't test that the stack limit is indeed enforced */ + exit(0); + } + + int status = 0; + CHECK(wait(&status)); + if (!WIFEXITED(status) || WEXITSTATUS(status)) + errx(1, "child wait status: %#x", status); + + CHECK(getrlimit(RLIMIT_STACK, &rlim)); + printf("(in parent, after setrlimit) RLIMIT_STACK soft limit: %lu\n", (uint64_t)rlim.rlim_cur); + + /* NOTE: we currently don't test that the stack limit is indeed enforced */ + puts("TEST OK"); + return 0; +} diff --git a/libos/test/regression/rlimit_stack.manifest.template b/libos/test/regression/rlimit_stack.manifest.template new file mode 100644 index 0000000000..c26f72bacd --- /dev/null +++ b/libos/test/regression/rlimit_stack.manifest.template @@ -0,0 +1,20 @@ +libos.entrypoint = "{{ entrypoint }}" + +loader.env.LD_LIBRARY_PATH = "/lib" + +fs.mounts = [ + { path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" }, + { path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" }, +] + +# we specify any non-standard stack size just for testing +sys.stack.size = "1M" + +sgx.max_threads = {{ '1' if env.get('EDMM', '0') == '1' else '4' }} +sgx.debug = true +sgx.edmm_enable = {{ 'true' if env.get('EDMM', '0') == '1' else 'false' }} + +sgx.trusted_files = [ + "file:{{ gramine.runtimedir(libc) }}/", + "file:{{ binary_dir }}/{{ entrypoint }}", +] diff --git a/libos/test/regression/test_libos.py b/libos/test/regression/test_libos.py index 54ed0905d4..ca7f169828 100644 --- a/libos/test/regression/test_libos.py +++ b/libos/test/regression/test_libos.py @@ -1100,6 +1100,15 @@ def test_161_rlimit_nofile_4k(self): self.assertIn("(after setrlimit) opened fd: 4096", stdout) self.assertIn("TEST OK", stdout) + def test_162_rlimit_stack(self): + # rlimit_stack.manifest.template specifies 1MB (= 1048576B) stack size + stdout, _ = self.run_binary(['rlimit_stack']) + self.assertIn("old RLIMIT_STACK soft limit: 1048576", stdout) + self.assertIn("new RLIMIT_STACK soft limit: 1048577", stdout) + self.assertIn("(in child, after setrlimit) RLIMIT_STACK soft limit: 1048577", stdout) + self.assertIn("(in parent, after setrlimit) RLIMIT_STACK soft limit: 1048577", stdout) + self.assertIn("TEST OK", stdout) + class TC_31_Syscall(RegressionTestCase): def test_000_syscall_redirect(self): stdout, _ = self.run_binary(['syscall']) diff --git a/libos/test/regression/tests.toml b/libos/test/regression/tests.toml index 1163f501e0..69394eddfe 100644 --- a/libos/test/regression/tests.toml +++ b/libos/test/regression/tests.toml @@ -104,6 +104,7 @@ manifests = [ "rename_unlink_fchown", "rlimit_nofile", "rlimit_nofile_4k", + "rlimit_stack", "run_test", "rwlock", "sched", diff --git a/libos/test/regression/tests_musl.toml b/libos/test/regression/tests_musl.toml index 86ee2a0036..7e72793d6a 100644 --- a/libos/test/regression/tests_musl.toml +++ b/libos/test/regression/tests_musl.toml @@ -106,6 +106,7 @@ manifests = [ "rename_unlink_fchown", "rlimit_nofile", "rlimit_nofile_4k", + "rlimit_stack", "run_test", "rwlock", "sched",