From 77b0e10e88f8f157b36c26f410daf3574ea5a846 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Mon, 11 Dec 2023 11:10:41 +0100 Subject: [PATCH] vfs/ram_file_system: deferred unlink This patch changes the unlink operation of the ram fs to defer the destruction of a file until it is no longer referenced by any VFS handle. When unlinked, the file no longer appears in the directory. But it can still be opened and accessed. With this change, a parent process of a Unix-like subsystem becomes able to pass the content of an unlinked file to a forked child process. This mechanism is required when using the 'exec' command in Tcl scripts. Another use case is the 'tmpfile()' function. Fixes #3577 --- .../pkg/test-libc_deferred_unlink/README | 1 + .../pkg/test-libc_deferred_unlink/archives | 4 + .../pkg/test-libc_deferred_unlink/hash | 1 + .../pkg/test-libc_deferred_unlink/runtime | 24 ++++++ .../src/test-libc_deferred_unlink/content.mk | 3 + .../src/test-libc_deferred_unlink/hash | 1 + .../src/test-libc_deferred_unlink/used_apis | 2 + .../src/test/libc_deferred_unlink/main.c | 77 +++++++++++++++++++ .../src/test/libc_deferred_unlink/target.mk | 3 + repos/os/src/lib/vfs/ram_file_system.h | 72 ++++++++++++----- 10 files changed, 168 insertions(+), 20 deletions(-) create mode 100644 repos/libports/recipes/pkg/test-libc_deferred_unlink/README create mode 100644 repos/libports/recipes/pkg/test-libc_deferred_unlink/archives create mode 100644 repos/libports/recipes/pkg/test-libc_deferred_unlink/hash create mode 100644 repos/libports/recipes/pkg/test-libc_deferred_unlink/runtime create mode 100644 repos/libports/recipes/src/test-libc_deferred_unlink/content.mk create mode 100644 repos/libports/recipes/src/test-libc_deferred_unlink/hash create mode 100644 repos/libports/recipes/src/test-libc_deferred_unlink/used_apis create mode 100644 repos/libports/src/test/libc_deferred_unlink/main.c create mode 100644 repos/libports/src/test/libc_deferred_unlink/target.mk diff --git a/repos/libports/recipes/pkg/test-libc_deferred_unlink/README b/repos/libports/recipes/pkg/test-libc_deferred_unlink/README new file mode 100644 index 0000000000..4fa0427196 --- /dev/null +++ b/repos/libports/recipes/pkg/test-libc_deferred_unlink/README @@ -0,0 +1 @@ +Test for tmp-file access after unlink diff --git a/repos/libports/recipes/pkg/test-libc_deferred_unlink/archives b/repos/libports/recipes/pkg/test-libc_deferred_unlink/archives new file mode 100644 index 0000000000..e4dbd75886 --- /dev/null +++ b/repos/libports/recipes/pkg/test-libc_deferred_unlink/archives @@ -0,0 +1,4 @@ +_/src/test-libc_deferred_unlink +_/src/libc +_/src/vfs +_/src/posix diff --git a/repos/libports/recipes/pkg/test-libc_deferred_unlink/hash b/repos/libports/recipes/pkg/test-libc_deferred_unlink/hash new file mode 100644 index 0000000000..0c3fb0a300 --- /dev/null +++ b/repos/libports/recipes/pkg/test-libc_deferred_unlink/hash @@ -0,0 +1 @@ +2023-11-29 6169da302568a61b2d2f8b01b657879e9e51acb2 diff --git a/repos/libports/recipes/pkg/test-libc_deferred_unlink/runtime b/repos/libports/recipes/pkg/test-libc_deferred_unlink/runtime new file mode 100644 index 0000000000..b1ed7b8c2c --- /dev/null +++ b/repos/libports/recipes/pkg/test-libc_deferred_unlink/runtime @@ -0,0 +1,24 @@ + + + + child * exited with exit value 0 + Error: + + + + + + + + + + + + + + + + + + + diff --git a/repos/libports/recipes/src/test-libc_deferred_unlink/content.mk b/repos/libports/recipes/src/test-libc_deferred_unlink/content.mk new file mode 100644 index 0000000000..37a1d5a8fc --- /dev/null +++ b/repos/libports/recipes/src/test-libc_deferred_unlink/content.mk @@ -0,0 +1,3 @@ +SRC_DIR = src/test/libc_deferred_unlink + +include $(GENODE_DIR)/repos/base/recipes/src/content.inc diff --git a/repos/libports/recipes/src/test-libc_deferred_unlink/hash b/repos/libports/recipes/src/test-libc_deferred_unlink/hash new file mode 100644 index 0000000000..64a7d494be --- /dev/null +++ b/repos/libports/recipes/src/test-libc_deferred_unlink/hash @@ -0,0 +1 @@ +2023-10-03 fdb0e5bff92af9d951b2bd293fd32b229b8e65b8 diff --git a/repos/libports/recipes/src/test-libc_deferred_unlink/used_apis b/repos/libports/recipes/src/test-libc_deferred_unlink/used_apis new file mode 100644 index 0000000000..30bd708b3d --- /dev/null +++ b/repos/libports/recipes/src/test-libc_deferred_unlink/used_apis @@ -0,0 +1,2 @@ +posix +libc diff --git a/repos/libports/src/test/libc_deferred_unlink/main.c b/repos/libports/src/test/libc_deferred_unlink/main.c new file mode 100644 index 0000000000..5efce60b42 --- /dev/null +++ b/repos/libports/src/test/libc_deferred_unlink/main.c @@ -0,0 +1,77 @@ +/* + * \brief Access tmp file after unlink + * \author Norman Feske + * \date 2023-12-08 + */ + +/* + * Copyright (C) 2023 Genode Labs GmbH + * + * This file is part of the Genode OS framework, which is distributed + * under the terms of the GNU Affero General Public License version 3. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +static int dir_entry_exists() +{ + DIR *dir = opendir("/tmp"); + struct dirent *dirent = NULL; + for (;;) { + dirent = readdir(dir); + if (!dirent || (strcmp(dirent->d_name, "test") == 0)) + break; + } + closedir(dir); + return (dirent != NULL); +} + + +int main(int argc, char **argv) +{ + char const * const path = "/tmp/test"; + char const * const content = "content of tmp file"; + + int const write_fd = open(path, O_RDWR|O_CREAT); + assert(write_fd >= 0); + assert(dir_entry_exists()); + + (void)unlink(path); + assert(!dir_entry_exists()); + + /* the open 'write_fd' keeps the vfs from unlinking the file now */ + size_t const num_written_bytes = write(write_fd, content, strlen(content)); + + /* open same file for reading before closing the 'write_fd' */ + int const read_fd = open(path, O_RDONLY); + assert(read_fd >= 0); + + close(write_fd); /* 'read_fd' still references the file */ + + char buf[100]; + size_t const num_read_bytes = read(read_fd, buf, sizeof(buf)); + assert(num_read_bytes == num_written_bytes); + + close(read_fd); + + /* since no fd refers to the file any longer, it is phyiscally removed now */ + int const expect_no_fd = open(path, O_RDONLY); + assert(expect_no_fd == -1); + + { + FILE *tmp = tmpfile(); + assert(tmp != NULL); + size_t fwrite_len = fwrite("123", 1, 3, tmp); + assert(fwrite_len == 3); + fclose(tmp); + } + + return 0; +} diff --git a/repos/libports/src/test/libc_deferred_unlink/target.mk b/repos/libports/src/test/libc_deferred_unlink/target.mk new file mode 100644 index 0000000000..2983608e05 --- /dev/null +++ b/repos/libports/src/test/libc_deferred_unlink/target.mk @@ -0,0 +1,3 @@ +TARGET = test-libc_deferred_unlink +SRC_C = main.c +LIBS = posix diff --git a/repos/os/src/lib/vfs/ram_file_system.h b/repos/os/src/lib/vfs/ram_file_system.h index 9b9ceada9d..a06984f81a 100644 --- a/repos/os/src/lib/vfs/ram_file_system.h +++ b/repos/os/src/lib/vfs/ram_file_system.h @@ -70,11 +70,17 @@ struct Vfs_ram::Io_handle final : public Vfs_handle, /* Track if this handle has modified its node */ bool modifying = false; + using Path = Genode::String; + + Path const path; /* needed for deferred unlink-on-close to look up the parent */ + Io_handle(Vfs::File_system &fs, Allocator &alloc, int status_flags, - Vfs_ram::Node &node) - : Vfs_handle(fs, fs, alloc, status_flags), node(node) + Vfs_ram::Node &node, + Path const &path) + : + Vfs_handle(fs, fs, alloc, status_flags), node(node), path(path) { } }; @@ -122,6 +128,8 @@ class Vfs_ram::Node : private Genode::Avl_node Vfs::Timestamp _modification_time { Vfs::Timestamp::INVALID }; + bool _marked_as_unlinked = false; + public: unsigned long inode; @@ -155,8 +163,9 @@ class Vfs_ram::Node : private Genode::Avl_node h->watch_response(); } - void unlink() { inode = 0; } - bool unlinked() const { return inode == 0; } + void mark_as_unlinked() { _marked_as_unlinked = true; } + + bool marked_as_unlinked() const { return _marked_as_unlinked; } bool update_modification_timestamp(Vfs::Timestamp time) { @@ -211,8 +220,10 @@ class Vfs_ram::Node : private Genode::Avl_node */ Node *index(size_t &i) { - if (i-- == 0) - return this; + if (!_marked_as_unlinked) { + if (i-- == 0) + return this; + } Node *n; @@ -537,7 +548,7 @@ class Vfs::Ram_file_system : public Vfs::File_system if (File * const file = dynamic_cast(node)) { if (file->opened()) { - file->unlink(); + file->mark_as_unlinked(); return; } } else if (Directory *dir = dynamic_cast(node)) { @@ -547,6 +558,18 @@ class Vfs::Ram_file_system : public Vfs::File_system destroy(_env.alloc(), node); } + void _try_complete_unlink(Vfs_ram::Directory *parent_ptr, Vfs_ram::Node &node) + { + if (node.marked_as_unlinked() && !node.opened()) { + if (parent_ptr) { + parent_ptr->release(&node); + parent_ptr->notify(); + } + node.notify(); + remove(&node); + } + } + public: Ram_file_system(Vfs::Env &env, Genode::Xml_node) : _env(env) { } @@ -616,7 +639,10 @@ class Vfs::Ram_file_system : public Vfs::File_system } try { - *handle = new (alloc) Io_handle(*this, alloc, mode, *file); + Io_handle * const io_handle_ptr = new (alloc) + Io_handle(*this, alloc, mode, *file, path); + file->open(*io_handle_ptr); + *handle = io_handle_ptr; return OPEN_OK; } catch (Genode::Out_of_ram) { if (create) { @@ -671,8 +697,10 @@ class Vfs::Ram_file_system : public Vfs::File_system } try { - *handle = new (alloc) Io_handle( - *this, alloc, Io_handle::STATUS_RDONLY, *dir); + Io_handle * const io_handle_ptr = new (alloc) + Io_handle(*this, alloc, Io_handle::STATUS_RDONLY, *dir, path); + dir->open(*io_handle_ptr); + *handle = io_handle_ptr; return OPENDIR_OK; } catch (Genode::Out_of_ram) { if (create) { @@ -727,8 +755,10 @@ class Vfs::Ram_file_system : public Vfs::File_system } try { - *handle = new (alloc) - Io_handle(*this, alloc, Io_handle::STATUS_RDWR, *link); + Io_handle * const io_handle_ptr = new (alloc) + Io_handle(*this, alloc, Io_handle::STATUS_RDWR, *link, path); + link->open(*io_handle_ptr); + *handle = io_handle_ptr; return OPENLINK_OK; } catch (Genode::Out_of_ram) { if (create) { @@ -753,14 +783,15 @@ class Vfs::Ram_file_system : public Vfs::File_system Vfs_ram::Node &node = ram_handle->node; bool node_modified = ram_handle->modifying; + Vfs_ram::Directory * const parent_ptr = lookup_parent(ram_handle->path.string()); + node.close(*ram_handle); destroy(vfs_handle->alloc(), ram_handle); - if (node.unlinked() && !node.opened()) { - destroy(_env.alloc(), &node); - } else if (node_modified) { + if (node_modified) node.notify(); - } + + _try_complete_unlink(parent_ptr, node); } Stat_result stat(char const *path, Stat &stat) override @@ -855,10 +886,11 @@ class Vfs::Ram_file_system : public Vfs::File_system if (!node) return UNLINK_ERR_NO_ENTRY; - parent->release(node); - node->notify(); - parent->notify(); - remove(node); + /* defer unlink of a node that is still referenced by an Io_handle */ + node->mark_as_unlinked(); + + _try_complete_unlink(parent, *node); + return UNLINK_OK; }