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; }