diff --git a/Kernel/Devices/Device.cpp b/Kernel/Devices/Device.cpp index 8e9cd4dd45a..a8dff9914ef 100644 --- a/Kernel/Devices/Device.cpp +++ b/Kernel/Devices/Device.cpp @@ -32,13 +32,14 @@ void Device::after_inserting_add_to_device_management() DeviceManagement::the().after_inserting_device({}, *this); } -void Device::after_inserting() +ErrorOr Device::after_inserting() { after_inserting_add_to_device_management(); VERIFY(!m_sysfs_component); auto sys_fs_component = SysFSDeviceComponent::must_create(*this); m_sysfs_component = sys_fs_component; after_inserting_add_to_device_identifier_directory(); + return {}; } void Device::will_be_destroyed() diff --git a/Kernel/Devices/Device.h b/Kernel/Devices/Device.h index 4a1e51c2191..a7d0369927b 100644 --- a/Kernel/Devices/Device.h +++ b/Kernel/Devices/Device.h @@ -50,7 +50,7 @@ public: virtual bool is_device() const override { return true; } virtual void will_be_destroyed() override; - virtual void after_inserting(); + virtual ErrorOr after_inserting(); virtual bool is_openable_by_jailed_processes() const { return false; } void process_next_queued_request(Badge, AsyncDeviceRequest const&); diff --git a/Kernel/Devices/DeviceManagement.h b/Kernel/Devices/DeviceManagement.h index d96a4d2a934..bd8f7a03f3e 100644 --- a/Kernel/Devices/DeviceManagement.h +++ b/Kernel/Devices/DeviceManagement.h @@ -57,7 +57,7 @@ public: requires(requires(Args... args) { DeviceType::try_create(args...); }) { auto device = TRY(DeviceType::try_create(forward(args)...)); - device->after_inserting(); + TRY(device->after_inserting()); return device; } @@ -65,7 +65,7 @@ public: static inline ErrorOr> try_create_device(Args&&... args) { auto device = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) DeviceType(forward(args)...))); - device->after_inserting(); + TRY(device->after_inserting()); return device; } diff --git a/Kernel/Graphics/DisplayConnector.cpp b/Kernel/Graphics/DisplayConnector.cpp index b358eda0e01..1e969deda80 100644 --- a/Kernel/Graphics/DisplayConnector.cpp +++ b/Kernel/Graphics/DisplayConnector.cpp @@ -55,43 +55,73 @@ void DisplayConnector::will_be_destroyed() { GraphicsManagement::the().detach_display_connector({}, *this); - VERIFY(m_symlink_sysfs_component); - before_will_be_destroyed_remove_symlink_from_device_identifier_directory(); + // NOTE: We check if m_symlink_sysfs_component is not null, because if we failed + // at some point in DisplayConnector::after_inserting(), then that method will tear down + // the object internal members safely, so we don't want to do it again here. + if (m_symlink_sysfs_component) { + before_will_be_destroyed_remove_symlink_from_device_identifier_directory(); + m_symlink_sysfs_component.clear(); + } + + // NOTE: We check if m_sysfs_device_directory is not null, because if we failed + // at some point in DisplayConnector::after_inserting(), then that method will tear down + // the object internal members safely, so we don't want to do it again here. + if (m_sysfs_device_directory) { + SysFSDisplayConnectorsDirectory::the().unplug({}, *m_sysfs_device_directory); + m_sysfs_device_directory.clear(); + } - m_symlink_sysfs_component.clear(); - SysFSDisplayConnectorsDirectory::the().unplug({}, *m_sysfs_device_directory); before_will_be_destroyed_remove_from_device_management(); } -void DisplayConnector::after_inserting() +ErrorOr DisplayConnector::after_inserting() { after_inserting_add_to_device_management(); + ArmedScopeGuard clean_from_device_management([&] { + before_will_be_destroyed_remove_from_device_management(); + }); + auto sysfs_display_connector_device_directory = DisplayConnectorSysFSDirectory::create(SysFSDisplayConnectorsDirectory::the(), *this); m_sysfs_device_directory = sysfs_display_connector_device_directory; SysFSDisplayConnectorsDirectory::the().plug({}, *sysfs_display_connector_device_directory); + ArmedScopeGuard clean_from_sysfs_display_connector_device_directory([&] { + SysFSDisplayConnectorsDirectory::the().unplug({}, *m_sysfs_device_directory); + m_sysfs_device_directory.clear(); + }); + VERIFY(!m_symlink_sysfs_component); - auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSCharacterDevicesDirectory::the(), *this, *m_sysfs_device_directory)); + auto sys_fs_component = TRY(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(); + ArmedScopeGuard clean_symlink_to_device_identifier_directory([&] { + VERIFY(m_symlink_sysfs_component); + before_will_be_destroyed_remove_symlink_from_device_identifier_directory(); + m_symlink_sysfs_component.clear(); + }); - auto rounded_size = MUST(Memory::page_round_up(m_framebuffer_resource_size)); + auto rounded_size = TRY(Memory::page_round_up(m_framebuffer_resource_size)); if (!m_framebuffer_at_arbitrary_physical_range) { VERIFY(m_framebuffer_address.value().page_base() == m_framebuffer_address.value()); - m_shared_framebuffer_vmobject = MUST(Memory::SharedFramebufferVMObject::try_create_for_physical_range(m_framebuffer_address.value(), rounded_size)); - m_framebuffer_region = MUST(MM.allocate_kernel_region(m_framebuffer_address.value().page_base(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite)); + m_shared_framebuffer_vmobject = TRY(Memory::SharedFramebufferVMObject::try_create_for_physical_range(m_framebuffer_address.value(), rounded_size)); + m_framebuffer_region = TRY(MM.allocate_kernel_region(m_framebuffer_address.value().page_base(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite)); } else { - m_shared_framebuffer_vmobject = MUST(Memory::SharedFramebufferVMObject::try_create_at_arbitrary_physical_range(rounded_size)); - m_framebuffer_region = MUST(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->real_writes_framebuffer_vmobject(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite)); + m_shared_framebuffer_vmobject = TRY(Memory::SharedFramebufferVMObject::try_create_at_arbitrary_physical_range(rounded_size)); + m_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->real_writes_framebuffer_vmobject(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite)); } m_framebuffer_data = m_framebuffer_region->vaddr().as_ptr(); - m_fake_writes_framebuffer_region = MUST(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->fake_writes_framebuffer_vmobject(), rounded_size, "Fake Writes Framebuffer"sv, Memory::Region::Access::ReadWrite)); + m_fake_writes_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->fake_writes_framebuffer_vmobject(), rounded_size, "Fake Writes Framebuffer"sv, Memory::Region::Access::ReadWrite)); + + clean_from_device_management.disarm(); + clean_from_sysfs_display_connector_device_directory.disarm(); + clean_symlink_to_device_identifier_directory.disarm(); GraphicsManagement::the().attach_new_display_connector({}, *this); if (m_enable_write_combine_optimization) { [[maybe_unused]] auto result = m_framebuffer_region->set_write_combine(true); } + return {}; } bool DisplayConnector::console_mode() const diff --git a/Kernel/Graphics/DisplayConnector.h b/Kernel/Graphics/DisplayConnector.h index 157dffcbb08..8c9db638070 100644 --- a/Kernel/Graphics/DisplayConnector.h +++ b/Kernel/Graphics/DisplayConnector.h @@ -146,7 +146,7 @@ private: DisplayConnector(DisplayConnector&&) = delete; virtual void will_be_destroyed() override; - virtual void after_inserting() override; + virtual ErrorOr after_inserting() override; ErrorOr ioctl_requires_ownership(unsigned request) const; diff --git a/Kernel/Storage/StorageDevice.cpp b/Kernel/Storage/StorageDevice.cpp index 6677794d7b3..af4ea96f3fa 100644 --- a/Kernel/Storage/StorageDevice.cpp +++ b/Kernel/Storage/StorageDevice.cpp @@ -36,23 +36,27 @@ StorageDevice::StorageDevice(Badge, LUNAddress logical_unit_numbe { } -void StorageDevice::after_inserting() +ErrorOr StorageDevice::after_inserting() { after_inserting_add_to_device_management(); auto sysfs_storage_device_directory = StorageDeviceSysFSDirectory::create(SysFSStorageDirectory::the(), *this); 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(SysFSBlockDevicesDirectory::the(), *this, *m_sysfs_device_directory)); + auto sys_fs_component = TRY(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(); + return {}; } void StorageDevice::will_be_destroyed() { - VERIFY(m_symlink_sysfs_component); - before_will_be_destroyed_remove_symlink_from_device_identifier_directory(); - m_symlink_sysfs_component.clear(); + // NOTE: We check if m_symlink_sysfs_component is not null, because if we failed + // in StorageDevice::after_inserting(), then that method will not set m_symlink_sysfs_component. + if (m_symlink_sysfs_component) { + before_will_be_destroyed_remove_symlink_from_device_identifier_directory(); + m_symlink_sysfs_component.clear(); + } SysFSStorageDirectory::the().unplug({}, *m_sysfs_device_directory); before_will_be_destroyed_remove_from_device_management(); } diff --git a/Kernel/Storage/StorageDevice.h b/Kernel/Storage/StorageDevice.h index 2732c44709d..165f8904fe3 100644 --- a/Kernel/Storage/StorageDevice.h +++ b/Kernel/Storage/StorageDevice.h @@ -88,7 +88,7 @@ protected: virtual StringView class_name() const override; private: - virtual void after_inserting() override; + virtual ErrorOr after_inserting() override; virtual void will_be_destroyed() override; mutable IntrusiveListNode> m_list_node; diff --git a/Kernel/TTY/MasterPTY.cpp b/Kernel/TTY/MasterPTY.cpp index acf897c8bb5..f8b9447540c 100644 --- a/Kernel/TTY/MasterPTY.cpp +++ b/Kernel/TTY/MasterPTY.cpp @@ -22,8 +22,8 @@ ErrorOr> MasterPTY::try_create(unsigned int index) auto master_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) MasterPTY(index, move(buffer)))); auto slave_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) SlavePTY(*master_pty, index))); master_pty->m_slave = slave_pty; - master_pty->after_inserting(); - slave_pty->after_inserting(); + TRY(master_pty->after_inserting()); + TRY(slave_pty->after_inserting()); return master_pty; } diff --git a/Kernel/TTY/PTYMultiplexer.cpp b/Kernel/TTY/PTYMultiplexer.cpp index d831926e582..118775858f2 100644 --- a/Kernel/TTY/PTYMultiplexer.cpp +++ b/Kernel/TTY/PTYMultiplexer.cpp @@ -35,7 +35,7 @@ UNMAP_AFTER_INIT PTYMultiplexer::~PTYMultiplexer() = default; UNMAP_AFTER_INIT void PTYMultiplexer::initialize() { - the().after_inserting(); + MUST(the().after_inserting()); } ErrorOr> PTYMultiplexer::open(int options)