From 90ac9d725346cd7add92890c7aaca415f0d2f289 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 7 Jan 2023 07:38:43 +0200 Subject: [PATCH] Kernel/Net: Allocate regions before invoking the RTL8139 constructor Instead of allocating those regions in the constructor, which makes it impossible to fail in case of OOM condition, allocate them in the static factory method so we could propagate errors in case of failure. --- Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp | 10 ++++++---- Kernel/Net/Realtek/RTL8139NetworkAdapter.h | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp b/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp index b856814511..54e7a2241a 100644 --- a/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp +++ b/Kernel/Net/Realtek/RTL8139NetworkAdapter.cpp @@ -118,18 +118,20 @@ UNMAP_AFTER_INIT ErrorOr> RTL8139NetworkAdapte if (pci_device_identifier.hardware_id() != rtl8139_id) return nullptr; u8 irq = pci_device_identifier.interrupt_line().value(); + auto rx_buffer = TRY(MM.allocate_contiguous_kernel_region(TRY(Memory::page_round_up(RX_BUFFER_SIZE + PACKET_SIZE_MAX)), "RTL8139 RX"sv, Memory::Region::Access::ReadWrite)); + auto packet_buffer = TRY(MM.allocate_contiguous_kernel_region(TRY(Memory::page_round_up(PACKET_SIZE_MAX)), "RTL8139 Packet buffer"sv, Memory::Region::Access::ReadWrite)); auto interface_name = TRY(NetworkingManagement::generate_interface_name_from_pci_address(pci_device_identifier)); auto registers_io_window = TRY(IOWindow::create_for_pci_device_bar(pci_device_identifier, PCI::HeaderType0BaseRegister::BAR0)); - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) RTL8139NetworkAdapter(pci_device_identifier.address(), irq, move(registers_io_window), move(interface_name)))); + return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) RTL8139NetworkAdapter(pci_device_identifier.address(), irq, move(rx_buffer), move(packet_buffer), move(registers_io_window), move(interface_name)))); } -UNMAP_AFTER_INIT RTL8139NetworkAdapter::RTL8139NetworkAdapter(PCI::Address address, u8 irq, NonnullOwnPtr registers_io_window, NonnullOwnPtr interface_name) +UNMAP_AFTER_INIT RTL8139NetworkAdapter::RTL8139NetworkAdapter(PCI::Address address, u8 irq, NonnullOwnPtr rx_buffer, NonnullOwnPtr packet_buffer, NonnullOwnPtr registers_io_window, NonnullOwnPtr interface_name) : NetworkAdapter(move(interface_name)) , PCI::Device(address) , IRQHandler(irq) , m_registers_io_window(move(registers_io_window)) - , m_rx_buffer(MM.allocate_contiguous_kernel_region(Memory::page_round_up(RX_BUFFER_SIZE + PACKET_SIZE_MAX).release_value_but_fixme_should_propagate_errors(), "RTL8139 RX"sv, Memory::Region::Access::ReadWrite).release_value()) - , m_packet_buffer(MM.allocate_contiguous_kernel_region(Memory::page_round_up(PACKET_SIZE_MAX).release_value_but_fixme_should_propagate_errors(), "RTL8139 Packet buffer"sv, Memory::Region::Access::ReadWrite).release_value()) + , m_rx_buffer(move(rx_buffer)) + , m_packet_buffer(move(packet_buffer)) { m_tx_buffers.ensure_capacity(RTL8139_TX_BUFFER_COUNT); diff --git a/Kernel/Net/Realtek/RTL8139NetworkAdapter.h b/Kernel/Net/Realtek/RTL8139NetworkAdapter.h index b1062b8f4b..73cbaa2438 100644 --- a/Kernel/Net/Realtek/RTL8139NetworkAdapter.h +++ b/Kernel/Net/Realtek/RTL8139NetworkAdapter.h @@ -35,7 +35,7 @@ public: virtual StringView device_name() const override { return "RTL8139"sv; } private: - RTL8139NetworkAdapter(PCI::Address, u8 irq, NonnullOwnPtr registers_io_window, NonnullOwnPtr); + RTL8139NetworkAdapter(PCI::Address, u8 irq, NonnullOwnPtr rx_buffer, NonnullOwnPtr packet_buffer, NonnullOwnPtr registers_io_window, NonnullOwnPtr); virtual bool handle_irq(RegisterState const&) override; virtual StringView class_name() const override { return "RTL8139NetworkAdapter"sv; } @@ -53,11 +53,11 @@ private: NonnullOwnPtr m_registers_io_window; u8 m_interrupt_line { 0 }; - OwnPtr m_rx_buffer; + NonnullOwnPtr m_rx_buffer; u16 m_rx_buffer_offset { 0 }; Vector> m_tx_buffers; u8 m_tx_next_buffer { 0 }; - OwnPtr m_packet_buffer; + NonnullOwnPtr m_packet_buffer; bool m_link_up { false }; EntropySource m_entropy_source; };