From 58acdce41f60fec39bd09838ca492a21dd0ea5bb Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 28 May 2022 15:42:03 +0300 Subject: [PATCH] Kernel/FileSystem: Simplify even more the mount syscall As with the previous commit, we put a distinction between filesystems that require a file description and those which don't, but now in a much more readable mechanism - all initialization properties as well as the create static method are grouped to create the FileSystemInitializer structure. Then when we need to initialize an instance, we iterate over a table of these structures, checking for matching structure and then validating the given arguments from userspace against the requirements to ensure we can create a valid instance of the requested filesystem. --- Kernel/FileSystem/DevPtsFS.cpp | 4 +- Kernel/FileSystem/DevPtsFS.h | 2 +- Kernel/FileSystem/DevTmpFS.cpp | 4 +- Kernel/FileSystem/DevTmpFS.h | 2 +- Kernel/FileSystem/Ext2FileSystem.cpp | 4 +- Kernel/FileSystem/Ext2FileSystem.h | 2 +- Kernel/FileSystem/ISO9660FileSystem.cpp | 4 +- Kernel/FileSystem/ISO9660FileSystem.h | 2 +- Kernel/FileSystem/Plan9FileSystem.cpp | 4 +- Kernel/FileSystem/Plan9FileSystem.h | 2 +- Kernel/FileSystem/ProcFS.cpp | 4 +- Kernel/FileSystem/ProcFS.h | 2 +- Kernel/FileSystem/SysFS.cpp | 4 +- Kernel/FileSystem/SysFS.h | 2 +- Kernel/FileSystem/TmpFS.cpp | 4 +- Kernel/FileSystem/TmpFS.h | 2 +- Kernel/Syscalls/mount.cpp | 112 ++++++++++-------------- 17 files changed, 72 insertions(+), 88 deletions(-) diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp index edd2f205cc..02df952b62 100644 --- a/Kernel/FileSystem/DevPtsFS.cpp +++ b/Kernel/FileSystem/DevPtsFS.cpp @@ -12,9 +12,9 @@ namespace Kernel { -ErrorOr> DevPtsFS::try_create() +ErrorOr> DevPtsFS::try_create() { - return adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFS); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFS)); } DevPtsFS::DevPtsFS() = default; diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h index fc030dc121..f5683edb08 100644 --- a/Kernel/FileSystem/DevPtsFS.h +++ b/Kernel/FileSystem/DevPtsFS.h @@ -20,7 +20,7 @@ class DevPtsFS final : public FileSystem { public: virtual ~DevPtsFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "DevPtsFS"sv; } diff --git a/Kernel/FileSystem/DevTmpFS.cpp b/Kernel/FileSystem/DevTmpFS.cpp index 1f881154bf..f6d0e81dc5 100644 --- a/Kernel/FileSystem/DevTmpFS.cpp +++ b/Kernel/FileSystem/DevTmpFS.cpp @@ -11,9 +11,9 @@ namespace Kernel { -ErrorOr> DevTmpFS::try_create() +ErrorOr> DevTmpFS::try_create() { - return adopt_nonnull_ref_or_enomem(new (nothrow) DevTmpFS); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevTmpFS)); } DevTmpFS::DevTmpFS() = default; diff --git a/Kernel/FileSystem/DevTmpFS.h b/Kernel/FileSystem/DevTmpFS.h index 91ca9cdd54..e94da24a6a 100644 --- a/Kernel/FileSystem/DevTmpFS.h +++ b/Kernel/FileSystem/DevTmpFS.h @@ -20,7 +20,7 @@ class DevTmpFS final : public FileSystem { public: virtual ~DevTmpFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "DevTmpFS"sv; } diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp index 533a43e85b..e8e1ec5638 100644 --- a/Kernel/FileSystem/Ext2FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FileSystem.cpp @@ -49,9 +49,9 @@ static u8 to_ext2_file_type(mode_t mode) return EXT2_FT_UNKNOWN; } -ErrorOr> Ext2FS::try_create(OpenFileDescription& file_description) +ErrorOr> Ext2FS::try_create(OpenFileDescription& file_description) { - return adopt_nonnull_ref_or_enomem(new (nothrow) Ext2FS(file_description)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Ext2FS(file_description))); } Ext2FS::Ext2FS(OpenFileDescription& file_description) diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h index 90563b9548..d407d21b12 100644 --- a/Kernel/FileSystem/Ext2FileSystem.h +++ b/Kernel/FileSystem/Ext2FileSystem.h @@ -86,7 +86,7 @@ public: FileSize64bits = 1 << 1, }; - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual ~Ext2FS() override; virtual ErrorOr initialize() override; diff --git a/Kernel/FileSystem/ISO9660FileSystem.cpp b/Kernel/FileSystem/ISO9660FileSystem.cpp index 4c08f6fbee..5d54a2e4c3 100644 --- a/Kernel/FileSystem/ISO9660FileSystem.cpp +++ b/Kernel/FileSystem/ISO9660FileSystem.cpp @@ -168,9 +168,9 @@ private: Vector m_directory_stack; }; -ErrorOr> ISO9660FS::try_create(OpenFileDescription& description) +ErrorOr> ISO9660FS::try_create(OpenFileDescription& description) { - return adopt_nonnull_ref_or_enomem(new (nothrow) ISO9660FS(description)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ISO9660FS(description))); } ISO9660FS::ISO9660FS(OpenFileDescription& description) diff --git a/Kernel/FileSystem/ISO9660FileSystem.h b/Kernel/FileSystem/ISO9660FileSystem.h index 48e4c6941f..99f9683d09 100644 --- a/Kernel/FileSystem/ISO9660FileSystem.h +++ b/Kernel/FileSystem/ISO9660FileSystem.h @@ -305,7 +305,7 @@ public: } }; - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual ~ISO9660FS() override; virtual ErrorOr initialize() override; diff --git a/Kernel/FileSystem/Plan9FileSystem.cpp b/Kernel/FileSystem/Plan9FileSystem.cpp index 3d079183be..7bc3b6f9df 100644 --- a/Kernel/FileSystem/Plan9FileSystem.cpp +++ b/Kernel/FileSystem/Plan9FileSystem.cpp @@ -9,9 +9,9 @@ namespace Kernel { -ErrorOr> Plan9FS::try_create(OpenFileDescription& file_description) +ErrorOr> Plan9FS::try_create(OpenFileDescription& file_description) { - return adopt_nonnull_ref_or_enomem(new (nothrow) Plan9FS(file_description)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Plan9FS(file_description))); } Plan9FS::Plan9FS(OpenFileDescription& file_description) diff --git a/Kernel/FileSystem/Plan9FileSystem.h b/Kernel/FileSystem/Plan9FileSystem.h index f992c3a0fb..590d317124 100644 --- a/Kernel/FileSystem/Plan9FileSystem.h +++ b/Kernel/FileSystem/Plan9FileSystem.h @@ -20,7 +20,7 @@ class Plan9FS final : public FileBackedFileSystem { public: virtual ~Plan9FS() override; - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual ErrorOr initialize() override; diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 844c95d7a4..aa44b2b849 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -37,9 +37,9 @@ UNMAP_AFTER_INIT ProcFSComponentRegistry::ProcFSComponentRegistry() { } -ErrorOr> ProcFS::try_create() +ErrorOr> ProcFS::try_create() { - return adopt_nonnull_ref_or_enomem(new (nothrow) ProcFS()); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcFS)); } ProcFS::ProcFS() = default; diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index 97d5c9051b..63811e31e1 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -28,7 +28,7 @@ class ProcFS final : public FileSystem { public: virtual ~ProcFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "ProcFS"sv; } diff --git a/Kernel/FileSystem/SysFS.cpp b/Kernel/FileSystem/SysFS.cpp index fe790cae68..3875597531 100644 --- a/Kernel/FileSystem/SysFS.cpp +++ b/Kernel/FileSystem/SysFS.cpp @@ -68,9 +68,9 @@ SysFSRootDirectory::SysFSRootDirectory() m_buses_directory = buses_directory; } -ErrorOr> SysFS::try_create() +ErrorOr> SysFS::try_create() { - return adopt_nonnull_ref_or_enomem(new (nothrow) SysFS); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) SysFS)); } SysFS::SysFS() = default; diff --git a/Kernel/FileSystem/SysFS.h b/Kernel/FileSystem/SysFS.h index fb50384158..1d600bcdae 100644 --- a/Kernel/FileSystem/SysFS.h +++ b/Kernel/FileSystem/SysFS.h @@ -118,7 +118,7 @@ class SysFS final : public FileSystem { public: virtual ~SysFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "SysFS"sv; } diff --git a/Kernel/FileSystem/TmpFS.cpp b/Kernel/FileSystem/TmpFS.cpp index e4b54416ea..d6f65fbc16 100644 --- a/Kernel/FileSystem/TmpFS.cpp +++ b/Kernel/FileSystem/TmpFS.cpp @@ -10,9 +10,9 @@ namespace Kernel { -ErrorOr> TmpFS::try_create() +ErrorOr> TmpFS::try_create() { - return adopt_nonnull_ref_or_enomem(new (nothrow) TmpFS); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) TmpFS)); } TmpFS::TmpFS() = default; diff --git a/Kernel/FileSystem/TmpFS.h b/Kernel/FileSystem/TmpFS.h index 1ca5754743..a6269aec05 100644 --- a/Kernel/FileSystem/TmpFS.h +++ b/Kernel/FileSystem/TmpFS.h @@ -19,7 +19,7 @@ class TmpFS final : public FileSystem { public: virtual ~TmpFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "TmpFS"sv; } diff --git a/Kernel/Syscalls/mount.cpp b/Kernel/Syscalls/mount.cpp index 00d68006aa..ecf4894b5d 100644 --- a/Kernel/Syscalls/mount.cpp +++ b/Kernel/Syscalls/mount.cpp @@ -18,61 +18,52 @@ namespace Kernel { -static ErrorOr> create_ram_backed_filesystem_instance(StringView fs_type) -{ - RefPtr fs; - if (fs_type == "proc"sv || fs_type == "ProcFS"sv) { - fs = TRY(ProcFS::try_create()); - } else if (fs_type == "devpts"sv || fs_type == "DevPtsFS"sv) { - fs = TRY(DevPtsFS::try_create()); - } else if (fs_type == "dev"sv || fs_type == "DevTmpFS"sv) { - fs = TRY(DevTmpFS::try_create()); - } else if (fs_type == "sys"sv || fs_type == "SysFS"sv) { - fs = TRY(SysFS::try_create()); - } else if (fs_type == "tmp"sv || fs_type == "TmpFS"sv) { - fs = TRY(TmpFS::try_create()); - } - if (!fs) - return ENODEV; - return fs.release_nonnull(); -} +struct FileSystemInitializer { + StringView short_name; + StringView name; + bool requires_open_file_description { false }; + bool requires_block_device { false }; + bool requires_seekable_file { false }; + ErrorOr> (*create_with_fd)(OpenFileDescription&) = nullptr; + ErrorOr> (*create)(void) = nullptr; +}; -static bool filesystem_mount_require_open_file_description(StringView fs_type) -{ - if (fs_type == "ext2"sv || fs_type == "Ext2FS"sv) { - return true; - } else if (fs_type == "9p"sv || fs_type == "Plan9FS"sv) { - return true; - } else if (fs_type == "iso9660"sv || fs_type == "ISO9660FS"sv) { - return true; - } - return false; -} +static constexpr FileSystemInitializer s_initializers[] = { + { "proc"sv, "ProcFS"sv, false, false, false, {}, ProcFS::try_create }, + { "devpts"sv, "DevPtsFS"sv, false, false, false, {}, DevPtsFS::try_create }, + { "dev"sv, "DevTmpFS"sv, false, false, false, {}, DevTmpFS::try_create }, + { "sys"sv, "SysFS"sv, false, false, false, {}, SysFS::try_create }, + { "tmp"sv, "TmpFS"sv, false, false, false, {}, TmpFS::try_create }, + { "ext2"sv, "Ext2FS"sv, true, true, true, Ext2FS::try_create, {} }, + { "9p"sv, "Plan9FS"sv, true, true, true, Plan9FS::try_create, {} }, + { "iso9660"sv, "ISO9660FS"sv, true, true, true, ISO9660FS::try_create, {} }, +}; -static ErrorOr> create_open_file_description_backed_filesystem_instance(StringView fs_type, OpenFileDescription& description) +static ErrorOr> create_filesystem_instance(StringView fs_type, OpenFileDescription* possible_description) { - RefPtr fs; + for (auto& initializer_entry : s_initializers) { + if (fs_type != initializer_entry.short_name && fs_type != initializer_entry.name) + continue; + if (!initializer_entry.requires_open_file_description) { + VERIFY(initializer_entry.create); + NonnullRefPtr fs = TRY(initializer_entry.create()); + return fs; + } + VERIFY(initializer_entry.create_with_fd); + if (!possible_description) + return EBADF; + OpenFileDescription& description = *possible_description; - if (fs_type == "ext2"sv || fs_type == "Ext2FS"sv) { - if (!description.file().is_block_device()) + if (initializer_entry.requires_block_device && !description.file().is_block_device()) return ENOTBLK; - if (!description.file().is_seekable()) { + if (initializer_entry.requires_seekable_file && !description.file().is_seekable()) { dbgln("mount: this is not a seekable file"); return ENODEV; } - fs = TRY(Ext2FS::try_create(description)); - } else if (fs_type == "9p"sv || fs_type == "Plan9FS"sv) { - fs = TRY(Plan9FS::try_create(description)); - } else if (fs_type == "iso9660"sv || fs_type == "ISO9660FS"sv) { - if (!description.file().is_seekable()) { - dbgln("mount: this is not a seekable file"); - return ENODEV; - } - fs = TRY(ISO9660FS::try_create(description)); + NonnullRefPtr fs = TRY(initializer_entry.create_with_fd(description)); + return fs; } - if (!fs) - return ENODEV; - return fs.release_nonnull(); + return ENODEV; } ErrorOr Process::sys$mount(Userspace user_params) @@ -116,26 +107,19 @@ ErrorOr Process::sys$mount(Userspace u return 0; } - // Note: Try to determine as early as possible if we deal with a filesystem type - // that must be backed by a open file description, so if there's no such valid - // description, we can fail with EBADF now. - if (filesystem_mount_require_open_file_description(fs_type) && description_or_error.is_error()) { - return EBADF; + RefPtr fs; + + if (!description_or_error.is_error()) { + auto description = description_or_error.release_value(); + fs = TRY(create_filesystem_instance(fs_type, description.ptr())); + auto source_pseudo_path = TRY(description->pseudo_path()); + dbgln("mount: attempting to mount {} on {}", source_pseudo_path, target); + } else { + fs = TRY(create_filesystem_instance(fs_type, {})); } - if (description_or_error.is_error()) { - auto synthetic_filesystem = TRY(create_ram_backed_filesystem_instance(fs_type)); - TRY(synthetic_filesystem->initialize()); - TRY(VirtualFileSystem::the().mount(*synthetic_filesystem, target_custody, params.flags)); - return 0; - } - auto description = description_or_error.release_value(); - auto open_file_description_backed_filesystem = TRY(create_open_file_description_backed_filesystem_instance(fs_type, description)); - auto source_pseudo_path = TRY(description->pseudo_path()); - dbgln("mount: attempting to mount {} on {}", source_pseudo_path, target); - TRY(open_file_description_backed_filesystem->initialize()); - TRY(VirtualFileSystem::the().mount(*open_file_description_backed_filesystem, target_custody, params.flags)); - + TRY(fs->initialize()); + TRY(VirtualFileSystem::the().mount(*fs, target_custody, params.flags)); return 0; }