From a6aee0c097ed244582ef7e1ed4d45a453ddb6302 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 21 Oct 2020 20:51:02 +0200 Subject: [PATCH] IPv4: Take the socket lock more (fixes TCP connection to localhost) This fixes an issue where making a TCP connection to localhost didn't work correctly since the loopback interface is currently synchronous. (Sending something to localhost would enqueue a packet on the same interface and then immediately wake the network task to process that packet.) This was preventing the TCP handshake from working correctly with localhost since we'd send out the SYN packet before moving to the SynSent state. The lock is now held long enough for this operation to be atomic. --- Kernel/Net/IPv4Socket.cpp | 3 +++ Kernel/Net/NetworkTask.cpp | 3 +++ Kernel/Net/Socket.cpp | 1 + Kernel/Net/TCPSocket.cpp | 5 +++++ 4 files changed, 12 insertions(+) diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index 459a9f2477..98d400456e 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -131,6 +131,7 @@ KResult IPv4Socket::bind(Userspace user_address, socklen_t addr KResult IPv4Socket::listen(size_t backlog) { + LOCKER(lock()); int rc = allocate_local_port_if_needed(); if (rc < 0) return KResult(-EADDRINUSE); @@ -203,6 +204,8 @@ int IPv4Socket::allocate_local_port_if_needed() KResultOr IPv4Socket::sendto(FileDescription&, const UserOrKernelBuffer& data, size_t data_length, int flags, Userspace addr, socklen_t addr_length) { + LOCKER(lock()); + (void)flags; if (addr && addr_length != sizeof(sockaddr_in)) return KResult(-EINVAL); diff --git a/Kernel/Net/NetworkTask.cpp b/Kernel/Net/NetworkTask.cpp index 69fb48f5ec..93fe9e6e1d 100644 --- a/Kernel/Net/NetworkTask.cpp +++ b/Kernel/Net/NetworkTask.cpp @@ -369,6 +369,8 @@ void handle_tcp(const IPv4Packet& ipv4_packet, const timeval& packet_timestamp) return; } + LOCKER(socket->lock()); + ASSERT(socket->type() == SOCK_STREAM); ASSERT(socket->local_port() == tcp_packet.destination_port()); @@ -401,6 +403,7 @@ void handle_tcp(const IPv4Packet& ipv4_packet, const timeval& packet_timestamp) klog() << "handle_tcp: couldn't create client socket"; return; } + LOCKER(client->lock()); #ifdef TCP_DEBUG klog() << "handle_tcp: created new client socket with tuple " << client->tuple().to_string().characters(); #endif diff --git a/Kernel/Net/Socket.cpp b/Kernel/Net/Socket.cpp index 0e197eeecf..96bfb8a42c 100644 --- a/Kernel/Net/Socket.cpp +++ b/Kernel/Net/Socket.cpp @@ -244,6 +244,7 @@ KResultOr Socket::write(FileDescription& description, size_t, const User KResult Socket::shutdown(int how) { + LOCKER(lock()); if (type() == SOCK_STREAM && !is_connected()) return KResult(-ENOTCONN); if (m_role == Role::Listener) diff --git a/Kernel/Net/TCPSocket.cpp b/Kernel/Net/TCPSocket.cpp index e14ecf2770..44e7f062f5 100644 --- a/Kernel/Net/TCPSocket.cpp +++ b/Kernel/Net/TCPSocket.cpp @@ -368,6 +368,8 @@ KResult TCPSocket::protocol_listen() KResult TCPSocket::protocol_connect(FileDescription& description, ShouldBlock should_block) { + Locker locker(lock()); + auto routing_decision = route_to(peer_address(), local_address()); if (routing_decision.is_zero()) return KResult(-EHOSTUNREACH); @@ -388,8 +390,10 @@ KResult TCPSocket::protocol_connect(FileDescription& description, ShouldBlock sh m_direction = Direction::Outgoing; if (should_block == ShouldBlock::Yes) { + locker.unlock(); if (Thread::current()->block(nullptr, description).was_interrupted()) return KResult(-EINTR); + locker.lock(); ASSERT(setup_state() == SetupState::Completed); if (has_error()) { m_role = Role::None; @@ -458,6 +462,7 @@ void TCPSocket::shut_down_for_writing() KResult TCPSocket::close() { + Locker socket_locker(lock()); auto result = IPv4Socket::close(); if (state() == State::CloseWait) { #ifdef TCP_SOCKET_DEBUG