From ca94a833374b3fec720cc8ee911b9824c3021828 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Sun, 1 Aug 2021 02:42:03 -0700 Subject: [PATCH] Kernel: Handle OOM from DoubleBuffer usage in IPv4Socket The IPv4Socket requires a DoubleBuffer for storage of any data it received on the socket. However it was previously using the default constructor which can not observe allocation failure. Address this by plumbing the receive buffer through the various derived classes. --- Kernel/Net/IPv4Socket.cpp | 32 +++++++++++++++++++++----------- Kernel/Net/IPv4Socket.h | 6 ++++-- Kernel/Net/TCPSocket.cpp | 13 ++++++++----- Kernel/Net/TCPSocket.h | 4 ++-- Kernel/Net/UDPSocket.cpp | 8 ++++---- Kernel/Net/UDPSocket.h | 4 ++-- 6 files changed, 41 insertions(+), 26 deletions(-) diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index a539b4c503..9f27421697 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -35,22 +35,31 @@ Lockable>& IPv4Socket::all_sockets() return *s_table; } +OwnPtr IPv4Socket::create_receive_buffer() +{ + return DoubleBuffer::try_create(256 * KiB); +} + KResultOr> IPv4Socket::create(int type, int protocol) { + auto receive_buffer = IPv4Socket::create_receive_buffer(); + if (!receive_buffer) + return ENOMEM; + if (type == SOCK_STREAM) { - auto tcp_socket = TCPSocket::create(protocol); + auto tcp_socket = TCPSocket::create(protocol, receive_buffer.release_nonnull()); if (tcp_socket.is_error()) return tcp_socket.error(); return tcp_socket.release_value(); } if (type == SOCK_DGRAM) { - auto udp_socket = UDPSocket::create(protocol); + auto udp_socket = UDPSocket::create(protocol, receive_buffer.release_nonnull()); if (udp_socket.is_error()) return udp_socket.error(); return udp_socket.release_value(); } if (type == SOCK_RAW) { - auto raw_socket = adopt_ref_if_nonnull(new (nothrow) IPv4Socket(type, protocol)); + auto raw_socket = adopt_ref_if_nonnull(new (nothrow) IPv4Socket(type, protocol, receive_buffer.release_nonnull())); if (raw_socket) return raw_socket.release_nonnull(); return ENOMEM; @@ -58,8 +67,9 @@ KResultOr> IPv4Socket::create(int type, int protocol) return EINVAL; } -IPv4Socket::IPv4Socket(int type, int protocol) +IPv4Socket::IPv4Socket(int type, int protocol, NonnullOwnPtr receive_buffer) : Socket(AF_INET, type, protocol) + , m_receive_buffer(move(receive_buffer)) { dbgln_if(IPV4_SOCKET_DEBUG, "IPv4Socket({}) created with type={}, protocol={}", this, type, protocol); m_buffer_mode = type == SOCK_STREAM ? BufferMode::Bytes : BufferMode::Packets; @@ -248,7 +258,7 @@ KResultOr IPv4Socket::sendto(FileDescription&, const UserOrKernelBuffer& KResultOr IPv4Socket::receive_byte_buffered(FileDescription& description, UserOrKernelBuffer& buffer, size_t buffer_length, int flags, Userspace, Userspace) { MutexLocker locker(lock()); - if (m_receive_buffer.is_empty()) { + if (m_receive_buffer->is_empty()) { if (protocol_is_disconnected()) return 0; if (!description.is_blocking()) @@ -270,14 +280,14 @@ KResultOr IPv4Socket::receive_byte_buffered(FileDescription& description KResultOr nreceived_or_error { 0 }; if (flags & MSG_PEEK) - nreceived_or_error = m_receive_buffer.peek(buffer, buffer_length); + nreceived_or_error = m_receive_buffer->peek(buffer, buffer_length); else - nreceived_or_error = m_receive_buffer.read(buffer, buffer_length); + nreceived_or_error = m_receive_buffer->read(buffer, buffer_length); if (!nreceived_or_error.is_error() && nreceived_or_error.value() > 0 && !(flags & MSG_PEEK)) Thread::current()->did_ipv4_socket_read(nreceived_or_error.value()); - set_can_read(!m_receive_buffer.is_empty()); + set_can_read(!m_receive_buffer->is_empty()); return nreceived_or_error; } @@ -406,7 +416,7 @@ bool IPv4Socket::did_receive(const IPv4Address& source_address, u16 source_port, auto packet_size = packet.size(); if (buffer_mode() == BufferMode::Bytes) { - size_t space_in_receive_buffer = m_receive_buffer.space_for_writing(); + size_t space_in_receive_buffer = m_receive_buffer->space_for_writing(); if (packet_size > space_in_receive_buffer) { dbgln("IPv4Socket({}): did_receive refusing packet since buffer is full.", this); VERIFY(m_can_read); @@ -416,10 +426,10 @@ bool IPv4Socket::did_receive(const IPv4Address& source_address, u16 source_port, auto nreceived_or_error = protocol_receive(ReadonlyBytes { packet.data(), packet.size() }, scratch_buffer, m_scratch_buffer.value().size(), 0); if (nreceived_or_error.is_error()) return false; - auto nwritten_or_error = m_receive_buffer.write(scratch_buffer, nreceived_or_error.value()); + auto nwritten_or_error = m_receive_buffer->write(scratch_buffer, nreceived_or_error.value()); if (nwritten_or_error.is_error()) return false; - set_can_read(!m_receive_buffer.is_empty()); + set_can_read(!m_receive_buffer->is_empty()); } else { if (m_receive_queue.size() > 2000) { dbgln("IPv4Socket({}): did_receive refusing packet since queue is full.", this); diff --git a/Kernel/Net/IPv4Socket.h b/Kernel/Net/IPv4Socket.h index 3d47160aba..06583bcc53 100644 --- a/Kernel/Net/IPv4Socket.h +++ b/Kernel/Net/IPv4Socket.h @@ -74,7 +74,7 @@ public: BufferMode buffer_mode() const { return m_buffer_mode; } protected: - IPv4Socket(int type, int protocol); + IPv4Socket(int type, int protocol, NonnullOwnPtr receive_buffer); virtual StringView class_name() const override { return "IPv4Socket"; } PortAllocationResult allocate_local_port_if_needed(); @@ -92,6 +92,8 @@ protected: void set_local_address(IPv4Address address) { m_local_address = address; } void set_peer_address(IPv4Address address) { m_peer_address = address; } + static OwnPtr create_receive_buffer(); + private: virtual bool is_ipv4() const override { return true; } @@ -115,7 +117,7 @@ private: SinglyLinkedListWithCount m_receive_queue; - DoubleBuffer m_receive_buffer { 256 * KiB }; + NonnullOwnPtr m_receive_buffer; u16 m_local_port { 0 }; u16 m_peer_port { 0 }; diff --git a/Kernel/Net/TCPSocket.cpp b/Kernel/Net/TCPSocket.cpp index 87b00b699b..d4da91fc01 100644 --- a/Kernel/Net/TCPSocket.cpp +++ b/Kernel/Net/TCPSocket.cpp @@ -96,7 +96,10 @@ RefPtr TCPSocket::create_client(const IPv4Address& new_local_address, return {}; } - auto result = TCPSocket::create(protocol()); + auto receive_buffer = create_receive_buffer(); + if (!receive_buffer) + return {}; + auto result = TCPSocket::create(protocol(), receive_buffer.release_nonnull()); if (result.is_error()) return {}; @@ -131,8 +134,8 @@ void TCPSocket::release_for_accept(RefPtr socket) [[maybe_unused]] auto rc = queue_connection_from(*socket); } -TCPSocket::TCPSocket(int protocol) - : IPv4Socket(SOCK_STREAM, protocol) +TCPSocket::TCPSocket(int protocol, NonnullOwnPtr receive_buffer) + : IPv4Socket(SOCK_STREAM, protocol, move(receive_buffer)) { m_last_retransmit_time = kgettimeofday(); } @@ -147,9 +150,9 @@ TCPSocket::~TCPSocket() dbgln_if(TCP_SOCKET_DEBUG, "~TCPSocket in state {}", to_string(state())); } -KResultOr> TCPSocket::create(int protocol) +KResultOr> TCPSocket::create(int protocol, NonnullOwnPtr receive_buffer) { - auto socket = adopt_ref_if_nonnull(new (nothrow) TCPSocket(protocol)); + auto socket = adopt_ref_if_nonnull(new (nothrow) TCPSocket(protocol, move(receive_buffer))); if (socket) return socket.release_nonnull(); return ENOMEM; diff --git a/Kernel/Net/TCPSocket.h b/Kernel/Net/TCPSocket.h index 83a74abdb6..73b076b924 100644 --- a/Kernel/Net/TCPSocket.h +++ b/Kernel/Net/TCPSocket.h @@ -18,7 +18,7 @@ namespace Kernel { class TCPSocket final : public IPv4Socket { public: static void for_each(Function); - static KResultOr> create(int protocol); + static KResultOr> create(int protocol, NonnullOwnPtr receive_buffer); virtual ~TCPSocket() override; enum class Direction { @@ -165,7 +165,7 @@ protected: void set_direction(Direction direction) { m_direction = direction; } private: - explicit TCPSocket(int protocol); + explicit TCPSocket(int protocol, NonnullOwnPtr receive_buffer); virtual StringView class_name() const override { return "TCPSocket"; } virtual void shut_down_for_writing() override; diff --git a/Kernel/Net/UDPSocket.cpp b/Kernel/Net/UDPSocket.cpp index edd7fb83ce..6f4d3006e9 100644 --- a/Kernel/Net/UDPSocket.cpp +++ b/Kernel/Net/UDPSocket.cpp @@ -43,8 +43,8 @@ SocketHandle UDPSocket::from_port(u16 port) return { *socket }; } -UDPSocket::UDPSocket(int protocol) - : IPv4Socket(SOCK_DGRAM, protocol) +UDPSocket::UDPSocket(int protocol, NonnullOwnPtr receive_buffer) + : IPv4Socket(SOCK_DGRAM, protocol, move(receive_buffer)) { } @@ -54,9 +54,9 @@ UDPSocket::~UDPSocket() sockets_by_port().resource().remove(local_port()); } -KResultOr> UDPSocket::create(int protocol) +KResultOr> UDPSocket::create(int protocol, NonnullOwnPtr receive_buffer) { - auto socket = adopt_ref_if_nonnull(new (nothrow) UDPSocket(protocol)); + auto socket = adopt_ref_if_nonnull(new (nothrow) UDPSocket(protocol, move(receive_buffer))); if (socket) return socket.release_nonnull(); return ENOMEM; diff --git a/Kernel/Net/UDPSocket.h b/Kernel/Net/UDPSocket.h index 338682ce5f..ebef463a6e 100644 --- a/Kernel/Net/UDPSocket.h +++ b/Kernel/Net/UDPSocket.h @@ -13,14 +13,14 @@ namespace Kernel { class UDPSocket final : public IPv4Socket { public: - static KResultOr> create(int protocol); + static KResultOr> create(int protocol, NonnullOwnPtr receive_buffer); virtual ~UDPSocket() override; static SocketHandle from_port(u16); static void for_each(Function); private: - explicit UDPSocket(int protocol); + explicit UDPSocket(int protocol, NonnullOwnPtr receive_buffer); virtual StringView class_name() const override { return "UDPSocket"; } static Lockable>& sockets_by_port();