From 5154a5957c15bea1f84eef730fbf75829f37001b Mon Sep 17 00:00:00 2001 From: Mariusz Zaborski Date: Wed, 15 Mar 2023 08:49:58 -0400 Subject: [PATCH] [LibOS] Fix NULL pointer dereference in pseudo fs `inode::data` field keeps specific file system data. In case of pseudo fs it is a `pseudo_node`. However, when a lookup is done the `dentry` might not be fully initialized. Signed-off-by: Mariusz Zaborski --- libos/src/fs/libos_fs_pseudo.c | 4 ++++ libos/test/regression/meson.build | 1 + libos/test/regression/open_file.c | 23 ++++++++++++++++++ .../shadow_pseudo_fs.manifest.template | 24 +++++++++++++++++++ libos/test/regression/test_libos.py | 4 ++++ libos/test/regression/tests.toml | 1 + libos/test/regression/tests_musl.toml | 1 + 7 files changed, 58 insertions(+) create mode 100644 libos/test/regression/open_file.c create mode 100644 libos/test/regression/shadow_pseudo_fs.manifest.template diff --git a/libos/src/fs/libos_fs_pseudo.c b/libos/src/fs/libos_fs_pseudo.c index 94411f770e..17003636c3 100644 --- a/libos/src/fs/libos_fs_pseudo.c +++ b/libos/src/fs/libos_fs_pseudo.c @@ -44,8 +44,12 @@ static struct pseudo_node* pseudo_find(struct libos_dentry* dent) { return pseudo_find_root(dent->mount->uri); } + assert(dent->parent != NULL); assert(dent->parent->inode); + struct pseudo_node* parent_node = dent->parent->inode->data; + if (parent_node == NULL) + return NULL; /* Look for a child node with matching name */ assert(parent_node->type == PSEUDO_DIR); diff --git a/libos/test/regression/meson.build b/libos/test/regression/meson.build index 18107ccbc8..86d2cd6ec7 100644 --- a/libos/test/regression/meson.build +++ b/libos/test/regression/meson.build @@ -66,6 +66,7 @@ tests = { 'mprotect_prot_growsdown': {}, 'multi_pthread': {}, 'munmap': {}, + 'open_file': {}, 'open_opath': {}, 'openmp': { # NOTE: This will use `libgomp` in GCC and `libomp` in Clang. diff --git a/libos/test/regression/open_file.c b/libos/test/regression/open_file.c new file mode 100644 index 0000000000..7488d3a8ef --- /dev/null +++ b/libos/test/regression/open_file.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: LGPL-3.0-or-later */ +/* Copyright (C) 2023 Intel Corporation + * Mariusz Zaborski + */ + +#include +#include +#include + +#include "common.h" + +int main(int argc, const char** argv) { + if (argc != 2) { + fprintf(stderr, "This test requires a file to test"); + return 1; + } + + int fd = CHECK(open(argv[1], O_RDONLY)); + CHECK(close(fd)); + + puts("TEST OK"); + return 0; +} diff --git a/libos/test/regression/shadow_pseudo_fs.manifest.template b/libos/test/regression/shadow_pseudo_fs.manifest.template new file mode 100644 index 0000000000..0714c4a45d --- /dev/null +++ b/libos/test/regression/shadow_pseudo_fs.manifest.template @@ -0,0 +1,24 @@ +{% set entrypoint = "open_file" -%} + +loader.entrypoint = "file:{{ gramine.libos }}" +libos.entrypoint = "{{ entrypoint }}" + +loader.env.LD_LIBRARY_PATH = "/lib" +loader.argv = [ "{{ entrypoint }}", "/proc/test/nested/dirs/exec" ] + +fs.mounts = [ + { path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" }, + { path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" }, + + # Let's shadow some file in /proc as it is a pseudo fs. + { path = "/proc/test/nested/dirs/exec", uri = "file:{{ binary_dir }}/{{ entrypoint }}" }, +] + +sgx.debug = true +sgx.edmm_enable = {{ 'true' if env.get('EDMM', '0') == '1' else 'false' }} + +sgx.trusted_files = [ + "file:{{ gramine.libos }}", + "file:{{ binary_dir }}/{{ entrypoint }}", + "file:{{ gramine.runtimedir(libc) }}/", +] diff --git a/libos/test/regression/test_libos.py b/libos/test/regression/test_libos.py index 0d2fa2a97d..38d848d5dc 100644 --- a/libos/test/regression/test_libos.py +++ b/libos/test/regression/test_libos.py @@ -1070,6 +1070,10 @@ def test_021_procstat(self): # proc/stat Linux-based formatting self.assertIn('/proc/stat test passed', stdout) + def test_022_shadow_pseudo_fs(self): + stdout, _ = self.run_binary(['shadow_pseudo_fs']) + self.assertIn('TEST OK', stdout) + def test_030_fdleak(self): # The fd limit is rather arbitrary, but must be in sync with numbers from the test. # Currently test opens 10 fds simultaneously, so 50 is a safe margin for any fds that diff --git a/libos/test/regression/tests.toml b/libos/test/regression/tests.toml index f36a1c4a2f..55417b4c02 100644 --- a/libos/test/regression/tests.toml +++ b/libos/test/regression/tests.toml @@ -96,6 +96,7 @@ manifests = [ "select", "send_handle", "shared_object", + "shadow_pseudo_fs", "shebang_test_script", "sigaction_per_process", "sigaltstack", diff --git a/libos/test/regression/tests_musl.toml b/libos/test/regression/tests_musl.toml index f872d14950..b4705567e5 100644 --- a/libos/test/regression/tests_musl.toml +++ b/libos/test/regression/tests_musl.toml @@ -97,6 +97,7 @@ manifests = [ "sealed_file_mod", "select", "send_handle", + "shadow_pseudo_fs", "shared_object", "sigaction_per_process", "sigaltstack",