From 60f7d61ad2b7bbaee4d78b0cf9c395c52b9587eb Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 16 Jul 2022 18:15:37 +0300 Subject: [PATCH] Kernel/SysFS: Fix parent directory hierarchy with symbolic links We should actually start counting from the parent directory and not from the symbolic link as it will represent a wrong count of hops from the actual mountpoint. The symlinks in /sys/dev/block and /sys/dev/char worked only by luck, because I have set it to the wrong parent directory which is the /sys/dev directory, so with the symlink it was 3 hops to /sys, together with the root directory, therefore, everything seemed to work. Now that the device symlinks in /sys/dev/block and /sys/dev/char are set to the right parent directory and we start measure hops from root directory with the parent directory of a symlink, everything seem to work correctly now. --- Kernel/FileSystem/SysFS/Component.cpp | 3 ++- .../SymbolicLinkDeviceComponent.cpp | 21 ++++++++++++++++--- .../SymbolicLinkDeviceComponent.h | 10 +++++++-- Kernel/Graphics/DisplayConnector.cpp | 2 +- Kernel/Storage/StorageDevice.cpp | 2 +- 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/Kernel/FileSystem/SysFS/Component.cpp b/Kernel/FileSystem/SysFS/Component.cpp index c42901ed7b..615bbe8fdc 100644 --- a/Kernel/FileSystem/SysFS/Component.cpp +++ b/Kernel/FileSystem/SysFS/Component.cpp @@ -91,7 +91,8 @@ static ErrorOr> generate_return_path_to_mount_point(Nonnu ErrorOr> SysFSSymbolicLink::try_generate_return_path_to_mount_point() const { - auto hops_from_mountpoint = TRY(relative_path_hops_count_from_mountpoint()); + VERIFY(m_parent_directory); + auto hops_from_mountpoint = TRY(m_parent_directory->relative_path_hops_count_from_mountpoint()); if (hops_from_mountpoint == 0) return KString::try_create("./"sv); auto start_return_path = TRY(KString::try_create("./"sv)); diff --git a/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.cpp b/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.cpp index ebf62c0a86..56725b58bb 100644 --- a/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.cpp +++ b/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.cpp @@ -11,17 +11,32 @@ namespace Kernel { -ErrorOr> SysFSSymbolicLinkDeviceComponent::try_create(SysFSDeviceIdentifiersDirectory const& parent_directory, Device const& device, SysFSComponent const& pointed_component) +ErrorOr> SysFSSymbolicLinkDeviceComponent::try_create(SysFSCharacterDevicesDirectory const& parent_directory, Device const& device, SysFSComponent const& pointed_component) { auto device_name = TRY(KString::formatted("{}:{}", device.major(), device.minor())); return adopt_nonnull_ref_or_enomem(new (nothrow) SysFSSymbolicLinkDeviceComponent(parent_directory, move(device_name), device, pointed_component)); } -SysFSSymbolicLinkDeviceComponent::SysFSSymbolicLinkDeviceComponent(SysFSDeviceIdentifiersDirectory const& parent_directory, NonnullOwnPtr major_minor_formatted_device_name, Device const& device, SysFSComponent const& pointed_component) + +ErrorOr> SysFSSymbolicLinkDeviceComponent::try_create(SysFSBlockDevicesDirectory const& parent_directory, Device const& device, SysFSComponent const& pointed_component) +{ + auto device_name = TRY(KString::formatted("{}:{}", device.major(), device.minor())); + return adopt_nonnull_ref_or_enomem(new (nothrow) SysFSSymbolicLinkDeviceComponent(parent_directory, move(device_name), device, pointed_component)); +} + +SysFSSymbolicLinkDeviceComponent::SysFSSymbolicLinkDeviceComponent(SysFSCharacterDevicesDirectory const& parent_directory, NonnullOwnPtr major_minor_formatted_device_name, Device const& device, SysFSComponent const& pointed_component) : SysFSSymbolicLink(parent_directory, pointed_component) , m_block_device(device.is_block_device()) , m_major_minor_formatted_device_name(move(major_minor_formatted_device_name)) { - VERIFY(device.is_block_device() || device.is_character_device()); + VERIFY(device.is_character_device()); +} + +SysFSSymbolicLinkDeviceComponent::SysFSSymbolicLinkDeviceComponent(SysFSBlockDevicesDirectory const& parent_directory, NonnullOwnPtr major_minor_formatted_device_name, Device const& device, SysFSComponent const& pointed_component) + : SysFSSymbolicLink(parent_directory, pointed_component) + , m_block_device(device.is_block_device()) + , m_major_minor_formatted_device_name(move(major_minor_formatted_device_name)) +{ + VERIFY(device.is_block_device()); } } diff --git a/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.h b/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.h index 8dcd9c6cb8..c900cca62d 100644 --- a/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.h +++ b/Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/SymbolicLinkDeviceComponent.h @@ -8,6 +8,8 @@ #include #include +#include +#include #include namespace Kernel { @@ -19,12 +21,16 @@ class SysFSSymbolicLinkDeviceComponent final friend class SysFSComponentRegistry; public: - static ErrorOr> try_create(SysFSDeviceIdentifiersDirectory const& parent_directory, Device const&, SysFSComponent const& pointed_component); + static ErrorOr> try_create(SysFSCharacterDevicesDirectory const& parent_directory, Device const& device, SysFSComponent const& pointed_component); + static ErrorOr> try_create(SysFSBlockDevicesDirectory const& parent_directory, Device const& device, SysFSComponent const& pointed_component); + virtual StringView name() const override { return m_major_minor_formatted_device_name->view(); } bool is_block_device() const { return m_block_device; } private: - SysFSSymbolicLinkDeviceComponent(SysFSDeviceIdentifiersDirectory const& parent_directory, NonnullOwnPtr major_minor_formatted_device_name, Device const&, SysFSComponent const& pointed_component); + SysFSSymbolicLinkDeviceComponent(SysFSCharacterDevicesDirectory const& parent_directory, NonnullOwnPtr major_minor_formatted_device_name, Device const&, SysFSComponent const& pointed_component); + SysFSSymbolicLinkDeviceComponent(SysFSBlockDevicesDirectory const& parent_directory, NonnullOwnPtr major_minor_formatted_device_name, Device const&, SysFSComponent const& pointed_component); + IntrusiveListNode> m_list_node; bool const m_block_device { false }; NonnullOwnPtr m_major_minor_formatted_device_name; diff --git a/Kernel/Graphics/DisplayConnector.cpp b/Kernel/Graphics/DisplayConnector.cpp index f9d82ec959..ffb47e792e 100644 --- a/Kernel/Graphics/DisplayConnector.cpp +++ b/Kernel/Graphics/DisplayConnector.cpp @@ -76,7 +76,7 @@ void DisplayConnector::after_inserting() m_sysfs_device_directory = sysfs_display_connector_device_directory; SysFSDisplayConnectorsDirectory::the().plug({}, *sysfs_display_connector_device_directory); VERIFY(!m_symlink_sysfs_component); - auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSDeviceIdentifiersDirectory::the(), *this, *m_sysfs_device_directory)); + auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSCharacterDevicesDirectory::the(), *this, *m_sysfs_device_directory)); m_symlink_sysfs_component = sys_fs_component; after_inserting_add_symlink_to_device_identifier_directory(); diff --git a/Kernel/Storage/StorageDevice.cpp b/Kernel/Storage/StorageDevice.cpp index d98a1af1c5..40bc8484aa 100644 --- a/Kernel/Storage/StorageDevice.cpp +++ b/Kernel/Storage/StorageDevice.cpp @@ -35,7 +35,7 @@ void StorageDevice::after_inserting() m_sysfs_device_directory = sysfs_storage_device_directory; SysFSStorageDirectory::the().plug({}, *sysfs_storage_device_directory); VERIFY(!m_symlink_sysfs_component); - auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSDeviceIdentifiersDirectory::the(), *this, *m_sysfs_device_directory)); + auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSBlockDevicesDirectory::the(), *this, *m_sysfs_device_directory)); m_symlink_sysfs_component = sys_fs_component; after_inserting_add_symlink_to_device_identifier_directory(); }