From 5dea876b8d0f4cbec7b03a0dc62731253ff5ee92 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 15:00:38 -0700 Subject: [PATCH 01/13] feat(fs): gave every filesystem node a stable identity --- kernel/drivers/graphics/gfxfb.cpp | 8 ---- kernel/drivers/input/input.cpp | 10 ----- kernel/fs/devfs/devfs.cpp | 15 +++---- kernel/fs/fs.cpp | 11 ++++- kernel/fs/fstypes.h | 3 ++ kernel/fs/mount.h | 2 + kernel/fs/node.h | 2 + kernel/fs/ramfs/ramfs.cpp | 15 +++---- kernel/fs/ramfs/ramfs.h | 1 - kernel/fs/socket_node.cpp | 11 ----- kernel/fs/socket_node.h | 2 - kernel/random/random.cpp | 8 ---- kernel/syscall/handlers/sys_fd.cpp | 4 +- kernel/sysstat/sysstat.cpp | 8 ---- kernel/terminal/console_node.cpp | 9 ----- kernel/terminal/console_node.h | 1 - kernel/tests/fs/fs.test.cpp | 65 ++++++++++++++++++++++++++++++ 17 files changed, 95 insertions(+), 80 deletions(-) diff --git a/kernel/drivers/graphics/gfxfb.cpp b/kernel/drivers/graphics/gfxfb.cpp index 74ccb589..bc62c463 100644 --- a/kernel/drivers/graphics/gfxfb.cpp +++ b/kernel/drivers/graphics/gfxfb.cpp @@ -96,14 +96,6 @@ class gfxfb_node : public fs::node { return rc; } - int32_t getattr(fs::vattr* attr) override { - if (!attr) return fs::ERR_INVAL; - - attr->type = fs::node_type::char_device; - attr->size = m_fb_size; - return fs::OK; - } - private: uint64_t m_phys; uint64_t m_width; diff --git a/kernel/drivers/input/input.cpp b/kernel/drivers/input/input.cpp index 38477ec6..94519136 100644 --- a/kernel/drivers/input/input.cpp +++ b/kernel/drivers/input/input.cpp @@ -55,16 +55,6 @@ class input_device_node : public fs::node { return ring_buffer_poll_read(m_rb, pt); } - int32_t getattr(fs::vattr* attr) override { - if (!attr) { - return fs::ERR_INVAL; - } - - attr->type = fs::node_type::char_device; - attr->size = 0; - return fs::OK; - } - private: ring_buffer* m_rb; }; diff --git a/kernel/fs/devfs/devfs.cpp b/kernel/fs/devfs/devfs.cpp index 624aa3cb..46dda8f7 100644 --- a/kernel/fs/devfs/devfs.cpp +++ b/kernel/fs/devfs/devfs.cpp @@ -61,6 +61,7 @@ class devfs_dir_node : public fs::node { string::memcpy(entries[written].name, child.name(), name_len); entries[written].name[name_len] = '\0'; entries[written].type = child.type(); + entries[written].ino = child.ino(); written++; } cur_idx++; @@ -71,9 +72,11 @@ class devfs_dir_node : public fs::node { } int32_t getattr(fs::vattr* attr) override { - if (!attr) return fs::ERR_INVAL; + int32_t rc = fs::node::getattr(attr); + if (rc != fs::OK) { + return rc; + } - attr->type = fs::node_type::directory; attr->size = m_child_count; return fs::OK; } @@ -114,14 +117,6 @@ class devfs_null_node : public fs::node { ssize_t write(fs::file*, const void*, size_t count) override { return static_cast(count); } - - int32_t getattr(fs::vattr* attr) override { - if (!attr) return fs::ERR_INVAL; - - attr->type = fs::node_type::char_device; - attr->size = 0; - return fs::OK; - } }; __PRIVILEGED_BSS static devfs_dir_node* g_devfs_root; diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index 6e2792f9..d19d7fde 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -7,6 +7,7 @@ #include "common/string.h" #include "mm/heap.h" #include "sync/spinlock.h" +#include "sync/atomic.h" #include "sync/poll.h" #include "dynpriv/dynpriv.h" #include "fs/cpio/cpio.h" @@ -30,12 +31,17 @@ __PRIVILEGED_BSS static uint32_t g_mount_count; static constexpr uint32_t MAX_MOUNTS = 32; __PRIVILEGED_BSS static mount_point* g_mounts[MAX_MOUNTS]; +// Identity counters start at 1 so that 0 always means "no identity" +static sync::atomic g_next_ino{1}; +static sync::atomic g_next_dev{1}; + node::node(node_type t, instance* fs, const char* name) : m_child_link{} , m_type(t) , m_fs(fs) , m_parent(nullptr) , m_size(0) + , m_ino(g_next_ino.fetch_add_relaxed(1)) , m_lock(sync::SPINLOCK_INIT) , m_mounted_here(nullptr) { if (name) { @@ -70,6 +76,8 @@ int32_t node::getattr(vattr* attr) { attr->type = m_type; attr->size = m_size; + attr->ino = m_ino; + attr->dev = m_fs ? m_fs->dev() : 0; return OK; } @@ -98,7 +106,8 @@ void file::ref_destroy(file* f) { instance::instance(driver* drv, node* root) : m_driver(drv) - , m_root(rc::strong_ref::adopt(root)) { + , m_root(rc::strong_ref::adopt(root)) + , m_dev(g_next_dev.fetch_add_relaxed(1)) { } int32_t instance::unmount() { diff --git a/kernel/fs/fstypes.h b/kernel/fs/fstypes.h index e392bad3..6eb87acc 100644 --- a/kernel/fs/fstypes.h +++ b/kernel/fs/fstypes.h @@ -37,11 +37,14 @@ constexpr int32_t SEEK_END = 2; struct vattr { node_type type; size_t size; + uint64_t ino; // Unique among all nodes for the node's lifetime, never 0 + uint64_t dev; // Identifies the mounted filesystem instance, 0 if unmounted }; struct dirent { char name[NAME_MAX + 1]; node_type type; + uint64_t ino; }; } // namespace fs diff --git a/kernel/fs/mount.h b/kernel/fs/mount.h index 726bc625..8dbc4b6a 100644 --- a/kernel/fs/mount.h +++ b/kernel/fs/mount.h @@ -29,12 +29,14 @@ class instance { node* root() const { return m_root.ptr(); } driver* fs_driver() const { return m_driver; } + uint64_t dev() const { return m_dev; } virtual int32_t unmount(); private: driver* m_driver; rc::strong_ref m_root; + uint64_t m_dev; public: list::node m_link; diff --git a/kernel/fs/node.h b/kernel/fs/node.h index 728e4592..bc064308 100644 --- a/kernel/fs/node.h +++ b/kernel/fs/node.h @@ -74,6 +74,7 @@ class node : public rc::ref_counted { node* parent() const { return m_parent; } const char* name() const { return m_name; } size_t size() const { return m_size; } + uint64_t ino() const { return m_ino; } instance* mounted_here() const { return m_mounted_here; } void set_parent(node* p) { m_parent = p; } @@ -88,6 +89,7 @@ class node : public rc::ref_counted { node* m_parent; char m_name[NAME_MAX + 1]; size_t m_size; + uint64_t m_ino; sync::spinlock m_lock; instance* m_mounted_here; }; diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index f57ab63b..77db5fe2 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -290,6 +290,7 @@ ssize_t dir_node::readdir(fs::file* f, fs::dirent* entries, size_t count) { string::memcpy(entries[written].name, child.name(), name_len); entries[written].name[name_len] = '\0'; entries[written].type = child.type(); + entries[written].ino = child.ino(); written++; } cur_idx++; @@ -300,9 +301,11 @@ ssize_t dir_node::readdir(fs::file* f, fs::dirent* entries, size_t count) { } int32_t dir_node::getattr(fs::vattr* attr) { - if (!attr) return fs::ERR_INVAL; + int32_t rc = fs::node::getattr(attr); + if (rc != fs::OK) { + return rc; + } - attr->type = fs::node_type::directory; attr->size = m_child_count; return fs::OK; } @@ -488,14 +491,6 @@ int64_t file_node::seek(fs::file* f, int64_t offset, int whence) { return new_off; } -int32_t file_node::getattr(fs::vattr* attr) { - if (!attr) return fs::ERR_INVAL; - - attr->type = fs::node_type::regular; - attr->size = m_size; - return fs::OK; -} - int32_t file_node::truncate(size_t size) { size_t max_alignable = ~(pmm::PAGE_SIZE - 1); if (size > max_alignable) { diff --git a/kernel/fs/ramfs/ramfs.h b/kernel/fs/ramfs/ramfs.h index 4e5aae61..f181e149 100644 --- a/kernel/fs/ramfs/ramfs.h +++ b/kernel/fs/ramfs/ramfs.h @@ -44,7 +44,6 @@ class file_node : public fs::node { ssize_t read(fs::file* f, void* buf, size_t count) override; ssize_t write(fs::file* f, const void* buf, size_t count) override; int64_t seek(fs::file* f, int64_t offset, int whence) override; - int32_t getattr(fs::vattr* attr) override; int32_t truncate(size_t size) override; private: diff --git a/kernel/fs/socket_node.cpp b/kernel/fs/socket_node.cpp index 81d5f83f..60b5ed4e 100644 --- a/kernel/fs/socket_node.cpp +++ b/kernel/fs/socket_node.cpp @@ -1,5 +1,4 @@ #include "fs/socket_node.h" -#include "fs/fs.h" #include "socket/listener.h" namespace fs { @@ -7,16 +6,6 @@ namespace fs { socket_node::socket_node(instance* fs, const char* name) : node(node_type::socket, fs, name) {} -int32_t socket_node::getattr(vattr* attr) { - if (!attr) { - return fs::ERR_INVAL; - } - - attr->type = node_type::socket; - attr->size = 0; - return fs::OK; -} - void socket_node::set_listener(rc::strong_ref ls) { m_listener = static_cast&&>(ls); } diff --git a/kernel/fs/socket_node.h b/kernel/fs/socket_node.h index d303f313..a30daa46 100644 --- a/kernel/fs/socket_node.h +++ b/kernel/fs/socket_node.h @@ -12,8 +12,6 @@ class socket_node : public node { public: socket_node(instance* fs, const char* name); - int32_t getattr(vattr* attr) override; - socket::listener_state* get_listener() const { return m_listener.ptr(); } void set_listener(rc::strong_ref ls); diff --git a/kernel/random/random.cpp b/kernel/random/random.cpp index 4a6b609e..0fb07d24 100644 --- a/kernel/random/random.cpp +++ b/kernel/random/random.cpp @@ -84,14 +84,6 @@ class urandom_node : public fs::node { return static_cast(count); } - - int32_t getattr(fs::vattr* attr) override { - if (!attr) return fs::ERR_INVAL; - - attr->type = fs::node_type::char_device; - attr->size = 0; - return fs::OK; - } }; } // anonymous namespace diff --git a/kernel/syscall/handlers/sys_fd.cpp b/kernel/syscall/handlers/sys_fd.cpp index 1a8bfc82..c25a1d61 100644 --- a/kernel/syscall/handlers/sys_fd.cpp +++ b/kernel/syscall/handlers/sys_fd.cpp @@ -198,6 +198,8 @@ static inline uint32_t node_type_default_perms(fs::node_type t) { static inline int64_t copy_stat_to_user(const fs::vattr& attr, uint64_t u_stat) { linux_kstat st = {}; + st.st_dev = attr.dev; + st.st_ino = attr.ino; st.st_mode = node_type_to_mode_bits(attr.type) | node_type_default_perms(attr.type); st.st_size = static_cast(attr.size); st.st_nlink = (attr.type == fs::node_type::directory) ? 2 : 1; @@ -1173,7 +1175,7 @@ DEFINE_SYSCALL3(getdents64, fd, dirp, count) { string::memset(record_buf, 0, reclen); linux_dirent64_hdr hdr = {}; - hdr.d_ino = 0; + hdr.d_ino = entry.ino; hdr.d_off = kfile->offset(); hdr.d_reclen = reclen; hdr.d_type = node_type_to_dirent_type(entry.type); diff --git a/kernel/sysstat/sysstat.cpp b/kernel/sysstat/sysstat.cpp index 86f2bf35..1d33880d 100644 --- a/kernel/sysstat/sysstat.cpp +++ b/kernel/sysstat/sysstat.cpp @@ -198,14 +198,6 @@ class stats_node : public fs::node { return static_cast(count); } - int32_t getattr(fs::vattr* attr) override { - if (!attr) return fs::ERR_INVAL; - - attr->type = fs::node_type::char_device; - attr->size = 0; - return fs::OK; - } - private: struct snapshot { char* text; diff --git a/kernel/terminal/console_node.cpp b/kernel/terminal/console_node.cpp index 9b9456d5..767a5b9e 100644 --- a/kernel/terminal/console_node.cpp +++ b/kernel/terminal/console_node.cpp @@ -2,7 +2,6 @@ #include "terminal/terminal.h" #include "common/ring_buffer.h" #include "serial/serial.h" -#include "fs/fs.h" #include "fs/file.h" namespace terminal { @@ -26,12 +25,4 @@ int32_t console_node::ioctl(fs::file*, uint32_t cmd, uint64_t arg) { return terminal::console_ioctl(cmd, arg); } -int32_t console_node::getattr(fs::vattr* attr) { - if (!attr) return fs::ERR_INVAL; - - attr->type = fs::node_type::char_device; - attr->size = 0; - return fs::OK; -} - } // namespace terminal diff --git a/kernel/terminal/console_node.h b/kernel/terminal/console_node.h index 8cca3816..caad681f 100644 --- a/kernel/terminal/console_node.h +++ b/kernel/terminal/console_node.h @@ -12,7 +12,6 @@ class console_node : public fs::node { ssize_t read(fs::file* f, void* buf, size_t count) override; ssize_t write(fs::file* f, const void* buf, size_t count) override; int32_t ioctl(fs::file* f, uint32_t cmd, uint64_t arg) override; - int32_t getattr(fs::vattr* attr) override; }; } // namespace terminal diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index 680f1e44..43f8079e 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -314,6 +314,71 @@ TEST(fs_test, err_notdir) { fs::unlink("/notdir_file"); } +TEST(fs_test, stat_identity_distinct_per_node) { + fs::file* a = fs::open("/ino_a", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(a); + fs::file* b = fs::open("/ino_b", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(b); + + fs::vattr attr_a = {}; + fs::vattr attr_b = {}; + EXPECT_EQ(fs::fstat(a, &attr_a), fs::OK); + EXPECT_EQ(fs::fstat(b, &attr_b), fs::OK); + EXPECT_NE(attr_a.ino, static_cast(0)); + EXPECT_NE(attr_a.ino, attr_b.ino); + EXPECT_EQ(attr_a.dev, attr_b.dev); + + fs::close(a); + fs::close(b); + fs::unlink("/ino_a"); + fs::unlink("/ino_b"); +} + +TEST(fs_test, stat_identity_stable_for_node) { + fs::file* f = fs::open("/ino_stable", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + + fs::vattr by_fd = {}; + fs::vattr by_path = {}; + EXPECT_EQ(fs::fstat(f, &by_fd), fs::OK); + EXPECT_EQ(fs::stat("/ino_stable", &by_path), fs::OK); + EXPECT_EQ(by_fd.ino, by_path.ino); + EXPECT_EQ(by_fd.dev, by_path.dev); + + fs::close(f); + fs::unlink("/ino_stable"); +} + +TEST(fs_test, stat_identity_distinct_per_filesystem) { + fs::vattr root = {}; + fs::vattr dev = {}; + EXPECT_EQ(fs::stat("/", &root), fs::OK); + EXPECT_EQ(fs::stat("/dev", &dev), fs::OK); + EXPECT_NE(root.dev, static_cast(0)); + EXPECT_NE(dev.dev, static_cast(0)); + EXPECT_NE(root.dev, dev.dev); +} + +TEST(fs_test, readdir_reports_node_identity) { + fs::mkdir("/ino_dir", 0); + fs::file* f = fs::open("/ino_dir/child", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/ino_dir/child", &attr), fs::OK); + + fs::file* dir = fs::open("/ino_dir", fs::O_RDONLY); + ASSERT_NOT_NULL(dir); + fs::dirent entry = {}; + EXPECT_EQ(fs::readdir(dir, &entry, 1), static_cast(1)); + EXPECT_EQ(entry.ino, attr.ino); + + fs::close(dir); + fs::unlink("/ino_dir/child"); + fs::rmdir("/ino_dir"); +} + TEST(fs_test, multi_page_write_read) { fs::file* f = fs::open("/bigfile", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); From 90333a3c8bd6ce3e74a514934adb1fae94022719 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 16:04:13 -0700 Subject: [PATCH 02/13] refactor(fs): shared the directory node logic between ramfs and devfs ramfs and devfs each carried their own copy of the child list, name lookup, directory listing, and attribute reporting, so every directory behavior had to be written twice and any new directory attribute could land in one filesystem and silently miss the other. A common directory base now owns the child list and the reads over it, while each filesystem keeps only the mutations it chooses to allow, which leaves ramfs writable and devfs populated solely by kernel drivers. The non-recursive teardown that ramfs already used becomes the one destructor, so nested directories in any filesystem unwind with bounded stack depth. --- kernel/fs/devfs/devfs.cpp | 95 ++------------------------ kernel/fs/dir_node.cpp | 128 +++++++++++++++++++++++++++++++++++ kernel/fs/dir_node.h | 40 +++++++++++ kernel/fs/ramfs/ramfs.cpp | 139 ++------------------------------------ kernel/fs/ramfs/ramfs.h | 15 +--- 5 files changed, 181 insertions(+), 236 deletions(-) create mode 100644 kernel/fs/dir_node.cpp create mode 100644 kernel/fs/dir_node.h diff --git a/kernel/fs/devfs/devfs.cpp b/kernel/fs/devfs/devfs.cpp index 46dda8f7..f6e680b3 100644 --- a/kernel/fs/devfs/devfs.cpp +++ b/kernel/fs/devfs/devfs.cpp @@ -1,109 +1,22 @@ #include "fs/devfs/devfs.h" -#include "fs/node.h" -#include "fs/file.h" +#include "fs/dir_node.h" #include "fs/mount.h" #include "fs/fs.h" -#include "common/list.h" #include "common/string.h" #include "mm/heap.h" #include "common/logging.h" namespace devfs { -class devfs_dir_node : public fs::node { +class devfs_dir_node : public fs::dir_node { public: devfs_dir_node(fs::instance* fs, const char* name) - : fs::node(fs::node_type::directory, fs, name) - , m_child_count(0) { - m_children.init(); - } - - ~devfs_dir_node() override { - while (!m_children.empty()) { - fs::node* child = m_children.pop_front(); - child->set_parent(nullptr); - if (child->release()) { - fs::node::ref_destroy(child); - } - } - m_child_count = 0; - } - - int32_t lookup(const char* name, size_t len, fs::node** out) override { - if (!name || !out) return fs::ERR_INVAL; - - sync::irq_lock_guard guard(m_lock); - fs::node* child = find_child(name, len); - if (!child) return fs::ERR_NOENT; - - child->add_ref(); - *out = child; - return fs::OK; - } - - ssize_t readdir(fs::file* f, fs::dirent* entries, size_t count) override { - if (!f || !entries) return fs::ERR_BADF; - - if (count == 0) return 0; - - sync::irq_lock_guard guard(m_lock); - - size_t idx = static_cast(f->offset()); - size_t written = 0; - - size_t cur_idx = 0; - for (auto& child : m_children) { - if (written >= count) break; - - if (cur_idx >= idx) { - size_t name_len = string::strlen(child.name()); - if (name_len > fs::NAME_MAX) name_len = fs::NAME_MAX; - string::memcpy(entries[written].name, child.name(), name_len); - entries[written].name[name_len] = '\0'; - entries[written].type = child.type(); - entries[written].ino = child.ino(); - written++; - } - cur_idx++; - } - - f->set_offset(static_cast(idx + written)); - return static_cast(written); - } - - int32_t getattr(fs::vattr* attr) override { - int32_t rc = fs::node::getattr(attr); - if (rc != fs::OK) { - return rc; - } - - attr->size = m_child_count; - return fs::OK; - } + : fs::dir_node(fs, name) {} void add_child(fs::node* child) { sync::irq_lock_guard guard(m_lock); - - child->set_parent(this); - child->set_filesystem(m_fs); - child->add_ref(); - m_children.push_back(child); - m_child_count++; + attach_child(child); } - -private: - fs::node* find_child(const char* name, size_t len) { - for (auto& child : m_children) { - size_t child_len = string::strlen(child.name()); - if (child_len == len && string::strncmp(child.name(), name, len) == 0) { - return &child; - } - } - return nullptr; - } - - list::head m_children; - uint32_t m_child_count; }; /* Built-in /dev/null: reads return EOF, writes are discarded. */ diff --git a/kernel/fs/dir_node.cpp b/kernel/fs/dir_node.cpp new file mode 100644 index 00000000..a3ecfb8a --- /dev/null +++ b/kernel/fs/dir_node.cpp @@ -0,0 +1,128 @@ +#include "fs/dir_node.h" +#include "fs/file.h" +#include "fs/fs.h" +#include "common/string.h" + +namespace fs { + +dir_node::dir_node(instance* fs, const char* name) + : node(node_type::directory, fs, name) + , m_child_count(0) { + m_children.init(); +} + +dir_node::~dir_node() { + // Destruction steals every directory's children into a flat worklist + // first, so destructors never recurse and stack depth stays bounded. + list::head worklist; + worklist.init(); + + while (!m_children.empty()) { + node* child = m_children.pop_front(); + child->set_parent(nullptr); + worklist.push_back(child); + } + m_child_count = 0; + + while (!worklist.empty()) { + node* n = worklist.pop_front(); + if (n->type() == node_type::directory) { + auto* dn = static_cast(n); + while (!dn->m_children.empty()) { + node* grandchild = dn->m_children.pop_front(); + grandchild->set_parent(nullptr); + worklist.push_back(grandchild); + } + dn->m_child_count = 0; + } + if (n->release()) { + node::ref_destroy(n); + } + } +} + +node* dir_node::find_child(const char* name, size_t len) { + for (auto& child : m_children) { + size_t child_len = string::strlen(child.name()); + if (child_len == len && string::strncmp(child.name(), name, len) == 0) { + return &child; + } + } + return nullptr; +} + +void dir_node::attach_child(node* child) { + child->set_parent(this); + child->set_filesystem(m_fs); + child->add_ref(); + m_children.push_back(child); + m_child_count++; +} + +void dir_node::detach_child(node* child) { + m_children.remove(child); + m_child_count--; + child->set_parent(nullptr); + + if (child->release()) { + node::ref_destroy(child); + } +} + +int32_t dir_node::lookup(const char* name, size_t len, node** out) { + if (!name || !out) return ERR_INVAL; + + sync::irq_lock_guard guard(m_lock); + node* child = find_child(name, len); + if (!child) return ERR_NOENT; + + child->add_ref(); + *out = child; + return OK; +} + +ssize_t dir_node::readdir(file* f, dirent* entries, size_t count) { + if (!f || !entries) return ERR_BADF; + + if (count == 0) return 0; + + sync::irq_lock_guard guard(m_lock); + + size_t idx = static_cast(f->offset()); + size_t written = 0; + + size_t cur_idx = 0; + for (auto& child : m_children) { + if (written >= count) { + break; + } + + if (cur_idx >= idx) { + size_t name_len = string::strlen(child.name()); + if (name_len > NAME_MAX) { + name_len = NAME_MAX; + } + string::memcpy(entries[written].name, child.name(), name_len); + entries[written].name[name_len] = '\0'; + entries[written].type = child.type(); + entries[written].ino = child.ino(); + written++; + } + cur_idx++; + } + + f->set_offset(static_cast(idx + written)); + return static_cast(written); +} + +int32_t dir_node::getattr(vattr* attr) { + int32_t rc = node::getattr(attr); + if (rc != OK) { + return rc; + } + + attr->size = m_child_count; + return OK; +} + +} // namespace fs diff --git a/kernel/fs/dir_node.h b/kernel/fs/dir_node.h new file mode 100644 index 00000000..7781e68b --- /dev/null +++ b/kernel/fs/dir_node.h @@ -0,0 +1,40 @@ +#ifndef STELLUX_FS_DIR_NODE_H +#define STELLUX_FS_DIR_NODE_H + +#include "fs/node.h" +#include "common/list.h" + +namespace fs { + +/** + * Base class for in-memory directory nodes. Owns the child list and the + * lookups over it, while each filesystem decides which mutations it allows: + * ramfs exposes create, mkdir, unlink, and rmdir, devfs is populated only by + * kernel drivers. Every directory node derives from this class, which the + * destructor relies on to tear down nested directories without recursion. + */ +class dir_node : public node { +public: + dir_node(instance* fs, const char* name); + ~dir_node() override; + + int32_t lookup(const char* name, size_t len, node** out) override; + ssize_t readdir(file* f, dirent* entries, size_t count) override; + int32_t getattr(vattr* attr) override; + + uint32_t child_count() const { return m_child_count; } + +protected: + // Callers hold m_lock across a find and the attach or detach it decides + node* find_child(const char* name, size_t len); + void attach_child(node* child); + void detach_child(node* child); + +private: + list::head m_children; + uint32_t m_child_count; +}; + +} // namespace fs + +#endif // STELLUX_FS_DIR_NODE_H diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index 77db5fe2..2be2643b 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -56,61 +56,7 @@ extern "C" __PRIVILEGED_CODE int32_t ramfs_init_driver() { namespace ramfs { dir_node::dir_node(fs::instance* fs, const char* name) - : fs::node(fs::node_type::directory, fs, name) - , m_child_count(0) { - m_children.init(); -} - -dir_node::~dir_node() { - // Destruction steals every directory's children into a flat worklist - // first, so destructors never recurse and stack depth stays bounded. - list::head worklist; - worklist.init(); - - while (!m_children.empty()) { - fs::node* child = m_children.pop_front(); - child->set_parent(nullptr); - worklist.push_back(child); - } - m_child_count = 0; - - while (!worklist.empty()) { - fs::node* n = worklist.pop_front(); - if (n->type() == fs::node_type::directory) { - auto* dn = static_cast(n); - while (!dn->m_children.empty()) { - fs::node* grandchild = dn->m_children.pop_front(); - grandchild->set_parent(nullptr); - worklist.push_back(grandchild); - } - dn->m_child_count = 0; - } - if (n->release()) { - fs::node::ref_destroy(n); - } - } -} - -fs::node* dir_node::find_child(const char* name, size_t len) { - for (auto& child : m_children) { - size_t child_len = string::strlen(child.name()); - if (child_len == len && string::strncmp(child.name(), name, len) == 0) { - return &child; - } - } - return nullptr; -} - -int32_t dir_node::lookup(const char* name, size_t len, fs::node** out) { - if (!name || !out) return fs::ERR_INVAL; - - sync::irq_lock_guard guard(m_lock); - fs::node* child = find_child(name, len); - if (!child) return fs::ERR_NOENT; - - child->add_ref(); - *out = child; - return fs::OK; + : fs::dir_node(fs, name) { } int32_t dir_node::create(const char* name, size_t len, uint32_t mode, fs::node** out) { @@ -136,12 +82,8 @@ int32_t dir_node::create(const char* name, size_t len, uint32_t mode, fs::node** } auto* child = new (mem) file_node(m_fs, name_buf); + attach_child(child); - child->set_parent(this); - m_children.push_back(child); - m_child_count++; - - child->add_ref(); *out = child; return fs::OK; } @@ -168,12 +110,8 @@ int32_t dir_node::create_socket(const char* name, size_t len, void* impl, fs::no } auto* child = new (mem) fs::socket_node(m_fs, name_buf); + attach_child(child); - child->set_parent(this); - m_children.push_back(child); - m_child_count++; - - child->add_ref(); *out = child; return fs::OK; } @@ -201,12 +139,8 @@ int32_t dir_node::mkdir(const char* name, size_t len, uint32_t mode, fs::node** } auto* child = new (mem) dir_node(m_fs, name_buf); + attach_child(child); - child->set_parent(this); - m_children.push_back(child); - m_child_count++; - - child->add_ref(); *out = child; return fs::OK; } @@ -225,14 +159,7 @@ int32_t dir_node::unlink(const char* name, size_t len) { return fs::ERR_ISDIR; } - m_children.remove(child); - m_child_count--; - child->set_parent(nullptr); - - if (child->release()) { - fs::node::ref_destroy(child); - } - + detach_child(child); return fs::OK; } @@ -250,63 +177,11 @@ int32_t dir_node::rmdir(const char* name, size_t len) { return fs::ERR_NOTDIR; } - auto* child_dir = static_cast(child); - if (child_dir->m_child_count > 0) { + if (static_cast(child)->child_count() > 0) { return fs::ERR_NOTEMPTY; } - m_children.remove(child); - m_child_count--; - child->set_parent(nullptr); - - if (child->release()) { - fs::node::ref_destroy(child); - } - - return fs::OK; -} - -ssize_t dir_node::readdir(fs::file* f, fs::dirent* entries, size_t count) { - if (!f || !entries) return fs::ERR_BADF; - - if (count == 0) return 0; - - sync::irq_lock_guard guard(m_lock); - - size_t idx = static_cast(f->offset()); - size_t written = 0; - - size_t cur_idx = 0; - for (auto& child : m_children) { - if (written >= count) { - break; - } - - if (cur_idx >= idx) { - size_t name_len = string::strlen(child.name()); - if (name_len > fs::NAME_MAX) { - name_len = fs::NAME_MAX; - } - string::memcpy(entries[written].name, child.name(), name_len); - entries[written].name[name_len] = '\0'; - entries[written].type = child.type(); - entries[written].ino = child.ino(); - written++; - } - cur_idx++; - } - - f->set_offset(static_cast(idx + written)); - return static_cast(written); -} - -int32_t dir_node::getattr(fs::vattr* attr) { - int32_t rc = fs::node::getattr(attr); - if (rc != fs::OK) { - return rc; - } - - attr->size = m_child_count; + detach_child(child); return fs::OK; } diff --git a/kernel/fs/ramfs/ramfs.h b/kernel/fs/ramfs/ramfs.h index f181e149..ef8c18fa 100644 --- a/kernel/fs/ramfs/ramfs.h +++ b/kernel/fs/ramfs/ramfs.h @@ -1,11 +1,10 @@ #ifndef STELLUX_FS_RAMFS_RAMFS_H #define STELLUX_FS_RAMFS_RAMFS_H -#include "fs/node.h" +#include "fs/dir_node.h" #include "fs/file.h" #include "fs/mount.h" #include "fs/fs.h" -#include "common/list.h" namespace ramfs { @@ -15,25 +14,15 @@ namespace ramfs { */ __PRIVILEGED_CODE int32_t init(); -class dir_node : public fs::node { +class dir_node : public fs::dir_node { public: dir_node(fs::instance* fs, const char* name); - ~dir_node() override; - int32_t lookup(const char* name, size_t len, fs::node** out) override; int32_t create(const char* name, size_t len, uint32_t mode, fs::node** out) override; int32_t mkdir(const char* name, size_t len, uint32_t mode, fs::node** out) override; int32_t unlink(const char* name, size_t len) override; int32_t rmdir(const char* name, size_t len) override; - ssize_t readdir(fs::file* f, fs::dirent* entries, size_t count) override; - int32_t getattr(fs::vattr* attr) override; int32_t create_socket(const char* name, size_t len, void* impl, fs::node** out) override; - -private: - fs::node* find_child(const char* name, size_t len); - - list::head m_children; - uint32_t m_child_count; }; class file_node : public fs::node { From 17db6b1e8615515ef4fa68eafd0452c6dc6829b1 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 16:52:02 -0700 Subject: [PATCH 03/13] fix(clock): exposed the boot epoch as soon as the RTC had been read --- kernel/arch/aarch64/clock/clock.cpp | 8 -------- kernel/arch/x86_64/clock/clock.cpp | 8 -------- kernel/clock/clock.h | 13 ++++++++++--- kernel/syscall/handlers/sys_clock.cpp | 8 ++------ 4 files changed, 12 insertions(+), 25 deletions(-) diff --git a/kernel/arch/aarch64/clock/clock.cpp b/kernel/arch/aarch64/clock/clock.cpp index 29d8cd6f..cce85c45 100644 --- a/kernel/arch/aarch64/clock/clock.cpp +++ b/kernel/arch/aarch64/clock/clock.cpp @@ -1,6 +1,5 @@ #include "clock/clock.h" #include "hw/hwtimer.h" -#include "hw/rtc.h" #include "common/logging.h" #include "sync/atomic.h" @@ -10,7 +9,6 @@ static uint64_t g_cnt_freq; static uint64_t g_mult; static uint32_t g_shift; static sync::atomic g_calibrated; -static uint64_t g_boot_realtime_ns; constexpr uint64_t NS_PER_SEC = 1000000000ULL; @@ -58,7 +56,6 @@ __PRIVILEGED_CODE int32_t init() { } compute_mult_shift(g_cnt_freq, &g_mult, &g_shift); - g_boot_realtime_ns = rtc::boot_unix_ns(); enable_el0_counter_access(); g_calibrated.store_release(true); @@ -94,9 +91,4 @@ uint64_t now_ns() { uint64_t freq_hz() { return g_cnt_freq; } - -uint64_t boot_realtime_ns() { - return g_boot_realtime_ns; -} - } // namespace clock diff --git a/kernel/arch/x86_64/clock/clock.cpp b/kernel/arch/x86_64/clock/clock.cpp index 09b057e5..ca3e7911 100644 --- a/kernel/arch/x86_64/clock/clock.cpp +++ b/kernel/arch/x86_64/clock/clock.cpp @@ -3,7 +3,6 @@ #include "hw/portio.h" #include "hw/mmio.h" #include "hw/cpu.h" -#include "hw/rtc.h" #include "hw/cpu_features.h" #include "common/logging.h" #include "sync/atomic.h" @@ -14,7 +13,6 @@ static uint64_t g_tsc_freq; static uint64_t g_mult; static uint32_t g_shift; static sync::atomic g_calibrated; -static uint64_t g_boot_realtime_ns; constexpr uint64_t NS_PER_SEC = 1000000000ULL; @@ -91,7 +89,6 @@ __PRIVILEGED_CODE int32_t init() { } compute_mult_shift(g_tsc_freq, &g_mult, &g_shift); - g_boot_realtime_ns = rtc::boot_unix_ns(); g_calibrated.store_release(true); log::info("clock: TSC freq=%lu Hz, mult=%lu shift=%u%s", @@ -126,9 +123,4 @@ uint64_t now_ns() { uint64_t freq_hz() { return g_tsc_freq; } - -uint64_t boot_realtime_ns() { - return g_boot_realtime_ns; -} - } // namespace clock diff --git a/kernel/clock/clock.h b/kernel/clock/clock.h index 20afefa2..769864d8 100644 --- a/kernel/clock/clock.h +++ b/kernel/clock/clock.h @@ -2,6 +2,7 @@ #define STELLUX_CLOCK_CLOCK_H #include "common/types.h" +#include "hw/rtc.h" namespace clock { @@ -43,10 +44,16 @@ uint64_t freq_hz(); /** * @brief Unix epoch in nanoseconds at boot time. - * Returns 0 if no RTC was available or rtc::init() was not called. - * Unprivileged: reads a cached value from regular .bss. + * Valid as soon as rtc::init() has run, which precedes init() here. + * Returns 0 if no RTC was available. */ -uint64_t boot_realtime_ns(); +inline uint64_t boot_realtime_ns() { return rtc::boot_unix_ns(); } + +/** + * @brief Unix epoch in nanoseconds now. Counts from 0 when no RTC was + * available, so ordering between readings holds even without wall time. + */ +inline uint64_t realtime_ns() { return boot_realtime_ns() + now_ns(); } } // namespace clock diff --git a/kernel/syscall/handlers/sys_clock.cpp b/kernel/syscall/handlers/sys_clock.cpp index ac1b1ff2..2fd9675e 100644 --- a/kernel/syscall/handlers/sys_clock.cpp +++ b/kernel/syscall/handlers/sys_clock.cpp @@ -38,10 +38,6 @@ static uint64_t get_monotonic_ns() { return clock::now_ns(); } -static uint64_t get_realtime_ns() { - return clock::boot_realtime_ns() + clock::now_ns(); -} - DEFINE_SYSCALL2(clock_gettime, clock_id, u_tp) { if (u_tp == 0) { return syscall::EFAULT; @@ -62,7 +58,7 @@ DEFINE_SYSCALL2(clock_gettime, clock_id, u_tp) { return syscall::EINVAL; } - ns = get_realtime_ns(); + ns = clock::realtime_ns(); break; case CLOCK_THREAD_CPUTIME_ID: return syscall::EINVAL; @@ -124,7 +120,7 @@ DEFINE_SYSCALL2(gettimeofday, u_tv, u_tz) { return syscall::EINVAL; } - uint64_t ns = get_realtime_ns(); + uint64_t ns = clock::realtime_ns(); kernel_timeval tv; tv.tv_sec = static_cast(ns / NS_PER_SEC); tv.tv_usec = static_cast((ns % NS_PER_SEC) / 1000); From a054168959eb3a6fa1d892add8ae9eef6714b5d3 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 16:52:39 -0700 Subject: [PATCH 04/13] feat(fs): gave filesystem nodes access, modification, and change times --- kernel/fs/cpio/cpio.cpp | 9 ++++ kernel/fs/dir_node.cpp | 2 + kernel/fs/fs.cpp | 42 +++++++++++++++ kernel/fs/fs.h | 1 + kernel/fs/fstypes.h | 11 +++- kernel/fs/node.h | 7 +++ kernel/fs/ramfs/ramfs.cpp | 4 ++ kernel/syscall/handlers/sys_fd.cpp | 12 ++++- kernel/tests/fs/fs.test.cpp | 87 ++++++++++++++++++++++++++++++ 9 files changed, 172 insertions(+), 3 deletions(-) diff --git a/kernel/fs/cpio/cpio.cpp b/kernel/fs/cpio/cpio.cpp index 72659a95..a4d37dd0 100644 --- a/kernel/fs/cpio/cpio.cpp +++ b/kernel/fs/cpio/cpio.cpp @@ -14,6 +14,7 @@ constexpr char CPIO_TRAILER[] = "TRAILER!!!"; constexpr uint32_t S_IFMT = 0170000; constexpr uint32_t S_IFDIR = 0040000; constexpr uint32_t S_IFREG = 0100000; +constexpr uint64_t NS_PER_SEC = 1000000000ULL; static uint32_t hex_to_u32(const char* s, size_t len) { uint32_t val = 0; @@ -100,6 +101,7 @@ __PRIVILEGED_CODE int32_t load_initrd() { uint32_t namesize = hex_to_u32(hdr->c_namesize, 8); uint32_t filesize = hex_to_u32(hdr->c_filesize, 8); uint32_t mode = hex_to_u32(hdr->c_mode, 8); + uint32_t mtime = hex_to_u32(hdr->c_mtime, 8); offset += sizeof(cpio_newc_header); @@ -159,6 +161,13 @@ __PRIVILEGED_CODE int32_t load_initrd() { if (filesize > 0 && offset + filesize <= archive_len) { fs::write(f, file_data, filesize); } + + // Files keep their archived timestamps so build tools see real ages + fs::vattr attr = {}; + attr.atime_ns = static_cast(mtime) * NS_PER_SEC; + attr.mtime_ns = attr.atime_ns; + fs::fsetattr(f, attr, fs::VATTR_ATIME | fs::VATTR_MTIME); + fs::close(f); files_extracted++; } else { diff --git a/kernel/fs/dir_node.cpp b/kernel/fs/dir_node.cpp index a3ecfb8a..889cc1e3 100644 --- a/kernel/fs/dir_node.cpp +++ b/kernel/fs/dir_node.cpp @@ -57,12 +57,14 @@ void dir_node::attach_child(node* child) { child->add_ref(); m_children.push_back(child); m_child_count++; + mark_modified(); } void dir_node::detach_child(node* child) { m_children.remove(child); m_child_count--; child->set_parent(nullptr); + mark_modified(); if (child->release()) { node::ref_destroy(child); diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index d19d7fde..ee40775d 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -3,6 +3,7 @@ #include "fs/file.h" #include "fs/mount.h" #include "fs/path.h" +#include "clock/clock.h" #include "common/logging.h" #include "common/string.h" #include "mm/heap.h" @@ -42,6 +43,9 @@ node::node(node_type t, instance* fs, const char* name) , m_parent(nullptr) , m_size(0) , m_ino(g_next_ino.fetch_add_relaxed(1)) + , m_atime_ns(clock::realtime_ns()) + , m_mtime_ns(m_atime_ns) + , m_ctime_ns(m_atime_ns) , m_lock(sync::SPINLOCK_INIT) , m_mounted_here(nullptr) { if (name) { @@ -78,9 +82,36 @@ int32_t node::getattr(vattr* attr) { attr->size = m_size; attr->ino = m_ino; attr->dev = m_fs ? m_fs->dev() : 0; + attr->atime_ns = m_atime_ns; + attr->mtime_ns = m_mtime_ns; + attr->ctime_ns = m_ctime_ns; + + return OK; +} + +int32_t node::setattr(const vattr& attr, uint32_t mask) { + if (mask & ~(VATTR_ATIME | VATTR_MTIME)) return ERR_INVAL; + + if (mask == 0) return OK; + + if (mask & VATTR_ATIME) { + m_atime_ns = attr.atime_ns; + } + + if (mask & VATTR_MTIME) { + m_mtime_ns = attr.mtime_ns; + } + + m_ctime_ns = clock::realtime_ns(); + return OK; } +void node::mark_modified() { + m_mtime_ns = clock::realtime_ns(); + m_ctime_ns = m_mtime_ns; +} + __PRIVILEGED_CODE void node::ref_destroy(node* n) { n->~node(); heap::kfree(n); @@ -799,6 +830,17 @@ int32_t fstat(file* f, vattr* attr) { return result; } +int32_t fsetattr(file* f, const vattr& attr, uint32_t mask) { + if (!f) return ERR_BADF; + + int32_t result; + RUN_ELEVATED({ + result = f->get_node()->setattr(attr, mask); + }); + + return result; +} + int32_t mkdir(const char* path, uint32_t mode) { if (!path || path[0] != '/') return ERR_INVAL; diff --git a/kernel/fs/fs.h b/kernel/fs/fs.h index 993c08e8..b509b71d 100644 --- a/kernel/fs/fs.h +++ b/kernel/fs/fs.h @@ -69,6 +69,7 @@ int32_t mmap(file* f, mm::mm_context* mm_ctx, uintptr_t addr, size_t length, int32_t stat(const char* path, vattr* attr); int32_t fstat(file* f, vattr* attr); +int32_t fsetattr(file* f, const vattr& attr, uint32_t mask); int32_t mkdir(const char* path, uint32_t mode); int32_t rmdir(const char* path); diff --git a/kernel/fs/fstypes.h b/kernel/fs/fstypes.h index 6eb87acc..e55831d4 100644 --- a/kernel/fs/fstypes.h +++ b/kernel/fs/fstypes.h @@ -37,10 +37,17 @@ constexpr int32_t SEEK_END = 2; struct vattr { node_type type; size_t size; - uint64_t ino; // Unique among all nodes for the node's lifetime, never 0 - uint64_t dev; // Identifies the mounted filesystem instance, 0 if unmounted + uint64_t ino; // Unique among all nodes for the node's lifetime, never 0 + uint64_t dev; // Identifies the mounted filesystem instance, 0 if unmounted + uint64_t atime_ns; // Last access, Unix epoch nanoseconds, reads do not update it + uint64_t mtime_ns; // Last content change + uint64_t ctime_ns; // Last content or attribute change, never set by callers }; +// setattr mask bits naming the vattr fields to apply +constexpr uint32_t VATTR_ATIME = 1u << 0; +constexpr uint32_t VATTR_MTIME = 1u << 1; + struct dirent { char name[NAME_MAX + 1]; node_type type; diff --git a/kernel/fs/node.h b/kernel/fs/node.h index bc064308..1d16314a 100644 --- a/kernel/fs/node.h +++ b/kernel/fs/node.h @@ -54,6 +54,7 @@ class node : public rc::ref_counted { // --- Metadata --- virtual int32_t getattr(vattr* attr); + virtual int32_t setattr(const vattr& attr, uint32_t mask); virtual int32_t truncate(size_t size); // --- Symlink --- @@ -84,12 +85,18 @@ class node : public rc::ref_counted { list::node m_child_link; protected: + // Records a content change by moving mtime and ctime to now + void mark_modified(); + node_type m_type; instance* m_fs; node* m_parent; char m_name[NAME_MAX + 1]; size_t m_size; uint64_t m_ino; + uint64_t m_atime_ns; + uint64_t m_mtime_ns; + uint64_t m_ctime_ns; sync::spinlock m_lock; instance* m_mounted_here; }; diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index 2be2643b..7e2c977c 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -334,7 +334,9 @@ ssize_t file_node::write(fs::file* f, const void* buf, size_t count) { m_size = end_pos; } + mark_modified(); f->set_offset(static_cast(end_pos)); + return static_cast(count); } @@ -415,6 +417,8 @@ int32_t file_node::truncate(size_t size) { } m_size = size; + mark_modified(); + return fs::OK; } diff --git a/kernel/syscall/handlers/sys_fd.cpp b/kernel/syscall/handlers/sys_fd.cpp index c25a1d61..e6c736e3 100644 --- a/kernel/syscall/handlers/sys_fd.cpp +++ b/kernel/syscall/handlers/sys_fd.cpp @@ -37,6 +37,8 @@ constexpr uint8_t DT_REG = 8; constexpr uint8_t DT_LNK = 10; constexpr uint8_t DT_SOCK = 12; +constexpr uint64_t NS_PER_SEC = 1000000000ULL; + namespace { struct linux_dirent64_hdr { @@ -201,11 +203,19 @@ static inline int64_t copy_stat_to_user(const fs::vattr& attr, uint64_t u_stat) st.st_dev = attr.dev; st.st_ino = attr.ino; st.st_mode = node_type_to_mode_bits(attr.type) | node_type_default_perms(attr.type); - st.st_size = static_cast(attr.size); st.st_nlink = (attr.type == fs::node_type::directory) ? 2 : 1; + + st.st_size = static_cast(attr.size); st.st_blksize = 4096; st.st_blocks = static_cast((attr.size + 511) / 512); + st.st_atime_sec = static_cast(attr.atime_ns / NS_PER_SEC); + st.st_atime_nsec = static_cast(attr.atime_ns % NS_PER_SEC); + st.st_mtime_sec = static_cast(attr.mtime_ns / NS_PER_SEC); + st.st_mtime_nsec = static_cast(attr.mtime_ns % NS_PER_SEC); + st.st_ctime_sec = static_cast(attr.ctime_ns / NS_PER_SEC); + st.st_ctime_nsec = static_cast(attr.ctime_ns % NS_PER_SEC); + int32_t copy_rc = mm::uaccess::copy_to_user( reinterpret_cast(u_stat), &st, sizeof(st)); if (copy_rc != mm::uaccess::OK) { diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index 43f8079e..91962216 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -379,6 +379,93 @@ TEST(fs_test, readdir_reports_node_identity) { fs::rmdir("/ino_dir"); } +TEST(fs_test, timestamps_start_equal_and_nonzero) { + fs::file* f = fs::open("/ts_new", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::fstat(f, &attr), fs::OK); + EXPECT_NE(attr.mtime_ns, static_cast(0)); + EXPECT_EQ(attr.atime_ns, attr.mtime_ns); + EXPECT_EQ(attr.ctime_ns, attr.mtime_ns); + + fs::close(f); + fs::unlink("/ts_new"); +} + +TEST(fs_test, setattr_applies_times_and_bumps_ctime) { + fs::file* f = fs::open("/ts_set", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + + fs::vattr before = {}; + EXPECT_EQ(fs::fstat(f, &before), fs::OK); + + fs::vattr want = {}; + want.atime_ns = 1000; + want.mtime_ns = 2000; + EXPECT_EQ(fs::fsetattr(f, want, fs::VATTR_ATIME | fs::VATTR_MTIME), fs::OK); + + fs::vattr after = {}; + EXPECT_EQ(fs::fstat(f, &after), fs::OK); + EXPECT_EQ(after.atime_ns, static_cast(1000)); + EXPECT_EQ(after.mtime_ns, static_cast(2000)); + EXPECT_GE(after.ctime_ns, before.ctime_ns); + + fs::close(f); + fs::unlink("/ts_set"); +} + +TEST(fs_test, setattr_rejects_unknown_mask) { + fs::file* f = fs::open("/ts_mask", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::fsetattr(f, attr, 1u << 31), fs::ERR_INVAL); + + fs::close(f); + fs::unlink("/ts_mask"); +} + +TEST(fs_test, write_moves_mtime_forward) { + fs::file* f = fs::open("/ts_write", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + + fs::vattr past = {}; + past.mtime_ns = 1; + EXPECT_EQ(fs::fsetattr(f, past, fs::VATTR_MTIME), fs::OK); + EXPECT_EQ(fs::write(f, "x", 1), static_cast(1)); + + fs::vattr attr = {}; + EXPECT_EQ(fs::fstat(f, &attr), fs::OK); + EXPECT_GT(attr.mtime_ns, static_cast(1)); + EXPECT_EQ(attr.ctime_ns, attr.mtime_ns); + + fs::close(f); + fs::unlink("/ts_write"); +} + +TEST(fs_test, directory_mtime_tracks_entries) { + EXPECT_EQ(fs::mkdir("/ts_dir", 0), fs::OK); + fs::file* dir = fs::open("/ts_dir", fs::O_RDONLY); + ASSERT_NOT_NULL(dir); + + fs::vattr past = {}; + past.mtime_ns = 1; + EXPECT_EQ(fs::fsetattr(dir, past, fs::VATTR_MTIME), fs::OK); + + fs::file* child = fs::open("/ts_dir/child", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(child); + fs::close(child); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/ts_dir", &attr), fs::OK); + EXPECT_GT(attr.mtime_ns, static_cast(1)); + + fs::close(dir); + fs::unlink("/ts_dir/child"); + fs::rmdir("/ts_dir"); +} + TEST(fs_test, multi_page_write_read) { fs::file* f = fs::open("/bigfile", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); From edbadfadf4f564fe0c6564c08611255dcae05600 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 17:46:15 -0700 Subject: [PATCH 05/13] fix(fs): truncated regular files opened with O_TRUNC --- kernel/fs/fs.cpp | 6 ++++++ kernel/tests/fs/fs.test.cpp | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index ee40775d..c5b60124 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -709,6 +709,12 @@ file* open_at(node* base_dir, const char* path, uint32_t flags, int32_t* out_err RUN_ELEVATED({ err = n->open(f, flags); + + // O_TRUNC only applies to regular files opened for writing + bool writable = (flags & ACCESS_MODE_MASK) != O_RDONLY; + if (err == OK && (flags & O_TRUNC) && writable && n->type() == node_type::regular) { + err = n->truncate(0); + } }); if (err != OK) { diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index 91962216..4ed7275d 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -466,6 +466,40 @@ TEST(fs_test, directory_mtime_tracks_entries) { fs::rmdir("/ts_dir"); } +TEST(fs_test, open_trunc_empties_writable_file) { + fs::file* f = fs::open("/trunc_rw", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "0123456789", 10), static_cast(10)); + fs::close(f); + + f = fs::open("/trunc_rw", fs::O_WRONLY | fs::O_TRUNC); + ASSERT_NOT_NULL(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::fstat(f, &attr), fs::OK); + EXPECT_EQ(attr.size, static_cast(0)); + + fs::close(f); + fs::unlink("/trunc_rw"); +} + +TEST(fs_test, open_trunc_ignored_for_read_only) { + fs::file* f = fs::open("/trunc_ro", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "0123456789", 10), static_cast(10)); + fs::close(f); + + f = fs::open("/trunc_ro", fs::O_RDONLY | fs::O_TRUNC); + ASSERT_NOT_NULL(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::fstat(f, &attr), fs::OK); + EXPECT_EQ(attr.size, static_cast(10)); + + fs::close(f); + fs::unlink("/trunc_ro"); +} + TEST(fs_test, multi_page_write_read) { fs::file* f = fs::open("/bigfile", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); From efa3d844ab00285a41a2d5217541f241b2e2f0f8 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 20:12:23 -0700 Subject: [PATCH 06/13] feat(fs): added rename with atomic replacement of the target --- kernel/fs/dir_node.cpp | 149 ++++++++++++++++++++++++ kernel/fs/dir_node.h | 10 ++ kernel/fs/fs.cpp | 52 +++++++++ kernel/fs/fs.h | 2 + kernel/fs/node.h | 6 + kernel/fs/ramfs/ramfs.cpp | 5 + kernel/fs/ramfs/ramfs.h | 2 + kernel/syscall/handlers/sys_error_map.h | 2 + kernel/syscall/handlers/sys_fd.cpp | 85 +++++++++++++- kernel/syscall/handlers/sys_fd.h | 1 + kernel/syscall/syscall_table.cpp | 2 +- kernel/syscall/syscall_table.h | 1 + kernel/tests/fs/fs.test.cpp | 131 +++++++++++++++++++++ kernel/tests/resource/resource.test.cpp | 9 +- 14 files changed, 449 insertions(+), 8 deletions(-) diff --git a/kernel/fs/dir_node.cpp b/kernel/fs/dir_node.cpp index 889cc1e3..f1b99249 100644 --- a/kernel/fs/dir_node.cpp +++ b/kernel/fs/dir_node.cpp @@ -5,6 +5,30 @@ namespace fs { +// Every rename serializes on this lock, which keeps the ancestor walk that +// refuses to move a directory into its own subtree reading a stable tree and +// makes locking a replaced directory beneath its parents deadlock free +static sync::spinlock g_rename_lock = sync::SPINLOCK_INIT; + +static bool is_dot_name(const char* name, size_t len) { + return (len == 1 && name[0] == '.') || (len == 2 && name[0] == '.' && name[1] == '.'); +} + +// True when dir is ancestor itself or lies anywhere beneath it +static bool is_within(node* dir, node* ancestor) { + for (node* n = dir; n; n = n->parent()) { + if (n == ancestor) { + return true; + } + + if (n->parent() == n) { + break; + } + } + + return false; +} + dir_node::dir_node(instance* fs, const char* name) : node(node_type::directory, fs, name) , m_child_count(0) { @@ -71,6 +95,131 @@ void dir_node::detach_child(node* child) { } } +int32_t dir_node::rename_child(const char* name, size_t len, node* new_parent, + const char* new_name, size_t new_len) { + if (!name || len == 0 || !new_parent || !new_name || new_len == 0) { + return ERR_INVAL; + } + + if (new_len > NAME_MAX) { + return ERR_NAMETOOLONG; + } + + if (is_dot_name(name, len) || is_dot_name(new_name, new_len)) { + return ERR_INVAL; + } + + if (new_parent->type() != node_type::directory) { + return ERR_NOTDIR; + } + + if (new_parent->filesystem() != m_fs) { + return ERR_XDEV; + } + + auto* dst = static_cast(new_parent); + sync::irq_lock_guard rename_guard(g_rename_lock); + + if (dst == this) { + sync::irq_lock_guard guard(m_lock); + return move_child_locked(name, len, dst, new_name, new_len); + } + + // Two directories lock in address order so concurrent renames in + // opposite directions cannot deadlock + dir_node* first = reinterpret_cast(this) < reinterpret_cast(dst) ? this : dst; + dir_node* second = first == this ? dst : this; + + sync::irq_lock_guard first_guard(first->m_lock); + sync::irq_lock_guard second_guard(second->m_lock); + + return move_child_locked(name, len, dst, new_name, new_len); +} + +int32_t dir_node::detach_empty_dir_locked(dir_node* dir) { + sync::irq_lock_guard guard(dir->m_lock); + + if (dir->mounted_here()) { + return ERR_BUSY; + } + + if (dir->m_child_count > 0) { + return ERR_NOTEMPTY; + } + + detach_child(dir); + return OK; +} + +int32_t dir_node::replace_child_locked(node* child, node* existing) { + bool child_is_dir = child->type() == node_type::directory; + bool existing_is_dir = existing->type() == node_type::directory; + + if (existing_is_dir && !child_is_dir) { + return ERR_ISDIR; + } + + if (!existing_is_dir && child_is_dir) { + return ERR_NOTDIR; + } + + if (!existing_is_dir) { + detach_child(existing); + return OK; + } + + // A temporary reference keeps the directory alive while its own lock is + // held across the detach, since the list reference may be its last + existing->add_ref(); + int32_t rc = detach_empty_dir_locked(static_cast(existing)); + if (existing->release()) { + node::ref_destroy(existing); + } + + return rc; +} + +int32_t dir_node::move_child_locked(const char* name, size_t len, dir_node* dst, + const char* new_name, size_t new_len) { + node* child = find_child(name, len); + if (!child) { + return ERR_NOENT; + } + + if (child->mounted_here()) { + return ERR_BUSY; + } + + node* existing = dst->find_child(new_name, new_len); + if (existing == child) { + return OK; + } + + if (child->type() == node_type::directory && is_within(dst, child)) { + return ERR_INVAL; + } + + if (existing) { + int32_t rc = dst->replace_child_locked(child, existing); + if (rc != OK) { + return rc; + } + } + + // A temporary reference keeps the child alive between the two directories + child->add_ref(); + detach_child(child); + child->set_name(new_name, new_len); + child->mark_changed(); + dst->attach_child(child); + + if (child->release()) { + node::ref_destroy(child); + } + + return OK; +} + int32_t dir_node::lookup(const char* name, size_t len, node** out) { if (!name || !out) return ERR_INVAL; diff --git a/kernel/fs/dir_node.h b/kernel/fs/dir_node.h index 7781e68b..b2ac0aa5 100644 --- a/kernel/fs/dir_node.h +++ b/kernel/fs/dir_node.h @@ -30,7 +30,17 @@ class dir_node : public node { void attach_child(node* child); void detach_child(node* child); + // Moves a child under new_parent as new_name, replacing an existing + // entry there when the types allow. Takes every lock it needs itself. + int32_t rename_child(const char* name, size_t len, node* new_parent, + const char* new_name, size_t new_len); + private: + int32_t move_child_locked(const char* name, size_t len, dir_node* dst, + const char* new_name, size_t new_len); + int32_t replace_child_locked(node* child, node* existing); + int32_t detach_empty_dir_locked(dir_node* dir); + list::head m_children; uint32_t m_child_count; }; diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index c5b60124..4b5f87c2 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -62,6 +62,7 @@ int32_t node::create(const char*, size_t, uint32_t, node**) { return ERR_NOSYS; int32_t node::mkdir(const char*, size_t, uint32_t, node**) { return ERR_NOSYS; } int32_t node::unlink(const char*, size_t) { return ERR_NOSYS; } int32_t node::rmdir(const char*, size_t) { return ERR_NOSYS; } +int32_t node::rename(const char*, size_t, node*, const char*, size_t) { return ERR_NOSYS; } ssize_t node::read(file*, void*, size_t) { return ERR_NOSYS; } ssize_t node::write(file*, const void*, size_t) { return ERR_NOSYS; } int64_t node::seek(file*, int64_t, int) { return ERR_NOSYS; } @@ -112,6 +113,19 @@ void node::mark_modified() { m_ctime_ns = m_mtime_ns; } +void node::mark_changed() { + m_ctime_ns = clock::realtime_ns(); +} + +void node::set_name(const char* name, size_t len) { + if (len > NAME_MAX) { + len = NAME_MAX; + } + + string::memcpy(m_name, name, len); + m_name[len] = '\0'; +} + __PRIVILEGED_CODE void node::ref_destroy(node* n) { n->~node(); heap::kfree(n); @@ -919,6 +933,43 @@ int32_t unlink(const char* path) { return err; } +int32_t rename(const char* oldpath, const char* newpath) { + if (!oldpath || !newpath) { + return ERR_INVAL; + } + + if (oldpath[0] != '/' || newpath[0] != '/') { + return ERR_INVAL; + } + + int32_t err; + RUN_ELEVATED({ + node* old_parent = nullptr; + const char* old_name; + size_t old_len; + + err = resolve_parent_at_internal( + nullptr, oldpath, &old_parent, &old_name, &old_len); + if (err == OK) { + node* new_parent = nullptr; + const char* new_name; + size_t new_len; + + err = resolve_parent_at_internal( + nullptr, newpath, &new_parent, &new_name, &new_len); + + if (err == OK) { + err = old_parent->rename(old_name, old_len, new_parent, new_name, new_len); + release_node_ref(new_parent); + } + + release_node_ref(old_parent); + } + }); + + return err; +} + ssize_t readdir(file* f, dirent* entries, size_t count) { if (!f || !entries) return ERR_BADF; @@ -941,6 +992,7 @@ __PRIVILEGED_CODE int32_t init() { for (uint32_t i = 0; i < MAX_DRIVERS; i++) { g_drivers[i] = nullptr; } + for (uint32_t i = 0; i < MAX_MOUNTS; i++) { g_mounts[i] = nullptr; } diff --git a/kernel/fs/fs.h b/kernel/fs/fs.h index b509b71d..45761e41 100644 --- a/kernel/fs/fs.h +++ b/kernel/fs/fs.h @@ -26,6 +26,7 @@ constexpr int32_t ERR_BUSY = -11; constexpr int32_t ERR_LOOP = -12; constexpr int32_t ERR_BADF = -13; constexpr int32_t ERR_AGAIN = -14; +constexpr int32_t ERR_XDEV = -15; /** * @brief Initialize the filesystem subsystem. Registers ramfs, @@ -74,6 +75,7 @@ int32_t fsetattr(file* f, const vattr& attr, uint32_t mask); int32_t mkdir(const char* path, uint32_t mode); int32_t rmdir(const char* path); int32_t unlink(const char* path); +int32_t rename(const char* oldpath, const char* newpath); ssize_t readdir(file* f, dirent* entries, size_t count); /** diff --git a/kernel/fs/node.h b/kernel/fs/node.h index 1d16314a..370132f9 100644 --- a/kernel/fs/node.h +++ b/kernel/fs/node.h @@ -36,6 +36,8 @@ class node : public rc::ref_counted { virtual int32_t mkdir(const char* name, size_t len, uint32_t mode, node** out); virtual int32_t unlink(const char* name, size_t len); virtual int32_t rmdir(const char* name, size_t len); + virtual int32_t rename(const char* name, size_t len, node* new_parent, + const char* new_name, size_t new_len); // --- I/O ops (file/device nodes override) --- virtual ssize_t read(file* f, void* buf, size_t count); @@ -81,6 +83,10 @@ class node : public rc::ref_counted { void set_parent(node* p) { m_parent = p; } void set_filesystem(instance* fs) { m_fs = fs; } void set_mounted_here(instance* inst) { m_mounted_here = inst; } + void set_name(const char* name, size_t len); + + // Records an attribute change by moving ctime to now + void mark_changed(); list::node m_child_link; diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index 7e2c977c..c721b16f 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -185,6 +185,11 @@ int32_t dir_node::rmdir(const char* name, size_t len) { return fs::OK; } +int32_t dir_node::rename(const char* name, size_t len, fs::node* new_parent, + const char* new_name, size_t new_len) { + return rename_child(name, len, new_parent, new_name, new_len); +} + file_node::file_node(fs::instance* fs, const char* name) : fs::node(fs::node_type::regular, fs, name) , m_pages(nullptr) diff --git a/kernel/fs/ramfs/ramfs.h b/kernel/fs/ramfs/ramfs.h index ef8c18fa..dc940057 100644 --- a/kernel/fs/ramfs/ramfs.h +++ b/kernel/fs/ramfs/ramfs.h @@ -22,6 +22,8 @@ class dir_node : public fs::dir_node { int32_t mkdir(const char* name, size_t len, uint32_t mode, fs::node** out) override; int32_t unlink(const char* name, size_t len) override; int32_t rmdir(const char* name, size_t len) override; + int32_t rename(const char* name, size_t len, fs::node* new_parent, + const char* new_name, size_t new_len) override; int32_t create_socket(const char* name, size_t len, void* impl, fs::node** out) override; }; diff --git a/kernel/syscall/handlers/sys_error_map.h b/kernel/syscall/handlers/sys_error_map.h index 9441c126..f53b83e0 100644 --- a/kernel/syscall/handlers/sys_error_map.h +++ b/kernel/syscall/handlers/sys_error_map.h @@ -34,6 +34,8 @@ inline int64_t map_fs_error(int32_t rc) { return syscall::EBADF; case fs::ERR_AGAIN: return syscall::EAGAIN; + case fs::ERR_XDEV: + return syscall::EXDEV; case fs::ERR_IO: default: return syscall::EIO; diff --git a/kernel/syscall/handlers/sys_fd.cpp b/kernel/syscall/handlers/sys_fd.cpp index e6c736e3..7734372a 100644 --- a/kernel/syscall/handlers/sys_fd.cpp +++ b/kernel/syscall/handlers/sys_fd.cpp @@ -1630,12 +1630,87 @@ DEFINE_SYSCALL3(readlink, pathname, buf, bufsize) { return do_readlinkat(static_cast(-100), pathname, buf, bufsize); } +static int64_t copy_user_path(uint64_t u_path, char* out, size_t cap) { + int32_t copy_rc = mm::uaccess::copy_cstr_from_user( + out, cap, reinterpret_cast(u_path)); + if (copy_rc == mm::uaccess::ERR_NAMETOOLONG) { + return syscall::ENAMETOOLONG; + } + + if (copy_rc != mm::uaccess::OK) { + return syscall::EFAULT; + } + + if (out[0] == '\0') { + return syscall::ENOENT; + } + + return 0; +} + DEFINE_SYSCALL4(renameat, olddirfd, oldpath, newdirfd, newpath) { - (void)olddirfd; - (void)oldpath; - (void)newdirfd; - (void)newpath; - return syscall::ENOSYS; + sched::task* task = sched::current(); + if (!task) { + return syscall::EIO; + } + + // Both paths outlive the rename because parent resolution hands back + // name pointers into them + char* paths = static_cast(heap::kzalloc(2 * fs::PATH_MAX)); + if (!paths) { + return syscall::ENOMEM; + } + + char* old_kpath = paths; + char* new_kpath = paths + fs::PATH_MAX; + int64_t rc = copy_user_path(oldpath, old_kpath, fs::PATH_MAX); + if (rc == 0) { + rc = copy_user_path(newpath, new_kpath, fs::PATH_MAX); + } + if (rc != 0) { + heap::kfree(paths); + return rc; + } + + fs::node* old_parent = nullptr; + const char* old_name = nullptr; + size_t old_len = 0; + rc = resolve_parent_for_dirfd_path( + task, static_cast(olddirfd), old_kpath, + &old_parent, &old_name, &old_len); + if (rc != 0) { + heap::kfree(paths); + return rc; + } + + fs::node* new_parent = nullptr; + const char* new_name = nullptr; + size_t new_len = 0; + rc = resolve_parent_for_dirfd_path( + task, static_cast(newdirfd), new_kpath, + &new_parent, &new_name, &new_len); + if (rc != 0) { + release_node_ref(old_parent); + heap::kfree(paths); + return rc; + } + + int32_t fs_rc = old_parent->rename(old_name, old_len, new_parent, new_name, new_len); + release_node_ref(new_parent); + release_node_ref(old_parent); + heap::kfree(paths); + if (fs_rc != fs::OK) { + return syscall::error_map::map_fs_error(fs_rc); + } + + return 0; +} + +DEFINE_SYSCALL2(rename, oldpath, newpath) { + // -100 is AT_FDCWD + return sys_renameat( + static_cast(-100), oldpath, + static_cast(-100), newpath, 0, 0); } // fsync is a no-op on ramfs, data is always in memory diff --git a/kernel/syscall/handlers/sys_fd.h b/kernel/syscall/handlers/sys_fd.h index 93fd18b6..3bb7b4b0 100644 --- a/kernel/syscall/handlers/sys_fd.h +++ b/kernel/syscall/handlers/sys_fd.h @@ -30,6 +30,7 @@ DECLARE_SYSCALL(access); DECLARE_SYSCALL(readlinkat); DECLARE_SYSCALL(readlink); DECLARE_SYSCALL(renameat); +DECLARE_SYSCALL(rename); DECLARE_SYSCALL(fsync); #endif // STELLUX_SYSCALL_HANDLERS_SYS_FD_H diff --git a/kernel/syscall/syscall_table.cpp b/kernel/syscall/syscall_table.cpp index 6f525f6f..841693f6 100644 --- a/kernel/syscall/syscall_table.cpp +++ b/kernel/syscall/syscall_table.cpp @@ -131,7 +131,7 @@ __PRIVILEGED_CODE void init_syscall_table() { REGISTER_SYSCALL(linux_nr::UNLINK, unlink); REGISTER_SYSCALL(linux_nr::RMDIR, rmdir); REGISTER_SYSCALL(linux_nr::ACCESS, access); - REGISTER_SYSCALL(linux_nr::RENAME, renameat); + REGISTER_SYSCALL(linux_nr::RENAME, rename); REGISTER_SYSCALL(linux_nr::READLINK, readlink); #endif diff --git a/kernel/syscall/syscall_table.h b/kernel/syscall/syscall_table.h index 91a04049..e3b3d166 100644 --- a/kernel/syscall/syscall_table.h +++ b/kernel/syscall/syscall_table.h @@ -31,6 +31,7 @@ constexpr int64_t ENOTEMPTY = -39; constexpr int64_t ELOOP = -40; constexpr int64_t EAGAIN = -11; constexpr int64_t EBUSY = -16; +constexpr int64_t EXDEV = -18; constexpr int64_t ESPIPE = -29; constexpr int64_t EPIPE = -32; constexpr int64_t ENOTSOCK = -88; diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index 4ed7275d..de7f5600 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -500,6 +500,137 @@ TEST(fs_test, open_trunc_ignored_for_read_only) { fs::unlink("/trunc_ro"); } +TEST(fs_test, rename_in_place_keeps_identity) { + fs::file* f = fs::open("/rn_a", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + fs::vattr before = {}; + EXPECT_EQ(fs::stat("/rn_a", &before), fs::OK); + EXPECT_EQ(fs::rename("/rn_a", "/rn_b"), fs::OK); + + fs::vattr after = {}; + EXPECT_EQ(fs::stat("/rn_b", &after), fs::OK); + EXPECT_EQ(after.ino, before.ino); + EXPECT_GE(after.ctime_ns, before.ctime_ns); + EXPECT_EQ(fs::stat("/rn_a", &before), fs::ERR_NOENT); + + fs::unlink("/rn_b"); +} + +TEST(fs_test, rename_across_directories_moves_content) { + EXPECT_EQ(fs::mkdir("/rn_src", 0), fs::OK); + EXPECT_EQ(fs::mkdir("/rn_dst", 0), fs::OK); + fs::file* f = fs::open("/rn_src/f", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "moved", 5), static_cast(5)); + fs::close(f); + + EXPECT_EQ(fs::rename("/rn_src/f", "/rn_dst/g"), fs::OK); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/rn_src", &attr), fs::OK); + EXPECT_EQ(attr.size, static_cast(0)); + EXPECT_EQ(fs::stat("/rn_dst", &attr), fs::OK); + EXPECT_EQ(attr.size, static_cast(1)); + + f = fs::open("/rn_dst/g", fs::O_RDONLY); + ASSERT_NOT_NULL(f); + char buf[8] = {}; + EXPECT_EQ(fs::read(f, buf, sizeof(buf)), static_cast(5)); + EXPECT_STREQ(buf, "moved"); + fs::close(f); + + fs::unlink("/rn_dst/g"); + fs::rmdir("/rn_src"); + fs::rmdir("/rn_dst"); +} + +TEST(fs_test, rename_replaces_existing_file) { + fs::file* f = fs::open("/rn_new", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "new", 3), static_cast(3)); + fs::close(f); + f = fs::open("/rn_old", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "old", 3), static_cast(3)); + fs::close(f); + + EXPECT_EQ(fs::rename("/rn_new", "/rn_old"), fs::OK); + + f = fs::open("/rn_old", fs::O_RDONLY); + ASSERT_NOT_NULL(f); + char buf[8] = {}; + EXPECT_EQ(fs::read(f, buf, sizeof(buf)), static_cast(3)); + EXPECT_STREQ(buf, "new"); + fs::close(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/rn_new", &attr), fs::ERR_NOENT); + + fs::unlink("/rn_old"); +} + +TEST(fs_test, rename_onto_itself_is_noop) { + fs::file* f = fs::open("/rn_self", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + EXPECT_EQ(fs::rename("/rn_self", "/rn_self"), fs::OK); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/rn_self", &attr), fs::OK); + + fs::unlink("/rn_self"); +} + +TEST(fs_test, rename_refuses_directory_into_own_subtree) { + EXPECT_EQ(fs::mkdir("/rn_outer", 0), fs::OK); + EXPECT_EQ(fs::mkdir("/rn_outer/inner", 0), fs::OK); + + EXPECT_EQ(fs::rename("/rn_outer", "/rn_outer/inner/x"), fs::ERR_INVAL); + EXPECT_EQ(fs::rename("/rn_outer", "/rn_outer/x"), fs::ERR_INVAL); + + fs::rmdir("/rn_outer/inner"); + fs::rmdir("/rn_outer"); +} + +TEST(fs_test, rename_directory_over_nonempty_fails) { + EXPECT_EQ(fs::mkdir("/rn_d1", 0), fs::OK); + EXPECT_EQ(fs::mkdir("/rn_d2", 0), fs::OK); + EXPECT_EQ(fs::mkdir("/rn_d2/x", 0), fs::OK); + + EXPECT_EQ(fs::rename("/rn_d1", "/rn_d2"), fs::ERR_NOTEMPTY); + + fs::rmdir("/rn_d2/x"); + EXPECT_EQ(fs::rename("/rn_d1", "/rn_d2"), fs::OK); + + fs::rmdir("/rn_d2"); +} + +TEST(fs_test, rename_type_mismatch_fails) { + fs::file* f = fs::open("/rn_file", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + EXPECT_EQ(fs::mkdir("/rn_dir", 0), fs::OK); + + EXPECT_EQ(fs::rename("/rn_file", "/rn_dir"), fs::ERR_ISDIR); + EXPECT_EQ(fs::rename("/rn_dir", "/rn_file"), fs::ERR_NOTDIR); + + fs::unlink("/rn_file"); + fs::rmdir("/rn_dir"); +} + +TEST(fs_test, rename_across_filesystems_fails) { + fs::file* f = fs::open("/rn_xdev", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + EXPECT_EQ(fs::rename("/rn_xdev", "/dev/rn_xdev"), fs::ERR_XDEV); + + fs::unlink("/rn_xdev"); +} + TEST(fs_test, multi_page_write_read) { fs::file* f = fs::open("/bigfile", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); diff --git a/kernel/tests/resource/resource.test.cpp b/kernel/tests/resource/resource.test.cpp index f7f00ce5..40d6ad9e 100644 --- a/kernel/tests/resource/resource.test.cpp +++ b/kernel/tests/resource/resource.test.cpp @@ -420,6 +420,11 @@ TEST(resource_test, faccessat_validates_mode_before_path) { EXPECT_EQ(sys_faccessat(at_fdcwd, kpath, 0, 0, 0, 0), syscall::EFAULT); } -TEST(resource_test, renameat_returns_enosys) { - EXPECT_EQ(sys_renameat(0, 0, 0, 0, 0, 0), syscall::ENOSYS); +TEST(resource_test, renameat_rejects_bad_user_paths) { + constexpr uint64_t at_fdcwd = static_cast(-100); + uint64_t kpath = reinterpret_cast("/a"); + + EXPECT_EQ(sys_renameat(at_fdcwd, 0, at_fdcwd, kpath, 0, 0), syscall::EFAULT); + EXPECT_EQ(sys_renameat(at_fdcwd, kpath, at_fdcwd, kpath, 0, 0), syscall::EFAULT); + EXPECT_EQ(sys_rename(kpath, kpath, 0, 0, 0, 0), syscall::EFAULT); } From b75fdd5545417903140be1b883f4559a9cb4963f Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 20:31:31 -0700 Subject: [PATCH 07/13] fix(fs): honored O_EXCL on open --- kernel/fs/fs.cpp | 15 +++++++++++---- kernel/resource/providers/file_provider.cpp | 2 ++ kernel/tests/fs/fs.test.cpp | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index 4b5f87c2..36709e72 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -687,13 +687,20 @@ file* open_at(node* base_dir, const char* path, uint32_t flags, int32_t* out_err err = resolve_parent_at_internal( base_dir, path, &parent, &name, &name_len); if (err == OK) { + // O_EXCL demands that this call be the one creating the file, so + // an existing entry and a concurrent create both fail err = parent->lookup(name, name_len, &n); - if (err == ERR_NOENT) { + if (err == OK && (flags & O_EXCL)) { + release_node_ref(n); + n = nullptr; + err = ERR_EXIST; + } else if (err == ERR_NOENT) { err = parent->create(name, name_len, 0, &n); + if (err == ERR_EXIST && !(flags & O_EXCL)) { + err = parent->lookup(name, name_len, &n); + } } - if (err == ERR_EXIST) { - err = parent->lookup(name, name_len, &n); - } + if (parent->release()) { node::ref_destroy(parent); } diff --git a/kernel/resource/providers/file_provider.cpp b/kernel/resource/providers/file_provider.cpp index d7e85194..df26e3bf 100644 --- a/kernel/resource/providers/file_provider.cpp +++ b/kernel/resource/providers/file_provider.cpp @@ -15,6 +15,8 @@ static int32_t map_fs_error_to_resource(int32_t fs_err) { switch (fs_err) { case fs::ERR_NOENT: return ERR_NOENT; + case fs::ERR_EXIST: + return ERR_EXIST; case fs::ERR_NOMEM: return ERR_NOMEM; case fs::ERR_NOTDIR: diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index de7f5600..a95dffba 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -631,6 +631,25 @@ TEST(fs_test, rename_across_filesystems_fails) { fs::unlink("/rn_xdev"); } +TEST(fs_test, open_excl_creates_only_new_files) { + int32_t err = fs::OK; + fs::file* f = fs::open("/excl_new", fs::O_CREAT | fs::O_EXCL | fs::O_RDWR, &err); + ASSERT_NOT_NULL(f); + EXPECT_EQ(err, fs::OK); + fs::close(f); + + f = fs::open("/excl_new", fs::O_CREAT | fs::O_EXCL | fs::O_RDWR, &err); + EXPECT_NULL(f); + EXPECT_EQ(err, fs::ERR_EXIST); + + f = fs::open("/excl_new", fs::O_CREAT | fs::O_RDWR, &err); + EXPECT_NOT_NULL(f); + EXPECT_EQ(err, fs::OK); + fs::close(f); + + fs::unlink("/excl_new"); +} + TEST(fs_test, multi_page_write_read) { fs::file* f = fs::open("/bigfile", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); From 92b823b7b9ec1ed3d344acb6983a8b4a5ce4b4b9 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 21:04:36 -0700 Subject: [PATCH 08/13] feat(fs): added symbolic links --- kernel/arch/aarch64/syscall/linux_syscalls.h | 1 + kernel/arch/x86_64/syscall/linux_syscalls.h | 2 + kernel/fs/cpio/cpio.cpp | 32 +++- kernel/fs/fs.cpp | 159 +++++++++++++++-- kernel/fs/fs.h | 14 +- kernel/fs/node.h | 1 + kernel/fs/path.h | 2 + kernel/fs/ramfs/ramfs.cpp | 80 +++++++++ kernel/fs/ramfs/ramfs.h | 14 ++ kernel/resource/providers/file_provider.cpp | 2 + kernel/resource/resource.h | 1 + kernel/syscall/handlers/sys_fd.cpp | 72 +++++++- kernel/syscall/handlers/sys_fd.h | 2 + kernel/syscall/syscall_table.cpp | 2 + kernel/tests/fs/fs.test.cpp | 177 +++++++++++++++++++ 15 files changed, 534 insertions(+), 27 deletions(-) diff --git a/kernel/arch/aarch64/syscall/linux_syscalls.h b/kernel/arch/aarch64/syscall/linux_syscalls.h index 41cc3df6..954a405b 100644 --- a/kernel/arch/aarch64/syscall/linux_syscalls.h +++ b/kernel/arch/aarch64/syscall/linux_syscalls.h @@ -12,6 +12,7 @@ constexpr uint64_t FCNTL = 25; constexpr uint64_t IOCTL = 29; constexpr uint64_t MKDIRAT = 34; constexpr uint64_t UNLINKAT = 35; +constexpr uint64_t SYMLINKAT = 36; constexpr uint64_t RENAMEAT = 38; constexpr uint64_t FTRUNCATE = 46; constexpr uint64_t FACCESSAT = 48; diff --git a/kernel/arch/x86_64/syscall/linux_syscalls.h b/kernel/arch/x86_64/syscall/linux_syscalls.h index 2febb9fc..c542c6ca 100644 --- a/kernel/arch/x86_64/syscall/linux_syscalls.h +++ b/kernel/arch/x86_64/syscall/linux_syscalls.h @@ -60,6 +60,7 @@ constexpr uint64_t RENAME = 82; constexpr uint64_t MKDIR = 83; constexpr uint64_t RMDIR = 84; constexpr uint64_t UNLINK = 87; +constexpr uint64_t SYMLINK = 88; constexpr uint64_t READLINK = 89; constexpr uint64_t CHMOD = 90; constexpr uint64_t UMASK = 95; @@ -89,6 +90,7 @@ constexpr uint64_t MKDIRAT = 258; constexpr uint64_t NEWFSTATAT = 262; constexpr uint64_t UNLINKAT = 263; constexpr uint64_t RENAMEAT = 264; +constexpr uint64_t SYMLINKAT = 266; constexpr uint64_t READLINKAT = 267; constexpr uint64_t FACCESSAT = 269; constexpr uint64_t PSELECT6 = 270; diff --git a/kernel/fs/cpio/cpio.cpp b/kernel/fs/cpio/cpio.cpp index a4d37dd0..d4e73745 100644 --- a/kernel/fs/cpio/cpio.cpp +++ b/kernel/fs/cpio/cpio.cpp @@ -4,6 +4,7 @@ #include "boot/boot_services.h" #include "mm/vmm.h" #include "mm/paging.h" +#include "mm/heap.h" #include "common/string.h" #include "common/logging.h" @@ -14,6 +15,7 @@ constexpr char CPIO_TRAILER[] = "TRAILER!!!"; constexpr uint32_t S_IFMT = 0170000; constexpr uint32_t S_IFDIR = 0040000; constexpr uint32_t S_IFREG = 0100000; +constexpr uint32_t S_IFLNK = 0120000; constexpr uint64_t NS_PER_SEC = 1000000000ULL; static uint32_t hex_to_u32(const char* s, size_t len) { @@ -87,8 +89,16 @@ __PRIVILEGED_CODE int32_t load_initrd() { const auto* archive = reinterpret_cast(usable_va); size_t archive_len = static_cast(size); + // Link targets are copied out so they can be null terminated + char* target_buf = static_cast(heap::uzalloc(fs::PATH_MAX)); + if (!target_buf) { + vmm::free(base_va); + return ERR_MAP_FAILED; + } + uint32_t files_extracted = 0; uint32_t dirs_created = 0; + uint32_t links_created = 0; size_t offset = 0; while (offset + sizeof(cpio_newc_header) <= archive_len) { @@ -173,16 +183,34 @@ __PRIVILEGED_CODE int32_t load_initrd() { } else { log::warn("cpio: failed to create file %s", path_buf); } + } else if ((mode & S_IFMT) == S_IFLNK) { + bool target_ok = filesize > 0 && filesize < fs::PATH_MAX && + offset + filesize <= archive_len; + + if (target_ok) { + string::memcpy(target_buf, file_data, filesize); + target_buf[filesize] = '\0'; + ensure_parents(path_buf); + + if (fs::symlink(target_buf, path_buf) == fs::OK) { + links_created++; + } else { + log::warn("cpio: failed to create link %s", path_buf); + } + } else { + log::warn("cpio: rejecting link with bad target: %s", path_buf); + } } offset += filesize; offset = align4(offset); } + heap::ufree(target_buf); vmm::free(base_va); - log::info("cpio: extracted %u files and %u directories into /", - files_extracted, dirs_created); + log::info("cpio: extracted %u files, %u directories, and %u links into /", + files_extracted, dirs_created, links_created); return OK; } diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index 36709e72..058450dd 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -63,6 +63,7 @@ int32_t node::mkdir(const char*, size_t, uint32_t, node**) { return ERR_NOSYS; int32_t node::unlink(const char*, size_t) { return ERR_NOSYS; } int32_t node::rmdir(const char*, size_t) { return ERR_NOSYS; } int32_t node::rename(const char*, size_t, node*, const char*, size_t) { return ERR_NOSYS; } +int32_t node::symlink(const char*, size_t, const char*, node**) { return ERR_NOSYS; } ssize_t node::read(file*, void*, size_t) { return ERR_NOSYS; } ssize_t node::write(file*, const void*, size_t) { return ERR_NOSYS; } int64_t node::seek(file*, int64_t, int) { return ERR_NOSYS; } @@ -255,8 +256,53 @@ __PRIVILEGED_CODE static int32_t acquire_start_node( return OK; } +// Builds a link target followed by the unresolved remainder of the path into +// the spare buffer, then makes that buffer the one being walked. Both buffers +// are allocated on first use so lookups without links pay nothing. +__PRIVILEGED_CODE static int32_t splice_symlink( + node* link, const char* rest, char** walked, char** spare, uint32_t* depth +) { + if (*depth >= SYMLOOP_MAX) { + return ERR_LOOP; + } + + (*depth)++; + + if (!*walked) { + *walked = static_cast(heap::uzalloc(PATH_MAX)); + *spare = static_cast(heap::uzalloc(PATH_MAX)); + if (!*walked || !*spare) { + return ERR_NOMEM; + } + } + + char* buf = *spare; + size_t len = 0; + int32_t err = link->readlink(buf, PATH_MAX - 1, &len); + if (err != OK) { + return err; + } + + size_t rest_len = string::strnlen(rest, PATH_MAX); + if (rest_len > 0) { + if (len + 1 + rest_len >= PATH_MAX) { + return ERR_NAMETOOLONG; + } + + buf[len] = '/'; + string::memcpy(buf + len + 1, rest, rest_len); + len += 1 + rest_len; + } + buf[len] = '\0'; + + *spare = *walked; + *walked = buf; + + return OK; +} + __PRIVILEGED_CODE static int32_t resolve_path_at_internal( - node* base_dir, const char* path, node** out + node* base_dir, const char* path, bool follow_last, node** out ) { if (!path || !out) { return ERR_INVAL; @@ -267,19 +313,25 @@ __PRIVILEGED_CODE static int32_t resolve_path_at_internal( } node* cur = nullptr; - int32_t start_err = acquire_start_node(base_dir, path, &cur); - if (start_err != OK) { - return start_err; + int32_t err = acquire_start_node(base_dir, path, &cur); + if (err != OK) { + return err; } + // Following a link restarts the walk on a spliced heap copy of the path, + // so a chain of links costs no stack depth however long it is + char* walked = nullptr; + char* spare = nullptr; + uint32_t depth = 0; + path_iterator it(path); const char* comp = nullptr; size_t comp_len = 0; while (it.next(comp, comp_len)) { if (comp_len > NAME_MAX) { - release_node_ref(cur); - return ERR_NAMETOOLONG; + err = ERR_NAMETOOLONG; + break; } if (comp_len == 2 && comp[0] == '.' && comp[1] == '.') { @@ -305,15 +357,41 @@ __PRIVILEGED_CODE static int32_t resolve_path_at_internal( } if (cur->type() != node_type::directory) { - release_node_ref(cur); - return ERR_NOTDIR; + err = ERR_NOTDIR; + break; } node* child = nullptr; - int32_t err = cur->lookup(comp, comp_len, &child); + err = cur->lookup(comp, comp_len, &child); if (err != OK) { - release_node_ref(cur); - return err; + break; + } + + // A link in the middle of a path is always followed, a final one only + // when the caller wants the target rather than the link itself + path_iterator ahead = it; + const char* ahead_comp = nullptr; + size_t ahead_len = 0; + bool is_last = !ahead.next(ahead_comp, ahead_len); + + if (child->type() == node_type::symlink && (follow_last || !is_last)) { + err = splice_symlink(child, it.remaining(), &walked, &spare, &depth); + release_node_ref(child); + if (err != OK) { + break; + } + + if (walked[0] == '/') { + release_node_ref(cur); + cur = nullptr; + err = acquire_global_root(&cur); + if (err != OK) { + break; + } + } + + it = path_iterator(walked); + continue; } while (child->mounted_here()) { @@ -327,6 +405,19 @@ __PRIVILEGED_CODE static int32_t resolve_path_at_internal( cur = child; } + if (walked) { + heap::ufree(walked); + } + + if (spare) { + heap::ufree(spare); + } + + if (err != OK) { + release_node_ref(cur); + return err; + } + *out = cur; return OK; } @@ -399,7 +490,7 @@ __PRIVILEGED_CODE static int32_t resolve_parent_at_internal( parent_buf[parent_len] = '\0'; } - err = resolve_path_at_internal(base_dir, parent_buf, out_parent); + err = resolve_path_at_internal(base_dir, parent_buf, true, out_parent); if (err != OK) { return err; } @@ -521,15 +612,21 @@ __PRIVILEGED_CODE int32_t lookup(const char* path, node** out) { return ERR_INVAL; } - return resolve_path_at_internal(nullptr, path, out); + return resolve_path_at_internal(nullptr, path, true, out); } __PRIVILEGED_CODE int32_t lookup_at(node* base_dir, const char* path, node** out) { + return lookup_at(base_dir, path, 0, out); +} + +__PRIVILEGED_CODE int32_t lookup_at(node* base_dir, const char* path, uint32_t flags, node** out) { if (!path || !out) { return ERR_INVAL; } - return resolve_path_at_internal(base_dir, path, out); + bool follow_last = (flags & LOOKUP_NOFOLLOW) == 0; + + return resolve_path_at_internal(base_dir, path, follow_last, out); } __PRIVILEGED_CODE int32_t resolve_parent_path( @@ -694,6 +791,10 @@ file* open_at(node* base_dir, const char* path, uint32_t flags, int32_t* out_err release_node_ref(n); n = nullptr; err = ERR_EXIST; + } else if (err == OK && n->type() == node_type::symlink) { + release_node_ref(n); + n = nullptr; + err = resolve_path_at_internal(base_dir, path, true, &n); } else if (err == ERR_NOENT) { err = parent->create(name, name_len, 0, &n); if (err == ERR_EXIST && !(flags & O_EXCL)) { @@ -706,7 +807,7 @@ file* open_at(node* base_dir, const char* path, uint32_t flags, int32_t* out_err } } } else { - err = resolve_path_at_internal(base_dir, path, &n); + err = resolve_path_at_internal(base_dir, path, true, &n); } }); @@ -977,6 +1078,34 @@ int32_t rename(const char* oldpath, const char* newpath) { return err; } +int32_t symlink(const char* target, const char* linkpath) { + if (!target || !linkpath) { + return ERR_INVAL; + } + + if (target[0] == '\0' || linkpath[0] != '/') { + return ERR_INVAL; + } + + int32_t err; + RUN_ELEVATED({ + node* parent = nullptr; + const char* name; + size_t name_len; + + err = resolve_parent_at_internal( + nullptr, linkpath, &parent, &name, &name_len); + if (err == OK) { + node* link = nullptr; + err = parent->symlink(name, name_len, target, &link); + release_node_ref(link); + release_node_ref(parent); + } + }); + + return err; +} + ssize_t readdir(file* f, dirent* entries, size_t count) { if (!f || !entries) return ERR_BADF; diff --git a/kernel/fs/fs.h b/kernel/fs/fs.h index 45761e41..27e6c1b1 100644 --- a/kernel/fs/fs.h +++ b/kernel/fs/fs.h @@ -76,16 +76,28 @@ int32_t mkdir(const char* path, uint32_t mode); int32_t rmdir(const char* path); int32_t unlink(const char* path); int32_t rename(const char* oldpath, const char* newpath); +int32_t symlink(const char* target, const char* linkpath); + +// Lookup flag: resolve the final component to the link itself, not its target +constexpr uint32_t LOOKUP_NOFOLLOW = 1u << 0; ssize_t readdir(file* f, dirent* entries, size_t count); /** * @brief Resolve path relative to base_dir when path is not absolute. - * If path is absolute, base_dir is ignored. + * If path is absolute, base_dir is ignored. Symbolic links are followed. * On success, *out has add_ref() called, caller must release. * @note Privilege: **required** */ __PRIVILEGED_CODE int32_t lookup_at(node* base_dir, const char* path, node** out); +/** + * @brief Resolve path like lookup_at, with LOOKUP_NOFOLLOW in flags leaving + * a final symbolic link unresolved so the caller receives the link itself. + * On success, *out has add_ref() called, caller must release. + * @note Privilege: **required** + */ +__PRIVILEGED_CODE int32_t lookup_at(node* base_dir, const char* path, uint32_t flags, node** out); + /** * @brief Resolve parent directory of path relative to base_dir. * If path is absolute, base_dir is ignored. diff --git a/kernel/fs/node.h b/kernel/fs/node.h index 370132f9..9c0c54e1 100644 --- a/kernel/fs/node.h +++ b/kernel/fs/node.h @@ -38,6 +38,7 @@ class node : public rc::ref_counted { virtual int32_t rmdir(const char* name, size_t len); virtual int32_t rename(const char* name, size_t len, node* new_parent, const char* new_name, size_t new_len); + virtual int32_t symlink(const char* name, size_t len, const char* target, node** out); // --- I/O ops (file/device nodes override) --- virtual ssize_t read(file* f, void* buf, size_t count); diff --git a/kernel/fs/path.h b/kernel/fs/path.h index a9172346..0a5e411b 100644 --- a/kernel/fs/path.h +++ b/kernel/fs/path.h @@ -30,6 +30,8 @@ class path_iterator { */ bool next(const char*& out_name, size_t& out_len); + const char* remaining() const { return m_path + m_pos; } + private: const char* m_path; size_t m_pos; diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index c721b16f..946cc0e7 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -190,6 +190,86 @@ int32_t dir_node::rename(const char* name, size_t len, fs::node* new_parent, return rename_child(name, len, new_parent, new_name, new_len); } +int32_t dir_node::symlink(const char* name, size_t len, const char* target, fs::node** out) { + if (!name || !out || !target || len == 0) { + return fs::ERR_INVAL; + } + + if (len > fs::NAME_MAX) { + return fs::ERR_NAMETOOLONG; + } + + sync::irq_lock_guard guard(m_lock); + + if (find_child(name, len)) { + return fs::ERR_EXIST; + } + + char name_buf[fs::NAME_MAX + 1]; + string::memcpy(name_buf, name, len); + name_buf[len] = '\0'; + + void* mem = heap::kzalloc(sizeof(symlink_node)); + if (!mem) { + return fs::ERR_NOMEM; + } + + auto* child = new (mem) symlink_node(m_fs, name_buf); + int32_t rc = child->set_target(target); + if (rc != fs::OK) { + fs::node::ref_destroy(child); + return rc; + } + + attach_child(child); + + *out = child; + return fs::OK; +} + +symlink_node::symlink_node(fs::instance* fs, const char* name) + : fs::node(fs::node_type::symlink, fs, name) + , m_target(nullptr) + , m_target_len(0) { +} + +symlink_node::~symlink_node() { + if (m_target) { + heap::kfree(m_target); + } +} + +int32_t symlink_node::set_target(const char* target) { + size_t len = string::strnlen(target, fs::PATH_MAX); + if (len == 0 || len >= fs::PATH_MAX) { + return fs::ERR_INVAL; + } + + auto* copy = static_cast(heap::kzalloc(len + 1)); + if (!copy) { + return fs::ERR_NOMEM; + } + + string::memcpy(copy, target, len); + m_target = copy; + m_target_len = len; + m_size = len; + + return fs::OK; +} + +int32_t symlink_node::readlink(char* buf, size_t size, size_t* out_len) { + if (!buf || !out_len) { + return fs::ERR_INVAL; + } + + size_t n = m_target_len < size ? m_target_len : size; + string::memcpy(buf, m_target, n); + *out_len = n; + + return fs::OK; +} + file_node::file_node(fs::instance* fs, const char* name) : fs::node(fs::node_type::regular, fs, name) , m_pages(nullptr) diff --git a/kernel/fs/ramfs/ramfs.h b/kernel/fs/ramfs/ramfs.h index dc940057..85f18161 100644 --- a/kernel/fs/ramfs/ramfs.h +++ b/kernel/fs/ramfs/ramfs.h @@ -24,9 +24,23 @@ class dir_node : public fs::dir_node { int32_t rmdir(const char* name, size_t len) override; int32_t rename(const char* name, size_t len, fs::node* new_parent, const char* new_name, size_t new_len) override; + int32_t symlink(const char* name, size_t len, const char* target, fs::node** out) override; int32_t create_socket(const char* name, size_t len, void* impl, fs::node** out) override; }; +class symlink_node : public fs::node { +public: + symlink_node(fs::instance* fs, const char* name); + ~symlink_node() override; + + int32_t set_target(const char* target); + int32_t readlink(char* buf, size_t size, size_t* out_len) override; + +private: + char* m_target; + size_t m_target_len; +}; + class file_node : public fs::node { public: file_node(fs::instance* fs, const char* name); diff --git a/kernel/resource/providers/file_provider.cpp b/kernel/resource/providers/file_provider.cpp index df26e3bf..534dfa81 100644 --- a/kernel/resource/providers/file_provider.cpp +++ b/kernel/resource/providers/file_provider.cpp @@ -31,6 +31,8 @@ static int32_t map_fs_error_to_resource(int32_t fs_err) { return ERR_UNSUP; case fs::ERR_AGAIN: return ERR_AGAIN; + case fs::ERR_LOOP: + return ERR_LOOP; default: return ERR_IO; } diff --git a/kernel/resource/resource.h b/kernel/resource/resource.h index 8f63af16..565598af 100644 --- a/kernel/resource/resource.h +++ b/kernel/resource/resource.h @@ -86,6 +86,7 @@ constexpr int32_t ERR_AGAIN = -16; constexpr int32_t ERR_EXIST = -17; constexpr int32_t ERR_INTR = -18; constexpr int32_t ERR_NOPROTOOPT = -19; +constexpr int32_t ERR_LOOP = -20; /** * @brief Allocate a private handle table and attach it to the task. diff --git a/kernel/syscall/handlers/sys_fd.cpp b/kernel/syscall/handlers/sys_fd.cpp index 7734372a..2ecff9f1 100644 --- a/kernel/syscall/handlers/sys_fd.cpp +++ b/kernel/syscall/handlers/sys_fd.cpp @@ -138,6 +138,8 @@ static inline int64_t map_resource_error(int64_t rc) { return syscall::EAGAIN; case resource::ERR_EXIST: return syscall::EEXIST; + case resource::ERR_LOOP: + return syscall::ELOOP; case resource::ERR_IO: default: return syscall::EIO; @@ -466,7 +468,8 @@ static int64_t lookup_node_for_dirfd_path( sched::task* task, int64_t dirfd, const char* input_path, - fs::node** out_node + fs::node** out_node, + uint32_t lookup_flags = 0 ) { if (!task || !input_path || !out_node) { return syscall::EINVAL; @@ -480,7 +483,7 @@ static int64_t lookup_node_for_dirfd_path( } } - int32_t fs_rc = fs::lookup_at(base, input_path, out_node); + int32_t fs_rc = fs::lookup_at(base, input_path, lookup_flags, out_node); release_node_ref(base); if (fs_rc != fs::OK) { return syscall::error_map::map_fs_error(fs_rc); @@ -698,7 +701,8 @@ static int64_t do_newfstatat_common(int64_t dirfd, uint64_t pathname, uint64_t u } fs::node* target = nullptr; - int64_t lookup_rc = lookup_node_for_dirfd_path(task, dirfd, kpath, &target); + uint32_t lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? fs::LOOKUP_NOFOLLOW : 0; + int64_t lookup_rc = lookup_node_for_dirfd_path(task, dirfd, kpath, &target, lookup_flags); if (lookup_rc != 0) { return lookup_rc; } @@ -1589,7 +1593,8 @@ static int64_t do_readlinkat(int64_t dirfd, uint64_t pathname, } fs::node* node = nullptr; - int64_t lookup_rc = lookup_node_for_dirfd_path(task, dirfd, kpath, &node); + int64_t lookup_rc = lookup_node_for_dirfd_path( + task, dirfd, kpath, &node, fs::LOOKUP_NOFOLLOW); if (lookup_rc != 0) { return lookup_rc; } @@ -1656,7 +1661,7 @@ DEFINE_SYSCALL4(renameat, olddirfd, oldpath, newdirfd, newpath) { // Both paths outlive the rename because parent resolution hands back // name pointers into them - char* paths = static_cast(heap::kzalloc(2 * fs::PATH_MAX)); + char* paths = static_cast(heap::uzalloc(2 * fs::PATH_MAX)); if (!paths) { return syscall::ENOMEM; } @@ -1668,7 +1673,7 @@ DEFINE_SYSCALL4(renameat, olddirfd, oldpath, newdirfd, newpath) { rc = copy_user_path(newpath, new_kpath, fs::PATH_MAX); } if (rc != 0) { - heap::kfree(paths); + heap::ufree(paths); return rc; } @@ -1679,7 +1684,7 @@ DEFINE_SYSCALL4(renameat, olddirfd, oldpath, newdirfd, newpath) { task, static_cast(olddirfd), old_kpath, &old_parent, &old_name, &old_len); if (rc != 0) { - heap::kfree(paths); + heap::ufree(paths); return rc; } @@ -1691,14 +1696,14 @@ DEFINE_SYSCALL4(renameat, olddirfd, oldpath, newdirfd, newpath) { &new_parent, &new_name, &new_len); if (rc != 0) { release_node_ref(old_parent); - heap::kfree(paths); + heap::ufree(paths); return rc; } int32_t fs_rc = old_parent->rename(old_name, old_len, new_parent, new_name, new_len); release_node_ref(new_parent); release_node_ref(old_parent); - heap::kfree(paths); + heap::ufree(paths); if (fs_rc != fs::OK) { return syscall::error_map::map_fs_error(fs_rc); } @@ -1713,6 +1718,55 @@ DEFINE_SYSCALL2(rename, oldpath, newpath) { static_cast(-100), newpath, 0, 0); } +DEFINE_SYSCALL3(symlinkat, target, newdirfd, linkpath) { + sched::task* task = sched::current(); + if (!task) { + return syscall::EIO; + } + + char* paths = static_cast(heap::uzalloc(2 * fs::PATH_MAX)); + if (!paths) { + return syscall::ENOMEM; + } + + char* ktarget = paths; + char* klink = paths + fs::PATH_MAX; + int64_t rc = copy_user_path(target, ktarget, fs::PATH_MAX); + if (rc == 0) { + rc = copy_user_path(linkpath, klink, fs::PATH_MAX); + } + if (rc != 0) { + heap::ufree(paths); + return rc; + } + + fs::node* parent = nullptr; + const char* name = nullptr; + size_t name_len = 0; + rc = resolve_parent_for_dirfd_path( + task, static_cast(newdirfd), klink, &parent, &name, &name_len); + if (rc != 0) { + heap::ufree(paths); + return rc; + } + + fs::node* link = nullptr; + int32_t fs_rc = parent->symlink(name, name_len, ktarget, &link); + release_node_ref(link); + release_node_ref(parent); + heap::ufree(paths); + if (fs_rc != fs::OK) { + return syscall::error_map::map_fs_error(fs_rc); + } + + return 0; +} + +DEFINE_SYSCALL2(symlink, target, linkpath) { + // -100 is AT_FDCWD + return sys_symlinkat(target, static_cast(-100), linkpath, 0, 0, 0); +} + // fsync is a no-op on ramfs, data is always in memory DEFINE_SYSCALL1(fsync, fd) { (void)fd; diff --git a/kernel/syscall/handlers/sys_fd.h b/kernel/syscall/handlers/sys_fd.h index 3bb7b4b0..88371f07 100644 --- a/kernel/syscall/handlers/sys_fd.h +++ b/kernel/syscall/handlers/sys_fd.h @@ -31,6 +31,8 @@ DECLARE_SYSCALL(readlinkat); DECLARE_SYSCALL(readlink); DECLARE_SYSCALL(renameat); DECLARE_SYSCALL(rename); +DECLARE_SYSCALL(symlinkat); +DECLARE_SYSCALL(symlink); DECLARE_SYSCALL(fsync); #endif // STELLUX_SYSCALL_HANDLERS_SYS_FD_H diff --git a/kernel/syscall/syscall_table.cpp b/kernel/syscall/syscall_table.cpp index 841693f6..19b5e6f4 100644 --- a/kernel/syscall/syscall_table.cpp +++ b/kernel/syscall/syscall_table.cpp @@ -124,6 +124,7 @@ __PRIVILEGED_CODE void init_syscall_table() { REGISTER_SYSCALL(linux_nr::FACCESSAT, faccessat); REGISTER_SYSCALL(linux_nr::FCHMODAT, fchmodat); REGISTER_SYSCALL(linux_nr::RENAMEAT, renameat); + REGISTER_SYSCALL(linux_nr::SYMLINKAT, symlinkat); REGISTER_SYSCALL(linux_nr::READLINKAT, readlinkat); #if defined(__x86_64__) REGISTER_SYSCALL(linux_nr::MKDIR, mkdir); @@ -132,6 +133,7 @@ __PRIVILEGED_CODE void init_syscall_table() { REGISTER_SYSCALL(linux_nr::RMDIR, rmdir); REGISTER_SYSCALL(linux_nr::ACCESS, access); REGISTER_SYSCALL(linux_nr::RENAME, rename); + REGISTER_SYSCALL(linux_nr::SYMLINK, symlink); REGISTER_SYSCALL(linux_nr::READLINK, readlink); #endif diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index a95dffba..8fa392b2 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -650,6 +650,183 @@ TEST(fs_test, open_excl_creates_only_new_files) { fs::unlink("/excl_new"); } +TEST(fs_test, symlink_stat_follows_and_lookup_nofollow_does_not) { + fs::file* f = fs::open("/sl_target", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "hello", 5), static_cast(5)); + fs::close(f); + + EXPECT_EQ(fs::symlink("/sl_target", "/sl_link"), fs::OK); + + fs::vattr target = {}; + fs::vattr through = {}; + EXPECT_EQ(fs::stat("/sl_target", &target), fs::OK); + EXPECT_EQ(fs::stat("/sl_link", &through), fs::OK); + EXPECT_EQ(through.type, fs::node_type::regular); + EXPECT_EQ(through.ino, target.ino); + EXPECT_EQ(through.size, static_cast(5)); + + fs::node* link = nullptr; + ASSERT_EQ(fs::lookup_at(nullptr, "/sl_link", fs::LOOKUP_NOFOLLOW, &link), fs::OK); + fs::vattr self = {}; + EXPECT_EQ(link->getattr(&self), fs::OK); + EXPECT_EQ(self.type, fs::node_type::symlink); + EXPECT_EQ(self.size, static_cast(10)); + EXPECT_NE(self.ino, target.ino); + + char buf[32] = {}; + size_t len = 0; + EXPECT_EQ(link->readlink(buf, sizeof(buf), &len), fs::OK); + EXPECT_EQ(len, static_cast(10)); + EXPECT_STREQ(buf, "/sl_target"); + release_node(link); + + fs::unlink("/sl_link"); + fs::unlink("/sl_target"); +} + +TEST(fs_test, symlink_open_reads_target_content) { + fs::file* f = fs::open("/sl_data", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "via link", 8), static_cast(8)); + fs::close(f); + EXPECT_EQ(fs::symlink("sl_data", "/sl_rel"), fs::OK); + + f = fs::open("/sl_rel", fs::O_RDONLY); + ASSERT_NOT_NULL(f); + char buf[16] = {}; + EXPECT_EQ(fs::read(f, buf, sizeof(buf)), static_cast(8)); + EXPECT_STREQ(buf, "via link"); + fs::close(f); + + fs::unlink("/sl_rel"); + fs::unlink("/sl_data"); +} + +TEST(fs_test, symlink_to_directory_in_the_middle_of_a_path) { + EXPECT_EQ(fs::mkdir("/sl_real", 0), fs::OK); + fs::file* f = fs::open("/sl_real/inner", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + EXPECT_EQ(fs::symlink("/sl_real", "/sl_dirlink"), fs::OK); + + fs::vattr direct = {}; + fs::vattr via = {}; + EXPECT_EQ(fs::stat("/sl_real/inner", &direct), fs::OK); + EXPECT_EQ(fs::stat("/sl_dirlink/inner", &via), fs::OK); + EXPECT_EQ(via.ino, direct.ino); + EXPECT_EQ(fs::stat("/sl_dirlink/../sl_real/inner", &via), fs::OK); + EXPECT_EQ(via.ino, direct.ino); + + fs::unlink("/sl_dirlink"); + fs::unlink("/sl_real/inner"); + fs::rmdir("/sl_real"); +} + +TEST(fs_test, symlink_loop_and_dangling_targets) { + EXPECT_EQ(fs::symlink("/sl_loop_b", "/sl_loop_a"), fs::OK); + EXPECT_EQ(fs::symlink("/sl_loop_a", "/sl_loop_b"), fs::OK); + EXPECT_EQ(fs::symlink("/sl_missing", "/sl_dangling"), fs::OK); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/sl_loop_a", &attr), fs::ERR_LOOP); + EXPECT_EQ(fs::stat("/sl_dangling", &attr), fs::ERR_NOENT); + + fs::node* link = nullptr; + EXPECT_EQ(fs::lookup_at(nullptr, "/sl_dangling", fs::LOOKUP_NOFOLLOW, &link), fs::OK); + release_node(link); + + fs::unlink("/sl_loop_a"); + fs::unlink("/sl_loop_b"); + fs::unlink("/sl_dangling"); +} + +// Writes "/sl_chain" with two decimal digits into out +static void chain_link_name(uint32_t index, char* out) { + const char prefix[] = "/sl_chain"; + string::memcpy(out, prefix, sizeof(prefix) - 1); + out[sizeof(prefix) - 1] = static_cast('0' + index / 10); + out[sizeof(prefix)] = static_cast('0' + index % 10); + out[sizeof(prefix) + 1] = '\0'; +} + +// Builds /sl_chain -> ... -> /sl_chain00 -> /sl_chain_end +static void make_symlink_chain(uint32_t links, char* head) { + fs::file* f = fs::open("/sl_chain_end", fs::O_CREAT | fs::O_RDWR); + if (f) { + fs::close(f); + } + + char target[32] = "/sl_chain_end"; + for (uint32_t i = 0; i < links; i++) { + chain_link_name(i, head); + fs::symlink(target, head); + string::memcpy(target, head, string::strlen(head) + 1); + } +} + +static void remove_symlink_chain(uint32_t links) { + fs::unlink("/sl_chain_end"); + + char name[32]; + for (uint32_t i = 0; i < links; i++) { + chain_link_name(i, name); + fs::unlink(name); + } +} + +TEST(fs_test, symlink_chain_within_limit_resolves) { + char head[32]; + make_symlink_chain(5, head); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat(head, &attr), fs::OK); + EXPECT_EQ(attr.type, fs::node_type::regular); + + remove_symlink_chain(5); +} + +TEST(fs_test, symlink_chain_beyond_limit_fails) { + char head[32]; + make_symlink_chain(fs::SYMLOOP_MAX + 1, head); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat(head, &attr), fs::ERR_LOOP); + + remove_symlink_chain(fs::SYMLOOP_MAX + 1); +} + +TEST(fs_test, symlink_unlink_and_rename_act_on_the_link) { + fs::file* f = fs::open("/sl_kept", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + EXPECT_EQ(fs::symlink("/sl_kept", "/sl_l1"), fs::OK); + + EXPECT_EQ(fs::rename("/sl_l1", "/sl_l2"), fs::OK); + fs::node* link = nullptr; + ASSERT_EQ(fs::lookup_at(nullptr, "/sl_l2", fs::LOOKUP_NOFOLLOW, &link), fs::OK); + EXPECT_EQ(link->type(), fs::node_type::symlink); + release_node(link); + + EXPECT_EQ(fs::unlink("/sl_l2"), fs::OK); + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/sl_kept", &attr), fs::OK); + EXPECT_EQ(fs::stat("/sl_l2", &attr), fs::ERR_NOENT); + + fs::unlink("/sl_kept"); +} + +TEST(fs_test, symlink_rejects_existing_name_and_empty_target) { + fs::file* f = fs::open("/sl_taken", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + EXPECT_EQ(fs::symlink("/anything", "/sl_taken"), fs::ERR_EXIST); + EXPECT_EQ(fs::symlink("", "/sl_empty"), fs::ERR_INVAL); + + fs::unlink("/sl_taken"); +} + TEST(fs_test, multi_page_write_read) { fs::file* f = fs::open("/bigfile", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); From e6de5a6196637076903f13734f7589cffa6e50f9 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 21:13:10 -0700 Subject: [PATCH 09/13] fix(fs): removed directories only while provably empty --- kernel/fs/dir_node.cpp | 26 ++++++++++++++++---------- kernel/fs/dir_node.h | 8 +++++--- kernel/fs/ramfs/ramfs.cpp | 7 +------ kernel/tests/fs/fs.test.cpp | 7 +++++++ 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/kernel/fs/dir_node.cpp b/kernel/fs/dir_node.cpp index f1b99249..1e9538f3 100644 --- a/kernel/fs/dir_node.cpp +++ b/kernel/fs/dir_node.cpp @@ -136,7 +136,9 @@ int32_t dir_node::rename_child(const char* name, size_t len, node* new_parent, return move_child_locked(name, len, dst, new_name, new_len); } -int32_t dir_node::detach_empty_dir_locked(dir_node* dir) { +// The directory's own lock is held from the emptiness check through the +// detach so no entry can appear in between +int32_t dir_node::detach_if_empty(dir_node* dir) { sync::irq_lock_guard guard(dir->m_lock); if (dir->mounted_here()) { @@ -151,6 +153,18 @@ int32_t dir_node::detach_empty_dir_locked(dir_node* dir) { return OK; } +int32_t dir_node::remove_empty_dir(dir_node* dir) { + // A temporary reference keeps the directory alive while its own lock is + // held across the detach, since the list reference may be its last + dir->add_ref(); + int32_t rc = detach_if_empty(dir); + if (dir->release()) { + node::ref_destroy(dir); + } + + return rc; +} + int32_t dir_node::replace_child_locked(node* child, node* existing) { bool child_is_dir = child->type() == node_type::directory; bool existing_is_dir = existing->type() == node_type::directory; @@ -168,15 +182,7 @@ int32_t dir_node::replace_child_locked(node* child, node* existing) { return OK; } - // A temporary reference keeps the directory alive while its own lock is - // held across the detach, since the list reference may be its last - existing->add_ref(); - int32_t rc = detach_empty_dir_locked(static_cast(existing)); - if (existing->release()) { - node::ref_destroy(existing); - } - - return rc; + return remove_empty_dir(static_cast(existing)); } int32_t dir_node::move_child_locked(const char* name, size_t len, dir_node* dst, diff --git a/kernel/fs/dir_node.h b/kernel/fs/dir_node.h index b2ac0aa5..48df3e12 100644 --- a/kernel/fs/dir_node.h +++ b/kernel/fs/dir_node.h @@ -22,8 +22,6 @@ class dir_node : public node { ssize_t readdir(file* f, dirent* entries, size_t count) override; int32_t getattr(vattr* attr) override; - uint32_t child_count() const { return m_child_count; } - protected: // Callers hold m_lock across a find and the attach or detach it decides node* find_child(const char* name, size_t len); @@ -35,11 +33,15 @@ class dir_node : public node { int32_t rename_child(const char* name, size_t len, node* new_parent, const char* new_name, size_t new_len); + // Caller holds m_lock. Detaches a child directory only while it is + // provably empty, refusing mount points and populated directories. + int32_t remove_empty_dir(dir_node* dir); + private: int32_t move_child_locked(const char* name, size_t len, dir_node* dst, const char* new_name, size_t new_len); int32_t replace_child_locked(node* child, node* existing); - int32_t detach_empty_dir_locked(dir_node* dir); + int32_t detach_if_empty(dir_node* dir); list::head m_children; uint32_t m_child_count; diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index 946cc0e7..f60d1382 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -177,12 +177,7 @@ int32_t dir_node::rmdir(const char* name, size_t len) { return fs::ERR_NOTDIR; } - if (static_cast(child)->child_count() > 0) { - return fs::ERR_NOTEMPTY; - } - - detach_child(child); - return fs::OK; + return remove_empty_dir(static_cast(child)); } int32_t dir_node::rename(const char* name, size_t len, fs::node* new_parent, diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index 8fa392b2..a50ba32d 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -119,6 +119,13 @@ TEST(fs_test, rmdir_nonempty_fails) { EXPECT_EQ(fs::rmdir("/parent"), fs::OK); } +TEST(fs_test, rmdir_refuses_mount_point) { + EXPECT_EQ(fs::rmdir("/dev"), fs::ERR_BUSY); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/dev/null", &attr), fs::OK); +} + TEST(fs_test, readdir_lists_children) { fs::mkdir("/rd_test", 0); fs::file* f1 = fs::open("/rd_test/a", fs::O_CREAT | fs::O_RDWR); From a64a2a31026b665f97f2ec259626d258cb9b80fc Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 21:19:23 -0700 Subject: [PATCH 10/13] style(fs): braced the single-line guards in the directory and node code --- kernel/fs/dir_node.cpp | 18 ++++++++++++++---- kernel/fs/fs.cpp | 12 +++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/kernel/fs/dir_node.cpp b/kernel/fs/dir_node.cpp index 1e9538f3..271f3067 100644 --- a/kernel/fs/dir_node.cpp +++ b/kernel/fs/dir_node.cpp @@ -227,21 +227,31 @@ int32_t dir_node::move_child_locked(const char* name, size_t len, dir_node* dst, } int32_t dir_node::lookup(const char* name, size_t len, node** out) { - if (!name || !out) return ERR_INVAL; + if (!name || !out) { + return ERR_INVAL; + } sync::irq_lock_guard guard(m_lock); + node* child = find_child(name, len); - if (!child) return ERR_NOENT; + if (!child) { + return ERR_NOENT; + } child->add_ref(); *out = child; + return OK; } ssize_t dir_node::readdir(file* f, dirent* entries, size_t count) { - if (!f || !entries) return ERR_BADF; + if (!f || !entries) { + return ERR_BADF; + } - if (count == 0) return 0; + if (count == 0) { + return 0; + } sync::irq_lock_guard guard(m_lock); diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index 058450dd..43c87cd2 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -92,9 +92,13 @@ int32_t node::getattr(vattr* attr) { } int32_t node::setattr(const vattr& attr, uint32_t mask) { - if (mask & ~(VATTR_ATIME | VATTR_MTIME)) return ERR_INVAL; + if (mask & ~(VATTR_ATIME | VATTR_MTIME)) { + return ERR_INVAL; + } - if (mask == 0) return OK; + if (mask == 0) { + return OK; + } if (mask & VATTR_ATIME) { m_atime_ns = attr.atime_ns; @@ -959,7 +963,9 @@ int32_t fstat(file* f, vattr* attr) { } int32_t fsetattr(file* f, const vattr& attr, uint32_t mask) { - if (!f) return ERR_BADF; + if (!f) { + return ERR_BADF; + } int32_t result; RUN_ELEVATED({ From b2bea96ee63b007035cb2f1bef5f18abda4a5cc3 Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 21:27:24 -0700 Subject: [PATCH 11/13] dynpriv(fs): moved path and I/O scratch buffers to the unprivileged heap --- kernel/fs/fs.cpp | 18 ++++++------ kernel/syscall/handlers/sys_fd.cpp | 46 +++++++++++++++--------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index 43c87cd2..f502880c 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -521,7 +521,7 @@ __PRIVILEGED_CODE int32_t path_from_node( return ERR_INVAL; } - char* path_buf = static_cast(heap::kzalloc(PATH_MAX)); + char* path_buf = static_cast(heap::uzalloc(PATH_MAX)); if (!path_buf) { return ERR_NOMEM; } @@ -541,7 +541,7 @@ __PRIVILEGED_CODE int32_t path_from_node( mount_point* mnt = find_mount_for_instance(cur->filesystem()); if (!mnt || !mnt->mountpoint) { release_node_ref(cur); - heap::kfree(path_buf); + heap::ufree(path_buf); return ERR_NOENT; } @@ -556,13 +556,13 @@ __PRIVILEGED_CODE int32_t path_from_node( size_t name_len = string::strnlen(name, NAME_MAX); if (name_len == 0) { release_node_ref(cur); - heap::kfree(path_buf); + heap::ufree(path_buf); return ERR_NOENT; } if (pos < name_len + 1) { release_node_ref(cur); - heap::kfree(path_buf); + heap::ufree(path_buf); return ERR_NAMETOOLONG; } @@ -573,7 +573,7 @@ __PRIVILEGED_CODE int32_t path_from_node( node* parent = cur->parent(); if (!parent) { release_node_ref(cur); - heap::kfree(path_buf); + heap::ufree(path_buf); return ERR_NOENT; } @@ -586,24 +586,24 @@ __PRIVILEGED_CODE int32_t path_from_node( if (pos == PATH_MAX - 1) { if (out_cap < 2) { - heap::kfree(path_buf); + heap::ufree(path_buf); return ERR_NAMETOOLONG; } out_path[0] = '/'; out_path[1] = '\0'; - heap::kfree(path_buf); + heap::ufree(path_buf); return OK; } size_t path_len = PATH_MAX - pos; if (out_cap < path_len) { - heap::kfree(path_buf); + heap::ufree(path_buf); return ERR_NAMETOOLONG; } string::memcpy(out_path, path_buf + pos, path_len); - heap::kfree(path_buf); + heap::ufree(path_buf); return OK; } diff --git a/kernel/syscall/handlers/sys_fd.cpp b/kernel/syscall/handlers/sys_fd.cpp index 2ecff9f1..61d41470 100644 --- a/kernel/syscall/handlers/sys_fd.cpp +++ b/kernel/syscall/handlers/sys_fd.cpp @@ -446,7 +446,7 @@ static int64_t normalize_path_for_dirfd( return base_rc; } - char* base_path = static_cast(heap::kzalloc(fs::PATH_MAX)); + char* base_path = static_cast(heap::uzalloc(fs::PATH_MAX)); if (!base_path) { release_node_ref(base_node); return syscall::ENOMEM; @@ -455,12 +455,12 @@ static int64_t normalize_path_for_dirfd( int32_t base_path_rc = fs::path_from_node(base_node, base_path, fs::PATH_MAX); release_node_ref(base_node); if (base_path_rc != fs::OK) { - heap::kfree(base_path); + heap::ufree(base_path); return syscall::error_map::map_fs_error(base_path_rc); } int64_t norm_rc = normalize_absolute_path(base_path, input_path, out_path, out_cap); - heap::kfree(base_path); + heap::ufree(base_path); return norm_rc; } @@ -743,7 +743,7 @@ static int64_t do_open_common(int64_t dirfd, uint64_t pathname, uint64_t flags, } uint32_t open_flags = static_cast(flags); - char* resolved_path = static_cast(heap::kzalloc(fs::PATH_MAX)); + char* resolved_path = static_cast(heap::uzalloc(fs::PATH_MAX)); if (!resolved_path) { return syscall::ENOMEM; } @@ -753,7 +753,7 @@ static int64_t do_open_common(int64_t dirfd, uint64_t pathname, uint64_t flags, int64_t norm_rc = normalize_absolute_path( nullptr, kpath, resolved_path, fs::PATH_MAX); if (norm_rc != 0) { - heap::kfree(resolved_path); + heap::ufree(resolved_path); return norm_rc; } @@ -763,7 +763,7 @@ static int64_t do_open_common(int64_t dirfd, uint64_t pathname, uint64_t flags, int64_t resolve_rc = resolve_open_resource_path( task, dirfd, kpath, open_flags, resolved_path, fs::PATH_MAX); if (resolve_rc != 0) { - heap::kfree(resolved_path); + heap::ufree(resolved_path); return resolve_rc; } @@ -773,7 +773,7 @@ static int64_t do_open_common(int64_t dirfd, uint64_t pathname, uint64_t flags, int64_t norm_rc = normalize_path_for_dirfd( task, dirfd, kpath, resolved_path, fs::PATH_MAX); if (norm_rc != 0) { - heap::kfree(resolved_path); + heap::ufree(resolved_path); return norm_rc; } @@ -781,7 +781,7 @@ static int64_t do_open_common(int64_t dirfd, uint64_t pathname, uint64_t flags, int64_t resolve_rc = resolve_open_resource_path( task, dirfd, kpath, open_flags, resolved_path, fs::PATH_MAX); if (resolve_rc != 0) { - heap::kfree(resolved_path); + heap::ufree(resolved_path); return resolve_rc; } } @@ -795,7 +795,7 @@ static int64_t do_open_common(int64_t dirfd, uint64_t pathname, uint64_t flags, open_flags, &handle ); - heap::kfree(resolved_path); + heap::ufree(resolved_path); if (rc != resource::OK) { return map_resource_error(rc); } @@ -863,7 +863,7 @@ DEFINE_SYSCALL3(read, fd, buf, count) { uint8_t* user_ptr = reinterpret_cast(buf); int64_t total = 0; - uint8_t* kbuf = static_cast(heap::kzalloc(IO_CHUNK_SIZE)); + uint8_t* kbuf = static_cast(heap::uzalloc(IO_CHUNK_SIZE)); if (!kbuf) { return syscall::ENOMEM; } @@ -872,7 +872,7 @@ DEFINE_SYSCALL3(read, fd, buf, count) { size_t chunk = remaining > IO_CHUNK_SIZE ? IO_CHUNK_SIZE : remaining; ssize_t n = resource::read(task, static_cast(fd), kbuf, chunk); if (n < 0) { - heap::kfree(kbuf); + heap::ufree(kbuf); if (total > 0) { return total; } @@ -886,7 +886,7 @@ DEFINE_SYSCALL3(read, fd, buf, count) { int32_t rc = mm::uaccess::copy_to_user(user_ptr, kbuf, static_cast(n)); if (rc != mm::uaccess::OK) { - heap::kfree(kbuf); + heap::ufree(kbuf); if (total > 0) { return total; } @@ -903,7 +903,7 @@ DEFINE_SYSCALL3(read, fd, buf, count) { } } - heap::kfree(kbuf); + heap::ufree(kbuf); return total; } @@ -925,7 +925,7 @@ DEFINE_SYSCALL3(write, fd, buf, count) { const uint8_t* user_ptr = reinterpret_cast(buf); int64_t total = 0; - uint8_t* kbuf = static_cast(heap::kzalloc(IO_CHUNK_SIZE)); + uint8_t* kbuf = static_cast(heap::uzalloc(IO_CHUNK_SIZE)); if (!kbuf) { return syscall::ENOMEM; } @@ -934,7 +934,7 @@ DEFINE_SYSCALL3(write, fd, buf, count) { size_t chunk = remaining > IO_CHUNK_SIZE ? IO_CHUNK_SIZE : remaining; int32_t copy_rc = mm::uaccess::copy_from_user(kbuf, user_ptr, chunk); if (copy_rc != mm::uaccess::OK) { - heap::kfree(kbuf); + heap::ufree(kbuf); if (total > 0) { return total; } @@ -944,7 +944,7 @@ DEFINE_SYSCALL3(write, fd, buf, count) { ssize_t n = resource::write(task, static_cast(fd), kbuf, chunk); if (n < 0) { - heap::kfree(kbuf); + heap::ufree(kbuf); if (total > 0) { return total; } @@ -965,7 +965,7 @@ DEFINE_SYSCALL3(write, fd, buf, count) { } } - heap::kfree(kbuf); + heap::ufree(kbuf); return total; } @@ -1337,7 +1337,7 @@ DEFINE_SYSCALL3(unlinkat, dirfd, pathname, flags_val) { char* normalized_path = nullptr; const char* shm_path = nullptr; if (kpath[0] == '/') { - normalized_path = static_cast(heap::kzalloc(fs::PATH_MAX)); + normalized_path = static_cast(heap::uzalloc(fs::PATH_MAX)); if (!normalized_path) { return syscall::ENOMEM; } @@ -1345,7 +1345,7 @@ DEFINE_SYSCALL3(unlinkat, dirfd, pathname, flags_val) { int64_t norm_rc = normalize_absolute_path( nullptr, kpath, normalized_path, fs::PATH_MAX); if (norm_rc != 0) { - heap::kfree(normalized_path); + heap::ufree(normalized_path); return norm_rc; } @@ -1353,7 +1353,7 @@ DEFINE_SYSCALL3(unlinkat, dirfd, pathname, flags_val) { shm_path = normalized_path; } } else { - normalized_path = static_cast(heap::kzalloc(fs::PATH_MAX)); + normalized_path = static_cast(heap::uzalloc(fs::PATH_MAX)); if (!normalized_path) { return syscall::ENOMEM; } @@ -1362,7 +1362,7 @@ DEFINE_SYSCALL3(unlinkat, dirfd, pathname, flags_val) { task, static_cast(dirfd), kpath, normalized_path, fs::PATH_MAX); if (norm_rc != 0) { - heap::kfree(normalized_path); + heap::ufree(normalized_path); return norm_rc; } @@ -1374,7 +1374,7 @@ DEFINE_SYSCALL3(unlinkat, dirfd, pathname, flags_val) { if (shm_path) { int32_t rc = resource::shm_provider::unlink_shm(shm_path); if (normalized_path) { - heap::kfree(normalized_path); + heap::ufree(normalized_path); } if (rc != resource::OK) { return map_resource_error(rc); @@ -1384,7 +1384,7 @@ DEFINE_SYSCALL3(unlinkat, dirfd, pathname, flags_val) { } if (normalized_path) { - heap::kfree(normalized_path); + heap::ufree(normalized_path); } fs::node* parent = nullptr; From 63582f1ff1d3dfccbf71d7f3292d5d6ee21fbb7e Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 21:59:37 -0700 Subject: [PATCH 12/13] fix(fs): closed the deadlocks and races between rename and rmdir --- kernel/fs/dir_node.cpp | 40 +++++++++++++++++++--- kernel/fs/dir_node.h | 7 ++-- kernel/fs/fs.cpp | 68 +++++++++++++++++++++++++++++++++++-- kernel/fs/ramfs/ramfs.cpp | 15 +------- kernel/tests/fs/fs.test.cpp | 61 +++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 24 deletions(-) diff --git a/kernel/fs/dir_node.cpp b/kernel/fs/dir_node.cpp index 271f3067..5532981a 100644 --- a/kernel/fs/dir_node.cpp +++ b/kernel/fs/dir_node.cpp @@ -5,10 +5,9 @@ namespace fs { -// Every rename serializes on this lock, which keeps the ancestor walk that -// refuses to move a directory into its own subtree reading a stable tree and -// makes locking a replaced directory beneath its parents deadlock free -static sync::spinlock g_rename_lock = sync::SPINLOCK_INIT; +// Serializes rename and rmdir, the operations that lock more than one directory, +// so nested directory locks cannot deadlock and ancestor walks see a stable tree +static sync::spinlock g_dir_tree_lock = sync::SPINLOCK_INIT; static bool is_dot_name(const char* name, size_t len) { return (len == 1 && name[0] == '.') || (len == 2 && name[0] == '.' && name[1] == '.'); @@ -118,7 +117,7 @@ int32_t dir_node::rename_child(const char* name, size_t len, node* new_parent, } auto* dst = static_cast(new_parent); - sync::irq_lock_guard rename_guard(g_rename_lock); + sync::irq_lock_guard tree_guard(g_dir_tree_lock); if (dst == this) { sync::irq_lock_guard guard(m_lock); @@ -153,6 +152,26 @@ int32_t dir_node::detach_if_empty(dir_node* dir) { return OK; } +int32_t dir_node::rmdir_child(const char* name, size_t len) { + if (!name || len == 0) { + return ERR_INVAL; + } + + sync::irq_lock_guard tree_guard(g_dir_tree_lock); + sync::irq_lock_guard guard(m_lock); + + node* child = find_child(name, len); + if (!child) { + return ERR_NOENT; + } + + if (child->type() != node_type::directory) { + return ERR_NOTDIR; + } + + return remove_empty_dir(static_cast(child)); +} + int32_t dir_node::remove_empty_dir(dir_node* dir) { // A temporary reference keeps the directory alive while its own lock is // held across the detach, since the list reference may be its last @@ -196,6 +215,11 @@ int32_t dir_node::move_child_locked(const char* name, size_t len, dir_node* dst, return ERR_BUSY; } + // A destination detached by a concurrent rmdir would swallow the child + if (dst->parent() == nullptr) { + return ERR_NOENT; + } + node* existing = dst->find_child(new_name, new_len); if (existing == child) { return OK; @@ -205,6 +229,12 @@ int32_t dir_node::move_child_locked(const char* name, size_t len, dir_node* dst, return ERR_INVAL; } + // A directory renamed onto its own parent's entry would replace the very + // directory still holding it, which is therefore never empty + if (existing == this && child->type() == node_type::directory) { + return ERR_NOTEMPTY; + } + if (existing) { int32_t rc = dst->replace_child_locked(child, existing); if (rc != OK) { diff --git a/kernel/fs/dir_node.h b/kernel/fs/dir_node.h index 48df3e12..664bb3ae 100644 --- a/kernel/fs/dir_node.h +++ b/kernel/fs/dir_node.h @@ -33,14 +33,15 @@ class dir_node : public node { int32_t rename_child(const char* name, size_t len, node* new_parent, const char* new_name, size_t new_len); - // Caller holds m_lock. Detaches a child directory only while it is - // provably empty, refusing mount points and populated directories. - int32_t remove_empty_dir(dir_node* dir); + // Removes an empty child directory, refusing mount points and populated + // directories. Takes every lock it needs itself. + int32_t rmdir_child(const char* name, size_t len); private: int32_t move_child_locked(const char* name, size_t len, dir_node* dst, const char* new_name, size_t new_len); int32_t replace_child_locked(node* child, node* existing); + int32_t remove_empty_dir(dir_node* dir); int32_t detach_if_empty(dir_node* dir); list::head m_children; diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index f502880c..440c1aee 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -747,6 +747,69 @@ __PRIVILEGED_CODE int32_t unmount(const char* target) { return ERR_NOSYS; } +// Creating through a link creates the final missing name of its chain, as the +// standard requires. dir holds the link and anchors its relative target. +__PRIVILEGED_CODE static int32_t create_symlink_target(node* dir, node* link, node** out) { + char* target = static_cast(heap::uzalloc(PATH_MAX)); + if (!target) { + return ERR_NOMEM; + } + + dir->add_ref(); + link->add_ref(); + + int32_t err = OK; + for (uint32_t depth = 0; ; depth++) { + if (depth >= SYMLOOP_MAX) { + err = ERR_LOOP; + break; + } + + size_t len = 0; + err = link->readlink(target, PATH_MAX - 1, &len); + if (err != OK) { + break; + } + target[len] = '\0'; + + node* parent = nullptr; + const char* name = nullptr; + size_t name_len = 0; + err = resolve_parent_at_internal(dir, target, &parent, &name, &name_len); + if (err != OK) { + break; + } + + release_node_ref(dir); + dir = parent; + + node* found = nullptr; + err = dir->lookup(name, name_len, &found); + if (err == ERR_NOENT) { + err = dir->create(name, name_len, 0, out); + break; + } + + if (err != OK) { + break; + } + + release_node_ref(link); + link = found; + if (link->type() != node_type::symlink) { + link->add_ref(); + *out = link; + break; + } + } + + release_node_ref(link); + release_node_ref(dir); + heap::ufree(target); + + return err; +} + file* open(const char* path, uint32_t flags) { return open_at(nullptr, path, flags, nullptr); } @@ -796,9 +859,10 @@ file* open_at(node* base_dir, const char* path, uint32_t flags, int32_t* out_err n = nullptr; err = ERR_EXIST; } else if (err == OK && n->type() == node_type::symlink) { - release_node_ref(n); + node* link = n; n = nullptr; - err = resolve_path_at_internal(base_dir, path, true, &n); + err = create_symlink_target(parent, link, &n); + release_node_ref(link); } else if (err == ERR_NOENT) { err = parent->create(name, name_len, 0, &n); if (err == ERR_EXIST && !(flags & O_EXCL)) { diff --git a/kernel/fs/ramfs/ramfs.cpp b/kernel/fs/ramfs/ramfs.cpp index f60d1382..2edd9fba 100644 --- a/kernel/fs/ramfs/ramfs.cpp +++ b/kernel/fs/ramfs/ramfs.cpp @@ -164,20 +164,7 @@ int32_t dir_node::unlink(const char* name, size_t len) { } int32_t dir_node::rmdir(const char* name, size_t len) { - if (!name || len == 0) return fs::ERR_INVAL; - - sync::irq_lock_guard guard(m_lock); - - fs::node* child = find_child(name, len); - if (!child) { - return fs::ERR_NOENT; - } - - if (child->type() != fs::node_type::directory) { - return fs::ERR_NOTDIR; - } - - return remove_empty_dir(static_cast(child)); + return rmdir_child(name, len); } int32_t dir_node::rename(const char* name, size_t len, fs::node* new_parent, diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index a50ba32d..03443d64 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -628,6 +628,43 @@ TEST(fs_test, rename_type_mismatch_fails) { fs::rmdir("/rn_dir"); } +TEST(fs_test, rename_onto_own_parent_fails) { + EXPECT_EQ(fs::mkdir("/rp", 0), fs::OK); + EXPECT_EQ(fs::mkdir("/rp/sub", 0), fs::OK); + fs::file* f = fs::open("/rp/f", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + EXPECT_EQ(fs::rename("/rp/sub", "/rp"), fs::ERR_NOTEMPTY); + EXPECT_EQ(fs::rename("/rp/f", "/rp"), fs::ERR_ISDIR); + + fs::unlink("/rp/f"); + fs::rmdir("/rp/sub"); + fs::rmdir("/rp"); +} + +TEST(fs_test, rename_into_removed_directory_fails) { + EXPECT_EQ(fs::mkdir("/rd_gone", 0), fs::OK); + fs::node* gone = nullptr; + ASSERT_EQ(fs::lookup("/rd_gone", &gone), fs::OK); + fs::file* f = fs::open("/rd_src", fs::O_CREAT | fs::O_RDWR); + ASSERT_NOT_NULL(f); + fs::close(f); + + EXPECT_EQ(fs::rmdir("/rd_gone"), fs::OK); + + fs::node* root = nullptr; + ASSERT_EQ(fs::lookup("/", &root), fs::OK); + EXPECT_EQ(root->rename("rd_src", 6, gone, "x", 1), fs::ERR_NOENT); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/rd_src", &attr), fs::OK); + + release_node(root); + release_node(gone); + fs::unlink("/rd_src"); +} + TEST(fs_test, rename_across_filesystems_fails) { fs::file* f = fs::open("/rn_xdev", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); @@ -823,6 +860,30 @@ TEST(fs_test, symlink_unlink_and_rename_act_on_the_link) { fs::unlink("/sl_kept"); } +TEST(fs_test, open_creat_through_dangling_symlink_creates_target) { + EXPECT_EQ(fs::symlink("/sl_ct_target", "/sl_ct_link"), fs::OK); + EXPECT_EQ(fs::symlink("sl_ct_link", "/sl_ct_link2"), fs::OK); + + int32_t err = fs::OK; + fs::file* f = fs::open("/sl_ct_link2", fs::O_CREAT | fs::O_RDWR, &err); + ASSERT_NOT_NULL(f); + EXPECT_EQ(fs::write(f, "made", 4), static_cast(4)); + fs::close(f); + + fs::vattr attr = {}; + EXPECT_EQ(fs::stat("/sl_ct_target", &attr), fs::OK); + EXPECT_EQ(attr.type, fs::node_type::regular); + EXPECT_EQ(attr.size, static_cast(4)); + + f = fs::open("/sl_ct_link", fs::O_CREAT | fs::O_EXCL | fs::O_RDWR, &err); + EXPECT_NULL(f); + EXPECT_EQ(err, fs::ERR_EXIST); + + fs::unlink("/sl_ct_link2"); + fs::unlink("/sl_ct_link"); + fs::unlink("/sl_ct_target"); +} + TEST(fs_test, symlink_rejects_existing_name_and_empty_target) { fs::file* f = fs::open("/sl_taken", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f); From 6dbcf7adbc2c105e0adfde5db84c5a2352cd629a Mon Sep 17 00:00:00 2001 From: Albert Slepak Date: Wed, 2 Sep 2026 22:18:52 -0700 Subject: [PATCH 13/13] fix(fs): reached mounted roots when creating through a symbolic link Opening a link with O_CREAT resolved its final target by plain directory lookup, which stops at a mount point instead of descending to the filesystem mounted there, so a link to a mount point opened the covered directory rather than the mounted root. The final step now descends mounts like every other resolution path, through one shared helper in place of three copies of the same loop, and losing a race to create the target looks the new file up instead of failing. --- kernel/fs/fs.cpp | 51 +++++++++++++++++-------------------- kernel/tests/fs/fs.test.cpp | 17 +++++++++++++ 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/kernel/fs/fs.cpp b/kernel/fs/fs.cpp index 440c1aee..a839f905 100644 --- a/kernel/fs/fs.cpp +++ b/kernel/fs/fs.cpp @@ -209,6 +209,18 @@ __PRIVILEGED_CODE static bool is_global_root_node(node* n) { return g_root_instance && n == g_root_instance->root(); } +// Steps from a mount point to the root mounted on it, carrying the reference +__PRIVILEGED_CODE static node* descend_mounts(node* n) { + while (n->mounted_here()) { + node* mounted_root = n->mounted_here()->root(); + mounted_root->add_ref(); + release_node_ref(n); + n = mounted_root; + } + + return n; +} + __PRIVILEGED_CODE static int32_t acquire_global_root(node** out_root) { if (!out_root || !g_root_mount || !g_root_instance) { return ERR_INVAL; @@ -221,14 +233,7 @@ __PRIVILEGED_CODE static int32_t acquire_global_root(node** out_root) { cur->add_ref(); - while (cur->mounted_here()) { - node* mounted_root = cur->mounted_here()->root(); - mounted_root->add_ref(); - release_node_ref(cur); - cur = mounted_root; - } - - *out_root = cur; + *out_root = descend_mounts(cur); return OK; } @@ -249,14 +254,8 @@ __PRIVILEGED_CODE static int32_t acquire_start_node( } start->add_ref(); - while (start->mounted_here()) { - node* mounted_root = start->mounted_here()->root(); - mounted_root->add_ref(); - release_node_ref(start); - start = mounted_root; - } - *out_start = start; + *out_start = descend_mounts(start); return OK; } @@ -398,15 +397,8 @@ __PRIVILEGED_CODE static int32_t resolve_path_at_internal( continue; } - while (child->mounted_here()) { - node* mounted_root = child->mounted_here()->root(); - mounted_root->add_ref(); - release_node_ref(child); - child = mounted_root; - } - release_node_ref(cur); - cur = child; + cur = descend_mounts(child); } if (walked) { @@ -787,6 +779,9 @@ __PRIVILEGED_CODE static int32_t create_symlink_target(node* dir, node* link, no err = dir->lookup(name, name_len, &found); if (err == ERR_NOENT) { err = dir->create(name, name_len, 0, out); + if (err == ERR_EXIST) { + err = dir->lookup(name, name_len, out); + } break; } @@ -794,13 +789,13 @@ __PRIVILEGED_CODE static int32_t create_symlink_target(node* dir, node* link, no break; } - release_node_ref(link); - link = found; - if (link->type() != node_type::symlink) { - link->add_ref(); - *out = link; + if (found->type() != node_type::symlink) { + *out = descend_mounts(found); break; } + + release_node_ref(link); + link = found; } release_node_ref(link); diff --git a/kernel/tests/fs/fs.test.cpp b/kernel/tests/fs/fs.test.cpp index 03443d64..600bdaf1 100644 --- a/kernel/tests/fs/fs.test.cpp +++ b/kernel/tests/fs/fs.test.cpp @@ -884,6 +884,23 @@ TEST(fs_test, open_creat_through_dangling_symlink_creates_target) { fs::unlink("/sl_ct_target"); } +TEST(fs_test, open_creat_through_symlink_reaches_mounted_root) { + EXPECT_EQ(fs::symlink("/dev", "/sl_devlink"), fs::OK); + + fs::file* f = fs::open("/sl_devlink", fs::O_CREAT | fs::O_RDONLY); + ASSERT_NOT_NULL(f); + + fs::vattr through = {}; + fs::vattr direct = {}; + EXPECT_EQ(fs::fstat(f, &through), fs::OK); + EXPECT_EQ(fs::stat("/dev", &direct), fs::OK); + EXPECT_EQ(through.ino, direct.ino); + EXPECT_EQ(through.dev, direct.dev); + + fs::close(f); + fs::unlink("/sl_devlink"); +} + TEST(fs_test, symlink_rejects_existing_name_and_empty_target) { fs::file* f = fs::open("/sl_taken", fs::O_CREAT | fs::O_RDWR); ASSERT_NOT_NULL(f);