From ff234849d6a41d2c2d29cf09408ac66e202d68e7 Mon Sep 17 00:00:00 2001 From: cirakg Date: Mon, 2 Nov 2026 16:23:40 +0100 Subject: ldn: wait for the host's reply when joining a network Connect slept for a second while holding packet_mutex, so the host's SyncNetwork reply could not be handled until Connect had already returned success in StationOpened. Players could see each other's group and enter it, but never actually met. Wait on a condition variable instead, accept the reply only from the selected host and session, and fail after five seconds. Check packet sizes and senders, stop duplicate joins from taking another station, and seed session IDs so separate networks don't share one. The SDL frontend never called Network::Init, so its multiplayer option could not join a room. Initialize and shut down networking around the core. Assisted-by: Sol (implementation) Assisted-by: Claude Opus 5.5 (code review) --- src/core/hle/service/ldn/lan_discovery.cpp | 85 +++++++++++++++++++++++++----- src/core/hle/service/ldn/lan_discovery.h | 8 ++- src/yuzu_cmd/yuzu.cpp | 11 ++++ 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/src/core/hle/service/ldn/lan_discovery.cpp b/src/core/hle/service/ldn/lan_discovery.cpp index 90adcdcee4..d7debdd67b 100644 --- a/src/core/hle/service/ldn/lan_discovery.cpp +++ b/src/core/hle/service/ldn/lan_discovery.cpp @@ -5,6 +5,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "core/hle/service/ldn/lan_discovery.h" +#include "common/random.h" #include "core/internal_network/network.h" #include "core/internal_network/network_interface.h" @@ -70,6 +71,7 @@ void LANDiscovery::InitNodeStateChange() { } State LANDiscovery::GetState() const { + std::scoped_lock lock{packet_mutex}; return state; } @@ -78,6 +80,7 @@ void LANDiscovery::SetState(State new_state) { } Result LANDiscovery::GetNetworkInfo(NetworkInfo& out_network) const { + std::scoped_lock lock{packet_mutex}; if (state == State::AccessPointCreated || state == State::StationConnected) { std::memcpy(&out_network, &network_info, sizeof(network_info)); return ResultSuccess; @@ -88,6 +91,7 @@ Result LANDiscovery::GetNetworkInfo(NetworkInfo& out_network) const { Result LANDiscovery::GetNetworkInfo(NetworkInfo& out_network, std::span out_updates) { + std::scoped_lock lock{packet_mutex}; if (out_updates.size() > NodeCountMax) { return ResultInvalidBufferCount; } @@ -105,6 +109,7 @@ Result LANDiscovery::GetNetworkInfo(NetworkInfo& out_network, } DisconnectReason LANDiscovery::GetDisconnectReason() const { + std::scoped_lock lock{packet_mutex}; return disconnect_reason; } @@ -194,7 +199,7 @@ Result LANDiscovery::CloseAccessPoint() { } if (state == State::AccessPointCreated) { - DestroyNetwork(); + DestroyNetworkImpl(); } ResetStations(); @@ -222,8 +227,8 @@ Result LANDiscovery::CloseStation() { return ResultBadState; } - if (state == State::StationConnected) { - Disconnect(); + if (state == State::StationConnected || host_ip) { + DisconnectImpl(); } ResetStations(); @@ -251,7 +256,8 @@ Result LANDiscovery::CreateNetwork(const SecurityConfig& security_config, network_info.common.channel = network_config.channel; } - std::independent_bits_engine bits_engine; + std::independent_bits_engine bits_engine{ + Common::Random::GetMT19937()}; network_info.network_id.session_id.high = bits_engine(); network_info.network_id.session_id.low = bits_engine(); network_info.network_id.intent_id = network_config.intent_id; @@ -272,6 +278,11 @@ Result LANDiscovery::CreateNetwork(const SecurityConfig& security_config, } Result LANDiscovery::DestroyNetwork() { + std::scoped_lock lock{packet_mutex}; + return DestroyNetworkImpl(); +} + +Result LANDiscovery::DestroyNetworkImpl() { for (auto local_ip : connected_clients) { SendPacket(Network::LDNPacketType::DestroyNetwork, local_ip); } @@ -286,7 +297,10 @@ Result LANDiscovery::DestroyNetwork() { Result LANDiscovery::Connect(const NetworkInfo& network_info_, const UserConfig& user_config, u16 local_communication_version) { - std::scoped_lock lock{packet_mutex}; + std::unique_lock lock{packet_mutex}; + if (state != State::StationOpened || host_ip) { + return ResultBadState; + } if (network_info_.ldn.node_count == 0) { return ResultInvalidNodeCount; } @@ -299,21 +313,39 @@ Result LANDiscovery::Connect(const NetworkInfo& network_info_, const UserConfig& Ipv4Address node_host = network_info_.ldn.nodes[0].ipv4_address; std::reverse(std::begin(node_host), std::end(node_host)); // htonl host_ip = node_host; - SendPacket(Network::LDNPacketType::Connect, node_info, *host_ip); - + network_info = network_info_; InitNodeStateChange(); + SendPacket(Network::LDNPacketType::Connect, node_info, *host_ip); - std::this_thread::sleep_for(std::chrono::seconds(1)); + // The room callback needs packet_mutex to install the host's connection reply. + const bool replied = connection_event.wait_for(lock, connection_timeout, [this] { + return state != State::StationOpened || !host_ip; + }); + if (!replied) { + SendPacket(Network::LDNPacketType::Disconnect, node_info, *host_ip); + host_ip.reset(); + return ResultAuthenticationTimeout; + } + if (state != State::StationConnected) { + return ResultConnectionFailed; + } return ResultSuccess; } Result LANDiscovery::Disconnect() { + std::scoped_lock lock{packet_mutex}; + return DisconnectImpl(); +} + +Result LANDiscovery::DisconnectImpl() { if (host_ip) { SendPacket(Network::LDNPacketType::Disconnect, node_info, *host_ip); } + host_ip.reset(); SetState(State::StationOpened); + connection_event.notify_all(); lan_event(); return ResultSuccess; @@ -346,10 +378,10 @@ Result LANDiscovery::Finalize() { if (inited) { if (state == State::AccessPointCreated) { - DestroyNetwork(); + DestroyNetworkImpl(); } - if (state == State::StationConnected) { - Disconnect(); + if (state == State::StationConnected || host_ip) { + DisconnectImpl(); } ResetStations(); @@ -392,6 +424,7 @@ void LANDiscovery::OnSyncNetwork(const NetworkInfo& info) { SetState(State::StationConnected); } OnNetworkInfoChanged(); + connection_event.notify_all(); } void LANDiscovery::OnDisconnectFromHost() { @@ -401,6 +434,7 @@ void LANDiscovery::OnDisconnectFromHost() { SetState(State::StationOpened); lan_event(); } + connection_event.notify_all(); } void LANDiscovery::OnNetworkInfoChanged() { @@ -489,6 +523,9 @@ void LANDiscovery::ReceivePacket(const Network::LDNPacket& packet) { break; } case Network::LDNPacketType::ScanResp: { + if (packet.data.size() != sizeof(NetworkInfo)) { + break; + } LOG_INFO(Frontend, "ScanResp packet received!"); NetworkInfo info{}; @@ -498,6 +535,17 @@ void LANDiscovery::ReceivePacket(const Network::LDNPacket& packet) { break; } case Network::LDNPacketType::Connect: { + if (state != State::AccessPointCreated || packet.data.size() != sizeof(NodeInfo)) { + break; + } + if (std::find(connected_clients.begin(), connected_clients.end(), packet.local_ip) != + connected_clients.end()) { + SendPacket(Network::LDNPacketType::SyncNetwork, network_info, packet.local_ip); + break; + } + if (network_info.ldn.node_count >= network_info.ldn.node_count_max) { + break; + } LOG_INFO(Frontend, "Connect packet received!"); NodeInfo info{}; @@ -518,6 +566,9 @@ void LANDiscovery::ReceivePacket(const Network::LDNPacket& packet) { break; } case Network::LDNPacketType::Disconnect: { + if (state != State::AccessPointCreated || packet.data.size() != sizeof(NodeInfo)) { + break; + } LOG_INFO(Frontend, "Disconnect packet received!"); connected_clients.erase( @@ -538,17 +589,23 @@ void LANDiscovery::ReceivePacket(const Network::LDNPacket& packet) { break; } case Network::LDNPacketType::DestroyNetwork: { + if (!host_ip || packet.local_ip != *host_ip) { + break; + } ResetStations(); OnDisconnectFromHost(); break; } case Network::LDNPacketType::SyncNetwork: { - if (state == State::StationOpened || state == State::StationConnected) { + if (host_ip && packet.local_ip == *host_ip && + packet.data.size() == sizeof(NetworkInfo) && + (state == State::StationOpened || state == State::StationConnected)) { LOG_INFO(Frontend, "SyncNetwork packet received!"); NetworkInfo info{}; std::memcpy(&info, packet.data.data(), sizeof(NetworkInfo)); - - OnSyncNetwork(info); + if (info.network_id.session_id == network_info.network_id.session_id) { + OnSyncNetwork(info); + } } else { LOG_INFO(Frontend, "SyncNetwork packet received but in wrong State!"); } diff --git a/src/core/hle/service/ldn/lan_discovery.h b/src/core/hle/service/ldn/lan_discovery.h index f1f3c4bece..e641b11409 100644 --- a/src/core/hle/service/ldn/lan_discovery.h +++ b/src/core/hle/service/ldn/lan_discovery.h @@ -10,6 +10,8 @@ #pragma once #include +#include +#include #include #include #include @@ -92,6 +94,8 @@ protected: void InitNodeStateChange(); void ResetStations(); + Result DestroyNetworkImpl(); + Result DisconnectImpl(); void UpdateNodes(); void OnSyncNetwork(const NetworkInfo& info); @@ -118,7 +122,9 @@ protected: static constexpr Ssid fake_ssid{"YuzuFakeSsidForLdn"}; bool inited{}; - std::mutex packet_mutex; + mutable std::mutex packet_mutex; + std::condition_variable connection_event; + std::chrono::nanoseconds connection_timeout{std::chrono::seconds(5)}; std::array stations; std::array node_changes{}; std::array node_last_states{}; diff --git a/src/yuzu_cmd/yuzu.cpp b/src/yuzu_cmd/yuzu.cpp index bc4002d8af..6342c5d353 100644 --- a/src/yuzu_cmd/yuzu.cpp +++ b/src/yuzu_cmd/yuzu.cpp @@ -249,6 +249,7 @@ struct SdlState { // Must outlive emu_window, which keeps a pointer to it. InputCommon::InputSubsystem input_subsystem{}; std::unique_ptr emu_window; + bool network_initialized{}; }; static std::atomic_bool pack_nup_cancel{false}; @@ -353,6 +354,12 @@ extern "C" SDL_AppResult SDL_AppInit(void **appstate, int argc, char **argv) { return SDL_APP_FAILURE; } + state->network_initialized = Network::Init(); + if (!state->network_initialized) { + LOG_CRITICAL(Network, "Failed to initialize room networking"); + return SDL_APP_FAILURE; + } + state->system.Initialize(); @@ -555,7 +562,11 @@ extern "C" void SDL_AppQuit(void *appstate, SDL_AppResult result) { void(state->system.Pause()); state->system.ShutdownMainProcess(); } + const bool network_initialized = state->network_initialized; delete state; + if (network_initialized) { + Network::Shutdown(); + } } #define VMA_IMPLEMENTATION -- cgit v1.2.3 From e5a2a13175378f2ed8c900432712d4d838dfe8bf Mon Sep 17 00:00:00 2001 From: cirakg Date: Mon, 2 Nov 2026 21:14:42 +0100 Subject: network: deliver room datagrams through ProxySocket::Recv When connected to an emulator room every guest socket is a ProxySocket. Its Recv was a stub that always returned zero bytes, so games that read UDP replies with recv() never received them. Mario Kart World LAN Play reads its discovery replies this way, so guests never saw the host's room in the LAN browser. Delegate Recv to RecvFrom without an address, so queued room packets, non-blocking, timeout, peek and truncation behaviour match recvfrom(). Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/internal_network/socket_proxy.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index 744765bfcc..d3ae33d065 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -102,11 +102,7 @@ Errno ProxySocket::Shutdown(ShutdownHow how) { } std::pair ProxySocket::Recv(int flags, std::span message) { - LOG_WARNING(Network, "(STUBBED) called"); - ASSERT(flags == 0); - ASSERT(message.size() < static_cast((std::numeric_limits::max)())); - - return {static_cast(0), Errno::SUCCESS}; + return RecvFrom(flags, message, nullptr); } std::pair ProxySocket::RecvFrom(int flags, std::span message, SockAddrIn* addr) { -- cgit v1.2.3 From 8aafca259892d2f3d573ae4890470bdb7cfc62a8 Mon Sep 17 00:00:00 2001 From: cirakg Date: Mon, 2 Nov 2026 21:36:20 +0100 Subject: network: fix ProxySocket receive edge cases Routing recv() through RecvFrom made several old RecvFrom problems reachable from the normal read path: - A partial TCP read kept zero bytes in front of the unread data. - A blocking read busy-waited with yield() and reported ETIMEDOUT. - MSG_PEEK hit the flags == 0 assert. - A UDP datagram larger than the buffer was dropped with EMSGSIZE. Wait on a condition variable signalled when a packet arrives, and report an expired wait as EAGAIN like an SO_RCVTIMEO timeout. Keep the existing 5s cap for sockets without a receive timeout. Allow MSG_PEEK. Truncate oversized datagrams as BSD recv() does, and keep the unread tail of a TCP packet for the next read. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/internal_network/socket_proxy.cpp | 83 ++++++++++++------------------ src/core/internal_network/socket_proxy.h | 4 +- 2 files changed, 36 insertions(+), 51 deletions(-) diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index d3ae33d065..f42ed61e21 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -3,8 +3,9 @@ // SPDX-FileCopyrightText: Copyright 2022 yuzu Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later +#include #include -#include +#include #include "common/assert.h" #include "common/logging.h" @@ -43,8 +44,11 @@ void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { auto decompressed = packet; decompressed.data = Common::Compression::DecompressDataZSTD(packet.data); - std::lock_guard guard(packets_mutex); - received_packets.push(decompressed); + { + std::lock_guard guard(packets_mutex); + received_packets.push(std::move(decompressed)); + } + packets_cv.notify_one(); } template @@ -106,37 +110,26 @@ std::pair ProxySocket::Recv(int flags, std::span message) { } std::pair ProxySocket::RecvFrom(int flags, std::span message, SockAddrIn* addr) { - ASSERT(flags == 0); + ASSERT((flags & ~FLAG_MSG_PEEK) == 0); ASSERT(message.size() < static_cast((std::numeric_limits::max)())); - // TODO (flTobi): Verify the timeout behavior and break when connection is lost - const auto timestamp = std::chrono::steady_clock::now(); - // When receive_timeout is set to zero, the socket is supposed to wait indefinitely until a - // packet arrives. In order to prevent lost packets from hanging the emulation thread, we set - // the timeout to 5s instead - const auto timeout = receive_timeout == 0 ? 5000 : receive_timeout; - while (true) { - { - std::lock_guard guard(packets_mutex); - if (received_packets.size() > 0) { - return ReceivePacket(flags, message, addr, message.size()); - } - } - + std::unique_lock lock(packets_mutex); + if (received_packets.empty()) { if (!blocking) { return {-1, Errno::AGAIN}; } - - std::this_thread::yield(); - - const auto time_diff = std::chrono::steady_clock::now() - timestamp; - const auto time_diff_ms = - std::chrono::duration_cast(time_diff).count(); - - if (time_diff_ms > timeout) { - return {-1, Errno::TIMEDOUT}; + // TODO (flTobi): Verify the timeout behavior and break when connection is lost + // When receive_timeout is set to zero, the socket is supposed to wait indefinitely until a + // packet arrives. In order to prevent lost packets from hanging the emulation thread, we set + // the timeout to 5s instead. An expired wait is reported like an SO_RCVTIMEO timeout. + const u32 timeout = receive_timeout == 0 ? 5000 : receive_timeout; + if (!packets_cv.wait_for(lock, std::chrono::milliseconds{timeout}, + [this] { return !received_packets.empty(); })) { + return {-1, Errno::AGAIN}; } } + + return ReceivePacket(flags, message, addr, message.size()); } std::pair ProxySocket::ReceivePacket(int flags, std::span message, SockAddrIn* addr, @@ -148,32 +141,22 @@ std::pair ProxySocket::ReceivePacket(int flags, std::span messag addr->portno = packet.local_endpoint.portno; // The senders port number } - bool peek = (flags & FLAG_MSG_PEEK) != 0; - std::size_t read_bytes; - if (packet.data.size() > max_length) { - read_bytes = max_length; - memcpy(message.data(), packet.data.data(), max_length); - - if (protocol == Protocol::UDP) { - if (!peek) { - received_packets.pop(); - } - return {-1, Errno::MSGSIZE}; - } else if (protocol == Protocol::TCP) { - std::vector numArray(packet.data.size() - max_length); - std::copy(packet.data.begin() + max_length, packet.data.end(), - std::back_inserter(numArray)); - packet.data = numArray; - } + const std::size_t read_bytes = (std::min)(packet.data.size(), max_length); + std::memcpy(message.data(), packet.data.data(), read_bytes); + + if ((flags & FLAG_MSG_PEEK) != 0) { + return {static_cast(read_bytes), Errno::SUCCESS}; + } + + if (protocol == Protocol::TCP && read_bytes < packet.data.size()) { + // Keep the unread part of the stream for the next call + packet.data.erase(packet.data.begin(), packet.data.begin() + read_bytes); } else { - read_bytes = packet.data.size(); - memcpy(message.data(), packet.data.data(), read_bytes); - if (!peek) { - received_packets.pop(); - } + // Like BSD recv(), the rest of a datagram that does not fit is discarded + received_packets.pop(); } - return {static_cast(read_bytes), Errno::SUCCESS}; + return {static_cast(read_bytes), Errno::SUCCESS}; } std::pair ProxySocket::Send(std::span message, int flags) { diff --git a/src/core/internal_network/socket_proxy.h b/src/core/internal_network/socket_proxy.h index caf5d1ee23..8343fcbae1 100644 --- a/src/core/internal_network/socket_proxy.h +++ b/src/core/internal_network/socket_proxy.h @@ -1,10 +1,11 @@ -// SPDX-FileCopyrightText: Copyright 2025 Eden Emulator Project +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project // SPDX-License-Identifier: GPL-3.0-or-later // SPDX-FileCopyrightText: Copyright 2022 yuzu Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include #include #include #include @@ -92,6 +93,7 @@ private: Protocol protocol; std::mutex packets_mutex; + std::condition_variable packets_cv; }; } // namespace Network -- cgit v1.2.3 From 693529b27ec898b867ad5806a0174f69597decc9 Mon Sep 17 00:00:00 2001 From: cirakg Date: Mon, 2 Nov 2026 22:52:39 +0100 Subject: nifm: treat a joined room as the LAN for local network games LAN Play through public lobby rooms failed in two ways. With airplane mode on (the default), IRequest::GetResult rejected the game's network request. Mario Kart World then opened the net-connect applet and returned to the main menu instead of the LAN browser. A joined room is the emulated network: its sockets only reach room members. Count it as an available network for GetResult and IsAnyInternetRequestAccepted. With airplane mode off, players could open the browser but never saw each other. Room fake IPs are spread over 192.168.0.0/16, and some room servers give members different /24s. NIFM reported the host adapter's /24 mask with the fake IP, so the games treated each other as off-subnet. Report 255.255.0.0 in a room instead. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/hle/service/nifm/nifm.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/core/hle/service/nifm/nifm.cpp b/src/core/hle/service/nifm/nifm.cpp index efada9b807..153b0f2193 100644 --- a/src/core/hle/service/nifm/nifm.cpp +++ b/src/core/hle/service/nifm/nifm.cpp @@ -70,6 +70,20 @@ static u128 MakeUuidFromName(std::string_view name) { return {h1, h2}; } +// A joined room is the emulated network, even in airplane mode. Its sockets only reach room +// members, and LAN games (e.g. Mario Kart World LAN Play) need their connection request accepted. +static bool IsNetworkAvailable() { + if (const auto room_member = Network::GetRoomMember().lock(); + room_member && room_member->GetState() == Network::RoomMember::State::Joined) { + return true; + } + return Network::GetHostIPv4Address().has_value() && !Settings::values.airplane_mode.GetValue(); +} + +// Room fake IPs are spread over 192.168.0.0/16, and some room servers give members different /24s. +// A host /24 mask would then put room members on different subnets, so LAN games ignore each other. +constexpr Network::IPv4Address RoomSubnetMask{255, 255, 0, 0}; + // This is nn::nifm::RequestState enum class RequestState : u32 { NotSubmitted = 1, @@ -430,8 +444,7 @@ private: LOG_DEBUG(Service_NIFM, "(STUBBED) called"); const auto result = [this] { - const auto has_connection = Network::GetHostIPv4Address().has_value() && - !Settings::values.airplane_mode.GetValue(); + const auto has_connection = IsNetworkAvailable(); switch (state) { case RequestState::NotSubmitted: return has_connection ? ResultSuccess : ResultNetworkCommunicationDisabled; @@ -575,6 +588,7 @@ void IGeneralService::GetCurrentNetworkProfile(HLERequestContext& ctx) { if (room_member->IsConnected()) { profile.ip_setting_data.ip_address_setting.ip_address = room_member->GetFakeIpAddress(); + profile.ip_setting_data.ip_address_setting.subnet_mask = RoomSubnetMask; } profile.ip_setting_data.dns_setting.secondary_dns = {8, 8, 8, 8}; } @@ -714,6 +728,7 @@ void IGeneralService::GetNetworkProfile(HLERequestContext& ctx) { if (auto room_member = Network::GetRoomMember().lock()) { if (room_member->IsConnected()) { profile.ip_setting_data.ip_address_setting.ip_address = room_member->GetFakeIpAddress(); + profile.ip_setting_data.ip_address_setting.subnet_mask = RoomSubnetMask; } } @@ -904,6 +919,7 @@ void IGeneralService::GetCurrentIpConfigInfo(HLERequestContext& ctx) { if (auto room_member = Network::GetRoomMember().lock()) { if (room_member->IsConnected()) { info.ip.ip_address = room_member->GetFakeIpAddress(); + info.ip.subnet_mask = RoomSubnetMask; } info.dns.is_automatic = true; info.dns.primary_dns = {1, 1, 1, 1}; @@ -984,7 +1000,7 @@ void IGeneralService::IsAnyInternetRequestAccepted(HLERequestContext& ctx) { IPC::ResponseBuilder rb{ctx, 3}; rb.Push(ResultSuccess); - if (Network::GetHostIPv4Address().has_value() && !Settings::values.airplane_mode.GetValue()) { + if (IsNetworkAvailable()) { rb.Push(1); } else { rb.Push(0); -- cgit v1.2.3 From 3e10d48548a285172ea7cb1f94c58783e1d97754 Mon Sep 17 00:00:00 2001 From: cirakg Date: Mon, 2 Nov 2026 23:40:30 +0100 Subject: network: fix room host LAN Play and room socket review issues The room host and room moderators are in the Moderator state, not Joined, so the airplane mode override for joined rooms did not apply to them and LAN Play still returned them to the menu. Accept both states. Share the room address override between the NIFM profile and IP config calls so they cannot drift apart. recv() on a room TCP socket now waited for the receive timeout, because Recv goes through RecvFrom. Room TCP sockets have no peer, so keep the previous immediate end-of-stream result when nothing is queued. Close() now drops queued packets and wakes a blocked recv, which then fails with EBADF. The socket state that the room thread reads when it queues a packet is now guarded by the packet mutex. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/hle/service/nifm/nifm.cpp | 41 ++++++++++---------- src/core/internal_network/socket_proxy.cpp | 62 +++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/src/core/hle/service/nifm/nifm.cpp b/src/core/hle/service/nifm/nifm.cpp index 153b0f2193..e0a23f7a63 100644 --- a/src/core/hle/service/nifm/nifm.cpp +++ b/src/core/hle/service/nifm/nifm.cpp @@ -73,17 +73,17 @@ static u128 MakeUuidFromName(std::string_view name) { // A joined room is the emulated network, even in airplane mode. Its sockets only reach room // members, and LAN games (e.g. Mario Kart World LAN Play) need their connection request accepted. static bool IsNetworkAvailable() { - if (const auto room_member = Network::GetRoomMember().lock(); - room_member && room_member->GetState() == Network::RoomMember::State::Joined) { - return true; + if (const auto room_member = Network::GetRoomMember().lock()) { + // The room host and moderators are in the Moderator state instead of Joined + const auto state = room_member->GetState(); + if (state == Network::RoomMember::State::Joined || + state == Network::RoomMember::State::Moderator) { + return true; + } } return Network::GetHostIPv4Address().has_value() && !Settings::values.airplane_mode.GetValue(); } -// Room fake IPs are spread over 192.168.0.0/16, and some room servers give members different /24s. -// A host /24 mask would then put room members on different subnets, so LAN games ignore each other. -constexpr Network::IPv4Address RoomSubnetMask{255, 255, 0, 0}; - // This is nn::nifm::RequestState enum class RequestState : u32 { NotSubmitted = 1, @@ -124,6 +124,17 @@ struct IpAddressSetting { }; static_assert(sizeof(IpAddressSetting) == 0xD, "IpAddressSetting has incorrect size."); +// Room fake IPs are spread over 192.168.0.0/16, and some room servers give members different /24s. +// A host /24 mask would then put room members on different subnets, so LAN games ignore each other. +constexpr Network::IPv4Address RoomSubnetMask{255, 255, 0, 0}; + +static void ApplyRoomAddress(const Network::RoomMember& room_member, IpAddressSetting& setting) { + if (room_member.IsConnected()) { + setting.ip_address = room_member.GetFakeIpAddress(); + setting.subnet_mask = RoomSubnetMask; + } +} + // This is nn::nifm::DnsSetting struct DnsSetting { bool is_automatic{}; @@ -585,11 +596,7 @@ void IGeneralService::GetCurrentNetworkProfile(HLERequestContext& ctx) { std::strncpy(profile.network_name.data(), st.ssid, sizeof(profile.network_name) - 1); if (auto room_member = Network::GetRoomMember().lock()) { - if (room_member->IsConnected()) { - profile.ip_setting_data.ip_address_setting.ip_address = - room_member->GetFakeIpAddress(); - profile.ip_setting_data.ip_address_setting.subnet_mask = RoomSubnetMask; - } + ApplyRoomAddress(*room_member, profile.ip_setting_data.ip_address_setting); profile.ip_setting_data.dns_setting.secondary_dns = {8, 8, 8, 8}; } @@ -726,10 +733,7 @@ void IGeneralService::GetNetworkProfile(HLERequestContext& ctx) { }(); if (auto room_member = Network::GetRoomMember().lock()) { - if (room_member->IsConnected()) { - profile.ip_setting_data.ip_address_setting.ip_address = room_member->GetFakeIpAddress(); - profile.ip_setting_data.ip_address_setting.subnet_mask = RoomSubnetMask; - } + ApplyRoomAddress(*room_member, profile.ip_setting_data.ip_address_setting); } ctx.WriteBuffer(profile); @@ -917,10 +921,7 @@ void IGeneralService::GetCurrentIpConfigInfo(HLERequestContext& ctx) { } if (auto room_member = Network::GetRoomMember().lock()) { - if (room_member->IsConnected()) { - info.ip.ip_address = room_member->GetFakeIpAddress(); - info.ip.subnet_mask = RoomSubnetMask; - } + ApplyRoomAddress(*room_member, info.ip); info.dns.is_automatic = true; info.dns.primary_dns = {1, 1, 1, 1}; info.dns.secondary_dns = {8, 8, 8, 8}; diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index f42ed61e21..83eb2ce9d0 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -31,21 +31,21 @@ ProxySocket::~ProxySocket() { } void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { - if (protocol != packet.protocol || local_endpoint.portno != packet.remote_endpoint.portno || - closed) { - return; - } - - if (!broadcast && packet.broadcast) { - LOG_INFO(Network, "Received broadcast packet, but not configured for broadcast mode"); - return; - } - - auto decompressed = packet; - decompressed.data = Common::Compression::DecompressDataZSTD(packet.data); - { + // Runs on the room thread, while the game thread may bind, close or reconfigure the socket std::lock_guard guard(packets_mutex); + if (protocol != packet.protocol || local_endpoint.portno != packet.remote_endpoint.portno || + closed) { + return; + } + + if (!broadcast && packet.broadcast) { + LOG_INFO(Network, "Received broadcast packet, but not configured for broadcast mode"); + return; + } + + auto decompressed = packet; + decompressed.data = Common::Compression::DecompressDataZSTD(packet.data); received_packets.push(std::move(decompressed)); } packets_cv.notify_one(); @@ -58,7 +58,10 @@ Errno ProxySocket::SetSockOpt(SOCKET fd_, int option, T value) { } Errno ProxySocket::Initialize(Domain domain, Type type, Protocol socket_protocol) { - protocol = socket_protocol; + { + std::lock_guard guard(packets_mutex); + protocol = socket_protocol; + } SetSockOpt(fd, SO_TYPE, type); return Errno::SUCCESS; @@ -85,6 +88,7 @@ std::pair ProxySocket::GetSockName() { } Errno ProxySocket::Bind(SockAddrIn addr) { + std::lock_guard guard(packets_mutex); if (is_bound) { LOG_WARNING(Network, "Rebinding Socket is unimplemented!"); return Errno::SUCCESS; @@ -106,6 +110,14 @@ Errno ProxySocket::Shutdown(ShutdownHow how) { } std::pair ProxySocket::Recv(int flags, std::span message) { + if (protocol == Protocol::TCP) { + // Room TCP sockets have no peer (Connect and Send are stubs). Report an empty queue as the + // end of the stream, as before, instead of blocking the bsd service until the timeout. + std::lock_guard guard(packets_mutex); + if (received_packets.empty()) { + return {0, Errno::SUCCESS}; + } + } return RecvFrom(flags, message, nullptr); } @@ -114,6 +126,9 @@ std::pair ProxySocket::RecvFrom(int flags, std::span message, So ASSERT(message.size() < static_cast((std::numeric_limits::max)())); std::unique_lock lock(packets_mutex); + if (closed) { + return {-1, Errno::BADF}; + } if (received_packets.empty()) { if (!blocking) { return {-1, Errno::AGAIN}; @@ -124,9 +139,12 @@ std::pair ProxySocket::RecvFrom(int flags, std::span message, So // the timeout to 5s instead. An expired wait is reported like an SO_RCVTIMEO timeout. const u32 timeout = receive_timeout == 0 ? 5000 : receive_timeout; if (!packets_cv.wait_for(lock, std::chrono::milliseconds{timeout}, - [this] { return !received_packets.empty(); })) { + [this] { return closed || !received_packets.empty(); })) { return {-1, Errno::AGAIN}; } + if (closed) { + return {-1, Errno::BADF}; + } } return ReceivePacket(flags, message, addr, message.size()); @@ -217,8 +235,13 @@ std::pair ProxySocket::SendTo(u32 flags, std::span message } Errno ProxySocket::Close() { - fd = INVALID_SOCKET; - closed = true; + { + std::lock_guard guard(packets_mutex); + fd = INVALID_SOCKET; + closed = true; + received_packets = {}; + } + packets_cv.notify_all(); return Errno::SUCCESS; } @@ -239,7 +262,10 @@ Errno ProxySocket::SetReuseAddr(bool enable) { } Errno ProxySocket::SetBroadcast(bool enable) { - broadcast = enable; + { + std::lock_guard guard(packets_mutex); + broadcast = enable; + } return SetSockOpt(fd, SO_BROADCAST, enable ? 1 : 0); } -- cgit v1.2.3 From fd3e2fe18a101e2bbfd5935263183e25ab26497b Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 00:17:50 +0100 Subject: network: fix room socket races and share the room network check bsd:s, bsd:u and bsd:a share one file descriptor table, but each service bound its own proxy packet callback, so every room packet was queued three times on each socket. Bind the callback once. The room thread walked the descriptor table without a lock while bsd created and closed sockets, so it could reach a destroyed or not yet created socket. Guard the table with a mutex, and publish a socket only after it is created and initialized. Check airplane mode before an fd is taken, so a refused socket no longer leaks its slot. ProxySocket now initializes protocol, decompresses packets outside the packet lock without copying the compressed data first, and keeps a read offset for partial TCP reads instead of erasing from the front. Add Network::GetRoomIPv4Address(), set only in the Joined and Moderator states. NIFM uses it for network availability and for every reported address, including EnumerateNetworkInterfaces and GetCurrentIpAddress, so no call reports the room address while another rejects the network. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/hle/service/nifm/nifm.cpp | 37 ++++++++-------- src/core/hle/service/sockets/bsd.cpp | 71 +++++++++++++++++++----------- src/core/hle/service/sockets/bsd.h | 12 +++-- src/core/internal_network/network.cpp | 11 +++++ src/core/internal_network/network.h | 4 ++ src/core/internal_network/socket_proxy.cpp | 29 +++++++++--- src/core/internal_network/socket_proxy.h | 3 +- 7 files changed, 111 insertions(+), 56 deletions(-) diff --git a/src/core/hle/service/nifm/nifm.cpp b/src/core/hle/service/nifm/nifm.cpp index e0a23f7a63..8714597f89 100644 --- a/src/core/hle/service/nifm/nifm.cpp +++ b/src/core/hle/service/nifm/nifm.cpp @@ -73,13 +73,8 @@ static u128 MakeUuidFromName(std::string_view name) { // A joined room is the emulated network, even in airplane mode. Its sockets only reach room // members, and LAN games (e.g. Mario Kart World LAN Play) need their connection request accepted. static bool IsNetworkAvailable() { - if (const auto room_member = Network::GetRoomMember().lock()) { - // The room host and moderators are in the Moderator state instead of Joined - const auto state = room_member->GetState(); - if (state == Network::RoomMember::State::Joined || - state == Network::RoomMember::State::Moderator) { - return true; - } + if (Network::GetRoomIPv4Address()) { + return true; } return Network::GetHostIPv4Address().has_value() && !Settings::values.airplane_mode.GetValue(); } @@ -128,9 +123,9 @@ static_assert(sizeof(IpAddressSetting) == 0xD, "IpAddressSetting has incorrect s // A host /24 mask would then put room members on different subnets, so LAN games ignore each other. constexpr Network::IPv4Address RoomSubnetMask{255, 255, 0, 0}; -static void ApplyRoomAddress(const Network::RoomMember& room_member, IpAddressSetting& setting) { - if (room_member.IsConnected()) { - setting.ip_address = room_member.GetFakeIpAddress(); +static void ApplyRoomAddress(IpAddressSetting& setting) { + if (const auto room_ip = Network::GetRoomIPv4Address()) { + setting.ip_address = *room_ip; setting.subnet_mask = RoomSubnetMask; } } @@ -596,7 +591,7 @@ void IGeneralService::GetCurrentNetworkProfile(HLERequestContext& ctx) { std::strncpy(profile.network_name.data(), st.ssid, sizeof(profile.network_name) - 1); if (auto room_member = Network::GetRoomMember().lock()) { - ApplyRoomAddress(*room_member, profile.ip_setting_data.ip_address_setting); + ApplyRoomAddress(profile.ip_setting_data.ip_address_setting); profile.ip_setting_data.dns_setting.secondary_dns = {8, 8, 8, 8}; } @@ -618,6 +613,8 @@ void IGeneralService::EnumerateNetworkInterfaces(HLERequestContext& ctx) { using Network::HostAdapterKind; const auto adapters = Network::GetAvailableNetworkInterfaces(); + const auto selected = Network::GetSelectedNetworkInterface(); + const auto room_ip = Network::GetRoomIPv4Address(); constexpr size_t kEntry = 0x3F0; std::vector blob(adapters.size() * kEntry, 0); @@ -633,6 +630,12 @@ void IGeneralService::EnumerateNetworkInterfaces(HLERequestContext& ctx) { *reinterpret_cast(base + 0x1C) = host.subnet_mask; *reinterpret_cast(base + 0x20) = host.gateway; + // The room traffic goes through the selected adapter, so report it like the other calls + if (room_ip && selected && host.name == selected->name) { + std::memcpy(base + 0x18, room_ip->data(), room_ip->size()); + std::memcpy(base + 0x1C, RoomSubnetMask.data(), RoomSubnetMask.size()); + } + std::string name_utf8 = host.name; name_utf8.resize(0x110, '\0'); std::memcpy(base + 0x2E0, name_utf8.data(), 0x110); @@ -732,9 +735,7 @@ void IGeneralService::GetNetworkProfile(HLERequestContext& ctx) { .wireless_setting_data{wifi}}; }(); - if (auto room_member = Network::GetRoomMember().lock()) { - ApplyRoomAddress(*room_member, profile.ip_setting_data.ip_address_setting); - } + ApplyRoomAddress(profile.ip_setting_data.ip_address_setting); ctx.WriteBuffer(profile); @@ -871,10 +872,8 @@ void IGeneralService::GetCurrentIpAddress(HLERequestContext& ctx) { ipv4.emplace(Network::IPv4Address{0, 0, 0, 0}); } - if (auto room_member = Network::GetRoomMember().lock()) { - if (room_member->IsConnected()) { - ipv4 = room_member->GetFakeIpAddress(); - } + if (const auto room_ip = Network::GetRoomIPv4Address()) { + ipv4 = room_ip; } IPC::ResponseBuilder rb{ctx, 3}; @@ -921,7 +920,7 @@ void IGeneralService::GetCurrentIpConfigInfo(HLERequestContext& ctx) { } if (auto room_member = Network::GetRoomMember().lock()) { - ApplyRoomAddress(*room_member, info.ip); + ApplyRoomAddress(info.ip); info.dns.is_automatic = true; info.dns.primary_dns = {1, 1, 1, 1}; info.dns.secondary_dns = {8, 8, 8, 8}; diff --git a/src/core/hle/service/sockets/bsd.cpp b/src/core/hle/service/sockets/bsd.cpp index 02c6bb7762..1b6b132769 100644 --- a/src/core/hle/service/sockets/bsd.cpp +++ b/src/core/hle/service/sockets/bsd.cpp @@ -535,33 +535,36 @@ std::pair BSD_USA::SocketImpl(Domain domain, Type type, Protocol pro UNIMPLEMENTED_IF_MSG(unk_flag, "Unknown flag in type"); type = static_cast(static_cast(type) & ~0x20000000); - const s32 fd = FindFreeFileDescriptorHandle(); - if (fd < 0) { - LOG_ERROR(Service, "No more file descriptors available"); - return {-1, Errno::MFILE}; + // Checked before taking an fd. A joined room does not lift it: room TCP is only stubbed. + if (Settings::values.airplane_mode.GetValue() && IsConnectionBased(type)) { + LOG_ERROR(Service, "Airplane mode is enabled, cannot create socket"); + return {-1, Errno::NOTCONN}; } - file_descriptors[fd] = FileDescriptor{}; - FileDescriptor& descriptor = *file_descriptors[fd]; - // ENONMEM might be thrown here - - LOG_INFO(Service, "New socket fd={}", fd); - + std::shared_ptr socket; auto room_member = Network::GetRoomMember().lock(); if (room_member && room_member->IsConnected()) { - descriptor.socket = std::make_shared(); + socket = std::make_shared(); } else { - descriptor.socket = std::make_shared(); + socket = std::make_shared(); } + socket->Initialize(Translate(domain), Translate(type), Translate(protocol)); - descriptor.socket->Initialize(Translate(domain), Translate(type), Translate(protocol)); - descriptor.is_connection_based = IsConnectionBased(type); - - if (Settings::values.airplane_mode.GetValue() && descriptor.is_connection_based) { - LOG_ERROR(Service, "Airplane mode is enabled, cannot create socket"); - return {-1, Errno::NOTCONN}; + // Publish the socket only once it is set up, since the room thread delivers packets to it + std::scoped_lock lock{file_descriptors_mutex}; + const s32 fd = FindFreeFileDescriptorHandle(); + if (fd < 0) { + LOG_ERROR(Service, "No more file descriptors available"); + return {-1, Errno::MFILE}; } + file_descriptors[fd] = FileDescriptor{ + .socket = std::move(socket), + .is_connection_based = IsConnectionBased(type), + }; + + LOG_INFO(Service, "New socket fd={}", fd); + return {fd, Errno::SUCCESS}; } @@ -649,10 +652,13 @@ std::pair BSD_USA::AcceptImpl(s32 fd, std::vector& write_buffer) return {-1, Translate(bsd_errno)}; } - file_descriptors[new_fd] = FileDescriptor{}; - FileDescriptor& new_descriptor = *file_descriptors[new_fd]; - new_descriptor.socket = std::move(result.socket); - new_descriptor.is_connection_based = descriptor.is_connection_based; + { + std::scoped_lock lock{file_descriptors_mutex}; + file_descriptors[new_fd] = FileDescriptor{ + .socket = std::move(result.socket), + .is_connection_based = descriptor.is_connection_based, + }; + } const SockAddrIn guest_addr_in = Translate(result.sockaddr_in); PutValue(write_buffer, guest_addr_in); @@ -1007,6 +1013,7 @@ Errno BSD_USA::CloseImpl(s32 fd) { LOG_INFO(Service, "Close socket fd={}", fd); + std::scoped_lock lock{file_descriptors_mutex}; file_descriptors[fd].reset(); return bsd_errno; } @@ -1016,6 +1023,7 @@ std::variant BSD_USA::DuplicateSocketImpl(s32 fd) { return Errno::BADF; } + std::scoped_lock lock{file_descriptors_mutex}; const s32 new_fd = FindFreeFileDescriptorHandle(); if (new_fd < 0) { LOG_ERROR(Service, "No more file descriptors available"); @@ -1071,12 +1079,14 @@ void BSD_USA::BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const } void BSD_USA::OnProxyPacketReceived(const Network::ProxyPacket& packet) { + // Runs on the room thread. The lock keeps a socket alive while a bsd thread closes it. + std::scoped_lock lock{file_descriptors_mutex}; for (auto& optional_descriptor : file_descriptors) { - if (!optional_descriptor.has_value()) { + if (!optional_descriptor.has_value() || !optional_descriptor->socket) { continue; } FileDescriptor& descriptor = *optional_descriptor; - descriptor.socket.get()->HandleProxyPacket(packet); + descriptor.socket->HandleProxyPacket(packet); } } @@ -1140,18 +1150,27 @@ BSD_USA::BSD_USA(Core::System& system_, const char* name, bool is_user_) RegisterHandlers(functions); + std::scoped_lock lock{proxy_binding_mutex}; + if (proxy_binding_count++ != 0) { + return; + } if (auto room_member = Network::GetRoomMember().lock()) { - proxy_packet_received = room_member->BindOnProxyPacketReceived( - [this](const Network::ProxyPacket& packet) { OnProxyPacketReceived(packet); }); + proxy_packet_received = + room_member->BindOnProxyPacketReceived(&BSD_USA::OnProxyPacketReceived); } else { LOG_ERROR(Service, "Network isn't initialized"); } } BSD_USA::~BSD_USA() { + std::scoped_lock lock{proxy_binding_mutex}; + if (--proxy_binding_count != 0) { + return; + } if (auto room_member = Network::GetRoomMember().lock()) { room_member->Unbind(proxy_packet_received); } + proxy_packet_received.reset(); } std::unique_lock BSD_USA::LockService() noexcept { diff --git a/src/core/hle/service/sockets/bsd.h b/src/core/hle/service/sockets/bsd.h index 5cc3eb5b81..0df5c58945 100644 --- a/src/core/hle/service/sockets/bsd.h +++ b/src/core/hle/service/sockets/bsd.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -181,12 +182,17 @@ private: void BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const noexcept; static inline std::array, MAX_FD> file_descriptors{}; + /// Guards file_descriptors against the room thread, which delivers proxy packets to them + static inline std::mutex file_descriptors_mutex; /// Callback to parse and handle a received wifi packet. - void OnProxyPacketReceived(const Network::ProxyPacket& packet); + static void OnProxyPacketReceived(const Network::ProxyPacket& packet); - // Callback identifier for the OnProxyPacketReceived event. - Network::RoomMember::CallbackHandle proxy_packet_received; + // Callback identifier for the OnProxyPacketReceived event. bsd:s, bsd:u and bsd:a share + // file_descriptors, so they bind once; otherwise each packet is queued once per service. + static inline Network::RoomMember::CallbackHandle proxy_packet_received; + static inline std::mutex proxy_binding_mutex; + static inline size_t proxy_binding_count = 0; protected: std::unique_lock LockService() noexcept override; diff --git a/src/core/internal_network/network.cpp b/src/core/internal_network/network.cpp index 01d37fb35b..d1c436aa08 100644 --- a/src/core/internal_network/network.cpp +++ b/src/core/internal_network/network.cpp @@ -722,6 +722,17 @@ std::optional GetHostIPv4Address() { return TranslateIPv4(network_interface->ip_address); } +std::optional GetRoomIPv4Address() { + if (const auto room_member = GetRoomMember().lock()) { + // The room host and moderators are in the Moderator state instead of Joined + const auto state = room_member->GetState(); + if (state == RoomMember::State::Joined || state == RoomMember::State::Moderator) { + return room_member->GetFakeIpAddress(); + } + } + return std::nullopt; +} + std::string IPv4AddressToString(IPv4Address ip_addr) { std::array buf = {}; ASSERT(inet_ntop(AF_INET, &ip_addr, buf.data(), sizeof(buf)) == buf.data()); diff --git a/src/core/internal_network/network.h b/src/core/internal_network/network.h index 2c9ae523c5..a598f75348 100644 --- a/src/core/internal_network/network.h +++ b/src/core/internal_network/network.h @@ -121,6 +121,10 @@ constexpr IPv4Address TranslateIPv4(in_addr addr) { /// @return human ordered IPv4 address (e.g. 192.168.0.1) as an array std::optional GetHostIPv4Address(); +/// @brief Returns the fake IPv4 address of the joined room, the emulated network while in a room +/// @return nullopt outside a room and while still joining it +std::optional GetRoomIPv4Address(); + std::string IPv4AddressToString(IPv4Address ip_addr); u32 IPv4AddressToInteger(IPv4Address ip_addr); diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index 83eb2ce9d0..6c2f79da6e 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -43,9 +43,21 @@ void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { LOG_INFO(Network, "Received broadcast packet, but not configured for broadcast mode"); return; } + } - auto decompressed = packet; - decompressed.data = Common::Compression::DecompressDataZSTD(packet.data); + // Decompress without the lock, so the game thread does not wait for it + ProxyPacket decompressed{ + .local_endpoint = packet.local_endpoint, + .remote_endpoint = packet.remote_endpoint, + .protocol = packet.protocol, + .broadcast = packet.broadcast, + .data = Common::Compression::DecompressDataZSTD(packet.data), + }; + { + std::lock_guard guard(packets_mutex); + if (closed) { + return; + } received_packets.push(std::move(decompressed)); } packets_cv.notify_one(); @@ -152,26 +164,28 @@ std::pair ProxySocket::RecvFrom(int flags, std::span message, So std::pair ProxySocket::ReceivePacket(int flags, std::span message, SockAddrIn* addr, std::size_t max_length) { - ProxyPacket& packet = received_packets.front(); + const ProxyPacket& packet = received_packets.front(); if (addr) { addr->family = Domain::INET; addr->ip = packet.local_endpoint.ip; // The senders ip address addr->portno = packet.local_endpoint.portno; // The senders port number } - const std::size_t read_bytes = (std::min)(packet.data.size(), max_length); - std::memcpy(message.data(), packet.data.data(), read_bytes); + const std::span unread = std::span{packet.data}.subspan(front_read_offset); + const std::size_t read_bytes = (std::min)(unread.size(), max_length); + std::memcpy(message.data(), unread.data(), read_bytes); if ((flags & FLAG_MSG_PEEK) != 0) { return {static_cast(read_bytes), Errno::SUCCESS}; } - if (protocol == Protocol::TCP && read_bytes < packet.data.size()) { + if (protocol == Protocol::TCP && read_bytes < unread.size()) { // Keep the unread part of the stream for the next call - packet.data.erase(packet.data.begin(), packet.data.begin() + read_bytes); + front_read_offset += read_bytes; } else { // Like BSD recv(), the rest of a datagram that does not fit is discarded received_packets.pop(); + front_read_offset = 0; } return {static_cast(read_bytes), Errno::SUCCESS}; @@ -240,6 +254,7 @@ Errno ProxySocket::Close() { fd = INVALID_SOCKET; closed = true; received_packets = {}; + front_read_offset = 0; } packets_cv.notify_all(); diff --git a/src/core/internal_network/socket_proxy.h b/src/core/internal_network/socket_proxy.h index 8343fcbae1..d2bd7aae77 100644 --- a/src/core/internal_network/socket_proxy.h +++ b/src/core/internal_network/socket_proxy.h @@ -90,7 +90,8 @@ private: SockAddrIn local_endpoint{}; bool blocking = true; std::queue received_packets; - Protocol protocol; + std::size_t front_read_offset = 0; ///< Bytes of the front TCP packet that were already read + Protocol protocol{}; std::mutex packets_mutex; std::condition_variable packets_cv; -- cgit v1.2.3 From af75958b0ed9a0e44a8c21afa768a63541d8b223 Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 00:28:31 +0100 Subject: network: use the socket type for room stream reads and wake all readers socket(AF_INET, SOCK_STREAM, 0) leaves the protocol unspecified, so the TCP checks in ProxySocket::Recv and ReceivePacket missed such sockets. recv() then waited for the timeout and returned EAGAIN instead of end of stream, and partial reads dropped the rest of the packet. Keep the socket type and use it for these checks. The protocol stays as it is, because it is sent to other room members. A MSG_PEEK reader leaves the packet queued, so notify every waiter when a packet arrives instead of only one. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/internal_network/socket_proxy.cpp | 8 +++++--- src/core/internal_network/socket_proxy.h | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index 6c2f79da6e..1b93ce93e5 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -60,7 +60,8 @@ void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { } received_packets.push(std::move(decompressed)); } - packets_cv.notify_one(); + // A MSG_PEEK reader leaves the packet queued, so every waiter must see it + packets_cv.notify_all(); } template @@ -72,6 +73,7 @@ Errno ProxySocket::SetSockOpt(SOCKET fd_, int option, T value) { Errno ProxySocket::Initialize(Domain domain, Type type, Protocol socket_protocol) { { std::lock_guard guard(packets_mutex); + socket_type = type; protocol = socket_protocol; } SetSockOpt(fd, SO_TYPE, type); @@ -122,7 +124,7 @@ Errno ProxySocket::Shutdown(ShutdownHow how) { } std::pair ProxySocket::Recv(int flags, std::span message) { - if (protocol == Protocol::TCP) { + if (socket_type == Type::STREAM) { // Room TCP sockets have no peer (Connect and Send are stubs). Report an empty queue as the // end of the stream, as before, instead of blocking the bsd service until the timeout. std::lock_guard guard(packets_mutex); @@ -179,7 +181,7 @@ std::pair ProxySocket::ReceivePacket(int flags, std::span messag return {static_cast(read_bytes), Errno::SUCCESS}; } - if (protocol == Protocol::TCP && read_bytes < unread.size()) { + if (socket_type == Type::STREAM && read_bytes < unread.size()) { // Keep the unread part of the stream for the next call front_read_offset += read_bytes; } else { diff --git a/src/core/internal_network/socket_proxy.h b/src/core/internal_network/socket_proxy.h index d2bd7aae77..5d6e35693c 100644 --- a/src/core/internal_network/socket_proxy.h +++ b/src/core/internal_network/socket_proxy.h @@ -92,6 +92,7 @@ private: std::queue received_packets; std::size_t front_read_offset = 0; ///< Bytes of the front TCP packet that were already read Protocol protocol{}; + Type socket_type{}; ///< Stream checks use the type, since protocol 0 means the default protocol std::mutex packets_mutex; std::condition_variable packets_cv; -- cgit v1.2.3 From a7e03702c10290dd46bc40e46599729510eda458 Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 00:46:55 +0100 Subject: network: keep bsd sockets alive across concurrent close and fix room gaps bsd runs on three host threads, but its calls kept a raw reference into the shared descriptor table while they used a socket. A close() on another thread could then destroy the socket mid-call. Waking a blocked room recv() on close made this happen every time. Every call now works on a copy of the descriptor taken under the table lock, poll() keeps its sockets alive while it waits, and close() resets the slot only if it still holds the same socket. accept() picked a free fd before blocking and only locked the write, so another thread could publish a socket in the same slot. It now picks and writes the fd in one critical section after Accept() returns. dup() now re-checks the source fd under the lock. A duplicated fd shares its socket, so the room packet loop queued each packet once per fd. Deliver it once per socket. NIFM: when the selected host adapter is missing, the room still works and IsNetworkAvailable reports it, but GetCurrentNetworkProfile returned an empty profile. Report a wired room profile in that case, and put the room address on the first adapter in EnumerateNetworkInterfaces. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/hle/service/nifm/nifm.cpp | 18 ++- src/core/hle/service/sockets/bsd.cpp | 257 +++++++++++++++++------------------ src/core/hle/service/sockets/bsd.h | 4 +- 3 files changed, 144 insertions(+), 135 deletions(-) diff --git a/src/core/hle/service/nifm/nifm.cpp b/src/core/hle/service/nifm/nifm.cpp index 8714597f89..ea00e9f35c 100644 --- a/src/core/hle/service/nifm/nifm.cpp +++ b/src/core/hle/service/nifm/nifm.cpp @@ -600,6 +600,19 @@ void IGeneralService::GetCurrentNetworkProfile(HLERequestContext& ctx) { std::memcpy(profile.wireless_setting_data.ssid.data(), st.ssid, profile.wireless_setting_data.ssid_length); } + } else if (Network::GetRoomIPv4Address()) { + // The selected adapter is missing, but the room still works and IsNetworkAvailable + // reports it, so describe the room as a wired network + profile.ip_setting_data.ip_address_setting.is_automatic = true; + ApplyRoomAddress(profile.ip_setting_data.ip_address_setting); + + profile.ip_setting_data.dns_setting.is_automatic = true; + profile.ip_setting_data.dns_setting.primary_dns = {1, 1, 1, 1}; + profile.ip_setting_data.dns_setting.secondary_dns = {8, 8, 8, 8}; + + profile.uuid = MakeUuidFromName(st.ssid); + profile.profile_type = static_cast(NetworkProfileType::User); + profile.interface_type = static_cast(NetworkInterfaceType::Ethernet); } ctx.WriteBuffer(profile); @@ -630,8 +643,9 @@ void IGeneralService::EnumerateNetworkInterfaces(HLERequestContext& ctx) { *reinterpret_cast(base + 0x1C) = host.subnet_mask; *reinterpret_cast(base + 0x20) = host.gateway; - // The room traffic goes through the selected adapter, so report it like the other calls - if (room_ip && selected && host.name == selected->name) { + // The room traffic goes through the selected adapter, so report it like the other calls. + // Without a selected adapter, the first one stands for the room. + if (room_ip && (selected ? host.name == selected->name : i == 0)) { std::memcpy(base + 0x18, room_ip->data(), room_ip->size()); std::memcpy(base + 0x1C, RoomSubnetMask.data(), RoomSubnetMask.size()); } diff --git a/src/core/hle/service/sockets/bsd.cpp b/src/core/hle/service/sockets/bsd.cpp index 1b6b132769..477f32626d 100644 --- a/src/core/hle/service/sockets/bsd.cpp +++ b/src/core/hle/service/sockets/bsd.cpp @@ -4,6 +4,7 @@ // SPDX-FileCopyrightText: Copyright 2018 yuzu Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later +#include #include #include #include @@ -598,31 +599,39 @@ std::pair BSD_USA::PollImpl(std::vector& write_buffer, std::span return {-1, Errno::INVAL}; } - for (PollFD& pollfd : fds) { - ASSERT(False(pollfd.revents)); - - if (pollfd.fd > static_cast(MAX_FD) || pollfd.fd < 0) { - LOG_ERROR(Service, "File descriptor handle={} is invalid", pollfd.fd); - pollfd.revents = PollEvents{}; - return {0, Errno::SUCCESS}; - } - - const std::optional& descriptor = file_descriptors[pollfd.fd]; - if (!descriptor) { - LOG_TRACE(Service, "File descriptor handle={} is not allocated", pollfd.fd); - pollfd.revents = PollEvents::Nval; - return {0, Errno::SUCCESS}; + // The copies keep the sockets alive while Network::Poll waits, even if another bsd thread + // closes one of them meanwhile + std::vector> sockets(fds.size()); + { + std::scoped_lock lock{file_descriptors_mutex}; + for (size_t i = 0; i < fds.size(); ++i) { + PollFD& pollfd = fds[i]; + ASSERT(False(pollfd.revents)); + + if (pollfd.fd > static_cast(MAX_FD) || pollfd.fd < 0) { + LOG_ERROR(Service, "File descriptor handle={} is invalid", pollfd.fd); + pollfd.revents = PollEvents{}; + return {0, Errno::SUCCESS}; + } + + const std::optional& descriptor = file_descriptors[pollfd.fd]; + if (!descriptor) { + LOG_TRACE(Service, "File descriptor handle={} is not allocated", pollfd.fd); + pollfd.revents = PollEvents::Nval; + return {0, Errno::SUCCESS}; + } + sockets[i] = descriptor->socket; } } std::vector host_pollfds(fds.size()); - std::transform(fds.begin(), fds.end(), host_pollfds.begin(), [](PollFD pollfd) { - Network::PollFD result; - result.socket = file_descriptors[pollfd.fd]->socket.get(); - result.events = Translate(pollfd.events); - result.revents = Network::PollEvents{}; - return result; - }); + for (size_t i = 0; i < fds.size(); ++i) { + host_pollfds[i] = Network::PollFD{ + .socket = sockets[i].get(), + .events = Translate(fds[i].events), + .revents = Network::PollEvents{}, + }; + } const auto result = Network::Poll(host_pollfds, timeout); @@ -636,27 +645,28 @@ std::pair BSD_USA::PollImpl(std::vector& write_buffer, std::span } std::pair BSD_USA::AcceptImpl(s32 fd, std::vector& write_buffer) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return {-1, Errno::BADF}; } - const s32 new_fd = FindFreeFileDescriptorHandle(); - if (new_fd < 0) { - LOG_ERROR(Service, "No more file descriptors available"); - return {-1, Errno::MFILE}; - } - - FileDescriptor& descriptor = *file_descriptors[fd]; - auto [result, bsd_errno] = descriptor.socket->Accept(); + auto [result, bsd_errno] = descriptor->socket->Accept(); if (bsd_errno != Network::Errno::SUCCESS) { return {-1, Translate(bsd_errno)}; } + // Accept may block, so the fd is picked afterwards, in the same critical section as the write + s32 new_fd; { std::scoped_lock lock{file_descriptors_mutex}; + new_fd = FindFreeFileDescriptorHandle(); + if (new_fd < 0) { + LOG_ERROR(Service, "No more file descriptors available"); + return {-1, Errno::MFILE}; + } file_descriptors[new_fd] = FileDescriptor{ .socket = std::move(result.socket), - .is_connection_based = descriptor.is_connection_based, + .is_connection_based = descriptor->is_connection_based, }; } @@ -667,34 +677,28 @@ std::pair BSD_USA::AcceptImpl(s32 fd, std::vector& write_buffer) } Errno BSD_USA::BindImpl(s32 fd, std::span addr) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } ASSERT(addr.size() >= 16); - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); - return Errno::BADF; - } auto addr_in = GetValue(addr); - return Translate(file_descriptors[fd]->socket->Bind(Translate(addr_in))); + return Translate(descriptor->socket->Bind(Translate(addr_in))); } Errno BSD_USA::ConnectImpl(s32 fd, std::span addr) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } ASSERT(addr.size() >= 16); - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); - return Errno::BADF; - } auto addr_in = GetValue(addr); - const Errno result = Translate(file_descriptors[fd]->socket->Connect(Translate(addr_in))); + const Errno result = Translate(descriptor->socket->Connect(Translate(addr_in))); if (result == Errno::ISCONN) { LOG_DEBUG(Service, "returned ISCONN - socket already connected"); @@ -705,16 +709,12 @@ Errno BSD_USA::ConnectImpl(s32 fd, std::span addr) { } Errno BSD_USA::GetPeerNameImpl(s32 fd, std::vector& write_buffer) { - if (!IsFileDescriptorValid(fd)) { - return Errno::BADF; - } - - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } - const auto [addr_in, bsd_errno] = file_descriptors[fd]->socket->GetPeerName(); + const auto [addr_in, bsd_errno] = descriptor->socket->GetPeerName(); if (bsd_errno != Network::Errno::SUCCESS) { return Translate(bsd_errno); } @@ -727,16 +727,12 @@ Errno BSD_USA::GetPeerNameImpl(s32 fd, std::vector& write_buffer) { } Errno BSD_USA::GetSockNameImpl(s32 fd, std::vector& write_buffer) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); - return Errno::BADF; - } - - const auto [addr_in, bsd_errno] = file_descriptors[fd]->socket->GetSockName(); + const auto [addr_in, bsd_errno] = descriptor->socket->GetSockName(); if (bsd_errno != Network::Errno::SUCCESS) { return Translate(bsd_errno); } @@ -749,38 +745,33 @@ Errno BSD_USA::GetSockNameImpl(s32 fd, std::vector& write_buffer) { } Errno BSD_USA::ListenImpl(s32 fd, s32 backlog) { - if (!IsFileDescriptorValid(fd)) { - return Errno::BADF; - } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } - return Translate(file_descriptors[fd]->socket->Listen(backlog)); + return Translate(descriptor->socket->Listen(backlog)); } std::pair BSD_USA::FcntlImpl(s32 fd, FcntlCmd cmd, s32 arg) { - if (!IsFileDescriptorValid(fd)) { - return {-1, Errno::BADF}; - } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return {-1, Errno::BADF}; } - FileDescriptor& descriptor = *file_descriptors[fd]; - switch (cmd) { case FcntlCmd::GETFL: ASSERT(arg == 0); - return {descriptor.flags, Errno::SUCCESS}; + return {descriptor->flags, Errno::SUCCESS}; case FcntlCmd::SETFL: { const bool enable = (arg & Network::FLAG_O_NONBLOCK) != 0; - const Errno bsd_errno = Translate(descriptor.socket->SetNonBlock(enable)); + const Errno bsd_errno = Translate(descriptor->socket->SetNonBlock(enable)); if (bsd_errno != Errno::SUCCESS) { return {-1, bsd_errno}; } - descriptor.flags = arg; + std::scoped_lock lock{file_descriptors_mutex}; + if (auto& entry = file_descriptors[fd]; entry && entry->socket == descriptor->socket) { + entry->flags = arg; + } return {0, Errno::SUCCESS}; } default: @@ -790,11 +781,8 @@ std::pair BSD_USA::FcntlImpl(s32 fd, FcntlCmd cmd, s32 arg) { } Errno BSD_USA::GetSockOptImpl(s32 fd, u32 level, OptName optname, std::vector& optval) { - if (!IsFileDescriptorValid(fd)) { - return Errno::BADF; - } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } @@ -803,7 +791,7 @@ Errno BSD_USA::GetSockOptImpl(s32 fd, u32 level, OptName optname, std::vectorsocket.get(); + Network::SocketBase* const socket = descriptor->socket.get(); switch (optname) { case OptName::ERROR_: { @@ -825,11 +813,8 @@ Errno BSD_USA::GetSockOptImpl(s32 fd, u32 level, OptName optname, std::vector optval) { - if (!IsFileDescriptorValid(fd)) { - return Errno::BADF; - } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } @@ -838,7 +823,7 @@ Errno BSD_USA::SetSockOptImpl(s32 fd, u32 level, OptName optname, std::spansocket.get(); + Network::SocketBase* const socket = descriptor->socket.get(); if (optname == OptName::LINGER) { ASSERT(optval.size() == sizeof(Linger)); @@ -879,39 +864,35 @@ Errno BSD_USA::SetSockOptImpl(s32 fd, u32 level, OptName optname, std::spansocket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } const Network::ShutdownHow host_how = Translate(static_cast(how)); - return Translate(file_descriptors[fd]->socket->Shutdown(host_how)); + return Translate(descriptor->socket->Shutdown(host_how)); } std::pair BSD_USA::RecvImpl(s32 fd, u32 flags, std::vector& message) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return {-1, Errno::BADF}; } - FileDescriptor& descriptor = *file_descriptors[fd]; - // Apply flags using Network::FLAG_MSG_DONTWAIT; using Network::FLAG_O_NONBLOCK; if ((flags & FLAG_MSG_DONTWAIT) != 0) { flags &= ~FLAG_MSG_DONTWAIT; - if ((descriptor.flags & FLAG_O_NONBLOCK) == 0) { - descriptor.socket->SetNonBlock(true); + if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { + descriptor->socket->SetNonBlock(true); } } - const auto [ret, bsd_errno] = Translate(descriptor.socket->Recv(flags, message)); + const auto [ret, bsd_errno] = Translate(descriptor->socket->Recv(flags, message)); // Restore original state - if ((descriptor.flags & FLAG_O_NONBLOCK) == 0) { - descriptor.socket->SetNonBlock(false); + if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { + descriptor->socket->SetNonBlock(false); } return {ret, bsd_errno}; @@ -919,15 +900,14 @@ std::pair BSD_USA::RecvImpl(s32 fd, u32 flags, std::vector& mess std::pair BSD_USA::RecvFromImpl(s32 fd, u32 flags, std::vector& message, std::vector& addr) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return {-1, Errno::BADF}; } - FileDescriptor& descriptor = *file_descriptors[fd]; - Network::SockAddrIn addr_in{}; Network::SockAddrIn* p_addr_in = nullptr; - if (descriptor.is_connection_based) { + if (descriptor->is_connection_based) { // Connection based file descriptors (e.g. TCP) zero addr addr.clear(); } else { @@ -939,16 +919,17 @@ std::pair BSD_USA::RecvFromImpl(s32 fd, u32 flags, std::vector& using Network::FLAG_O_NONBLOCK; if ((flags & FLAG_MSG_DONTWAIT) != 0) { flags &= ~FLAG_MSG_DONTWAIT; - if ((descriptor.flags & FLAG_O_NONBLOCK) == 0) { - descriptor.socket->SetNonBlock(true); + if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { + descriptor->socket->SetNonBlock(true); } } - const auto [ret, bsd_errno] = Translate(descriptor.socket->RecvFrom(flags, message, p_addr_in)); + const auto [ret, bsd_errno] = + Translate(descriptor->socket->RecvFrom(flags, message, p_addr_in)); // Restore original state - if ((descriptor.flags & FLAG_O_NONBLOCK) == 0) { - descriptor.socket->SetNonBlock(false); + if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { + descriptor->socket->SetNonBlock(false); } if (p_addr_in) { @@ -965,23 +946,17 @@ std::pair BSD_USA::RecvFromImpl(s32 fd, u32 flags, std::vector& } std::pair BSD_USA::SendImpl(s32 fd, u32 flags, std::span message) { - if (!IsFileDescriptorValid(fd)) { + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return {-1, Errno::BADF}; } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); - return {-1, Errno::BADF}; - } - return Translate(file_descriptors[fd]->socket->Send(message, flags)); + return Translate(descriptor->socket->Send(message, flags)); } std::pair BSD_USA::SendToImpl(s32 fd, u32 flags, std::span message, std::span addr) { - if (!IsFileDescriptorValid(fd)) { - return {-1, Errno::BADF}; - } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return {-1, Errno::BADF}; } @@ -994,36 +969,36 @@ std::pair BSD_USA::SendToImpl(s32 fd, u32 flags, std::span p_addr_in = &addr_in; } - return Translate(file_descriptors[fd]->socket->SendTo(flags, message, p_addr_in)); + return Translate(descriptor->socket->SendTo(flags, message, p_addr_in)); } Errno BSD_USA::CloseImpl(s32 fd) { - if (!IsFileDescriptorValid(fd)) { - return Errno::BADF; - } - if (!file_descriptors[fd]->socket) { - LOG_WARNING(Service, "Uninitialized socket"); + const auto descriptor = CopyDescriptor(fd); + if (!descriptor) { return Errno::BADF; } - const Errno bsd_errno = Translate(file_descriptors[fd]->socket->Close()); + const Errno bsd_errno = Translate(descriptor->socket->Close()); if (bsd_errno != Errno::SUCCESS) { return bsd_errno; } LOG_INFO(Service, "Close socket fd={}", fd); + // Another thread may have closed this fd and reused the slot meanwhile std::scoped_lock lock{file_descriptors_mutex}; - file_descriptors[fd].reset(); + if (file_descriptors[fd] && file_descriptors[fd]->socket == descriptor->socket) { + file_descriptors[fd].reset(); + } return bsd_errno; } std::variant BSD_USA::DuplicateSocketImpl(s32 fd) { - if (!IsFileDescriptorValid(fd)) { + std::scoped_lock lock{file_descriptors_mutex}; + if (!IsFileDescriptorValid(fd) || !file_descriptors[fd]->socket) { return Errno::BADF; } - std::scoped_lock lock{file_descriptors_mutex}; const s32 new_fd = FindFreeFileDescriptorHandle(); if (new_fd < 0) { LOG_ERROR(Service, "No more file descriptors available"); @@ -1039,6 +1014,16 @@ std::variant BSD_USA::DuplicateSocketImpl(s32 fd) { } std::optional> BSD_USA::GetSocket(s32 fd) { + if (auto descriptor = CopyDescriptor(fd)) { + return std::move(descriptor->socket); + } + return std::nullopt; +} + +std::optional BSD_USA::CopyDescriptor(s32 fd) { + // The copy holds a reference to the socket, so a close on another bsd thread cannot destroy + // it while the caller uses it + std::scoped_lock lock{file_descriptors_mutex}; if (!IsFileDescriptorValid(fd)) { return std::nullopt; } @@ -1046,7 +1031,7 @@ std::optional> BSD_USA::GetSocket(s32 fd) { LOG_WARNING(Service, "Uninitialized socket"); return std::nullopt; } - return file_descriptors[fd]->socket; + return file_descriptors[fd]; } s32 BSD_USA::FindFreeFileDescriptorHandle() noexcept { @@ -1081,12 +1066,20 @@ void BSD_USA::BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const void BSD_USA::OnProxyPacketReceived(const Network::ProxyPacket& packet) { // Runs on the room thread. The lock keeps a socket alive while a bsd thread closes it. std::scoped_lock lock{file_descriptors_mutex}; + // A duplicated fd shares its socket, which must still get each packet only once + std::array delivered{}; + size_t num_delivered = 0; for (auto& optional_descriptor : file_descriptors) { if (!optional_descriptor.has_value() || !optional_descriptor->socket) { continue; } - FileDescriptor& descriptor = *optional_descriptor; - descriptor.socket->HandleProxyPacket(packet); + Network::SocketBase* const socket = optional_descriptor->socket.get(); + const auto delivered_end = delivered.begin() + num_delivered; + if (std::find(delivered.begin(), delivered_end, socket) != delivered_end) { + continue; + } + delivered[num_delivered++] = socket; + socket->HandleProxyPacket(packet); } } diff --git a/src/core/hle/service/sockets/bsd.h b/src/core/hle/service/sockets/bsd.h index 0df5c58945..f908edc7d2 100644 --- a/src/core/hle/service/sockets/bsd.h +++ b/src/core/hle/service/sockets/bsd.h @@ -178,11 +178,13 @@ private: s32 FindFreeFileDescriptorHandle() noexcept; bool IsFileDescriptorValid(s32 fd) const noexcept; + /// Copies a descriptor under file_descriptors_mutex; nullopt if fd has no socket + std::optional CopyDescriptor(s32 fd); void BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const noexcept; static inline std::array, MAX_FD> file_descriptors{}; - /// Guards file_descriptors against the room thread, which delivers proxy packets to them + /// Guards file_descriptors. Calls use CopyDescriptor, so sockets outlive a concurrent close. static inline std::mutex file_descriptors_mutex; /// Callback to parse and handle a received wifi packet. -- cgit v1.2.3 From 01090e8e44d8d64bb5839f8d842bdd3028447608 Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 01:08:49 +0100 Subject: tests: cover room ProxySocket receives and the LDN join handshake Add unit tests for the room LAN fixes in this branch. The socket tests check datagram truncation, TCP remainders and peeks, timeouts, wakeups on close and on a new packet, and stream behavior with the default protocol. The LDN tests check that a join waits for the host's reply, rejects bad or duplicate joins, ignores late replies and uses fresh identities. Assisted-by: Sol (LDN join tests) Assisted-by: Claude Opus 5.5 (room socket tests) --- src/tests/CMakeLists.txt | 2 + src/tests/core/internal_network/socket_proxy.cpp | 231 +++++++++++++++++++++++ src/tests/core/ldn.cpp | 202 ++++++++++++++++++++ 3 files changed, 435 insertions(+) create mode 100644 src/tests/core/internal_network/socket_proxy.cpp create mode 100644 src/tests/core/ldn.cpp diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 1eb1040e34..a31c073765 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -19,6 +19,8 @@ add_executable(tests common/undefined_fix.cpp common/unique_function.cpp core/core_timing.cpp + core/ldn.cpp + core/internal_network/socket_proxy.cpp core/nup_writer.cpp core/priority_queue.cpp core/internal_network/network.cpp diff --git a/src/tests/core/internal_network/socket_proxy.cpp b/src/tests/core/internal_network/socket_proxy.cpp new file mode 100644 index 0000000000..d97d7d5102 --- /dev/null +++ b/src/tests/core/internal_network/socket_proxy.cpp @@ -0,0 +1,231 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include +#include + +#include + +#include "common/socket_types.h" +#include "common/zstd_compression.h" +#include "core/internal_network/socket_proxy.h" + +namespace { + +void BindSocket(Network::ProxySocket& socket, Network::Protocol protocol) { + const auto type = + protocol == Network::Protocol::TCP ? Network::Type::STREAM : Network::Type::DGRAM; + REQUIRE(socket.Initialize(Network::Domain::INET, type, protocol) == Network::Errno::SUCCESS); + REQUIRE(socket.Bind({.family = Network::Domain::INET, + .ip = {192, 168, 1, 1}, + .portno = 35000}) == Network::Errno::SUCCESS); +} + +template +void QueuePacket(Network::ProxySocket& socket, Network::Protocol protocol, + const std::array& payload) { + Network::ProxyPacket packet{}; + packet.local_endpoint = {.family = Network::Domain::INET, + .ip = {192, 168, 1, 2}, + .portno = 35000}; + packet.remote_endpoint = {.family = Network::Domain::INET, + .ip = {192, 168, 1, 1}, + .portno = 35000}; + packet.protocol = protocol; + packet.data = Common::Compression::CompressDataZSTDDefault(payload.data(), payload.size()); + socket.HandleProxyPacket(packet); +} + +} // namespace + +TEST_CASE("Room UDP sockets receive queued datagrams with recv", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + REQUIRE(socket.SetNonBlock(true) == Network::Errno::SUCCESS); + + const std::array payload{1, 2, 3, 4}; + QueuePacket(socket, Network::Protocol::UDP, payload); + + std::array received{}; + const auto [count, error] = socket.Recv(0, received); + REQUIRE(error == Network::Errno::SUCCESS); + REQUIRE(count == payload.size()); + REQUIRE(received == payload); + + const auto [empty_count, empty_error] = socket.Recv(0, received); + REQUIRE(empty_count == -1); + REQUIRE(empty_error == Network::Errno::AGAIN); +} + +TEST_CASE("Room sockets peek without consuming", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + REQUIRE(socket.SetNonBlock(true) == Network::Errno::SUCCESS); + + const std::array payload{1, 2, 3, 4}; + QueuePacket(socket, Network::Protocol::UDP, payload); + + std::array received{}; + REQUIRE(socket.Recv(Network::FLAG_MSG_PEEK, received) == + std::pair{4, Network::Errno::SUCCESS}); + REQUIRE(socket.Recv(0, received) == std::pair{4, Network::Errno::SUCCESS}); + REQUIRE(received == payload); + REQUIRE(socket.Recv(0, received).second == Network::Errno::AGAIN); +} + +TEST_CASE("Room UDP sockets truncate oversized datagrams", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + REQUIRE(socket.SetNonBlock(true) == Network::Errno::SUCCESS); + + QueuePacket(socket, Network::Protocol::UDP, std::array{1, 2, 3, 4}); + + std::array received{}; + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{1, 2}); + REQUIRE(socket.Recv(0, received).second == Network::Errno::AGAIN); +} + +TEST_CASE("Room TCP sockets keep the unread part of a packet", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::TCP); + REQUIRE(socket.SetNonBlock(true) == Network::Errno::SUCCESS); + + QueuePacket(socket, Network::Protocol::TCP, std::array{1, 2, 3, 4, 5, 6}); + + std::array received{}; + REQUIRE(socket.Recv(0, received) == std::pair{4, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{1, 2, 3, 4}); + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received[0] == 5); + REQUIRE(received[1] == 6); + REQUIRE(socket.Recv(0, received) == std::pair{0, Network::Errno::SUCCESS}); +} + +TEST_CASE("Room TCP sockets read a packet in small parts and peek at the unread part", + "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::TCP); + REQUIRE(socket.SetNonBlock(true) == Network::Errno::SUCCESS); + + QueuePacket(socket, Network::Protocol::TCP, std::array{1, 2, 3, 4, 5}); + QueuePacket(socket, Network::Protocol::TCP, std::array{6, 7}); + + std::array received{}; + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{1, 2}); + REQUIRE(socket.Recv(Network::FLAG_MSG_PEEK, received) == + std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{3, 4}); + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{3, 4}); + REQUIRE(socket.Recv(0, received) == std::pair{1, Network::Errno::SUCCESS}); + REQUIRE(received[0] == 5); + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{6, 7}); +} + +TEST_CASE("Room TCP sockets report an empty queue as end of stream", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::TCP); + + std::array received{}; + const auto start = std::chrono::steady_clock::now(); + REQUIRE(socket.Recv(0, received) == std::pair{0, Network::Errno::SUCCESS}); + REQUIRE(std::chrono::steady_clock::now() - start < std::chrono::seconds{1}); +} + +TEST_CASE("Blocking room sockets wait for packets and time out with EAGAIN", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + REQUIRE(socket.SetRcvTimeo(50) == Network::Errno::SUCCESS); + + std::array received{}; + const auto start = std::chrono::steady_clock::now(); + REQUIRE(socket.Recv(0, received) == std::pair{-1, Network::Errno::AGAIN}); + REQUIRE(std::chrono::steady_clock::now() - start >= std::chrono::milliseconds{50}); + + REQUIRE(socket.SetRcvTimeo(5000) == Network::Errno::SUCCESS); + const std::array payload{1, 2, 3, 4}; + std::jthread sender([&socket, &payload] { + std::this_thread::sleep_for(std::chrono::milliseconds{20}); + QueuePacket(socket, Network::Protocol::UDP, payload); + }); + REQUIRE(socket.Recv(0, received) == std::pair{4, Network::Errno::SUCCESS}); + REQUIRE(received == payload); +} + +TEST_CASE("Closing a room socket wakes a blocked recv and drops queued packets", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + REQUIRE(socket.SetRcvTimeo(5000) == Network::Errno::SUCCESS); + + std::array received{}; + const auto start = std::chrono::steady_clock::now(); + std::jthread closer([&socket] { + std::this_thread::sleep_for(std::chrono::milliseconds{20}); + socket.Close(); + }); + REQUIRE(socket.Recv(0, received) == std::pair{-1, Network::Errno::BADF}); + REQUIRE(std::chrono::steady_clock::now() - start < std::chrono::seconds{1}); + closer.join(); + + Network::ProxySocket queued; + BindSocket(queued, Network::Protocol::UDP); + QueuePacket(queued, Network::Protocol::UDP, std::array{1, 2, 3, 4}); + REQUIRE(queued.Close() == Network::Errno::SUCCESS); + QueuePacket(queued, Network::Protocol::UDP, std::array{5, 6, 7, 8}); + REQUIRE(queued.Recv(0, received) == std::pair{-1, Network::Errno::BADF}); +} + +TEST_CASE("Room stream sockets with the default protocol keep stream behavior", "[core][sockets]") { + // socket(AF_INET, SOCK_STREAM, 0) leaves the protocol unspecified + Network::ProxySocket socket; + REQUIRE(socket.Initialize(Network::Domain::INET, Network::Type::STREAM, + Network::Protocol::Unspecified) == Network::Errno::SUCCESS); + REQUIRE(socket.Bind({.family = Network::Domain::INET, + .ip = {192, 168, 1, 1}, + .portno = 35000}) == Network::Errno::SUCCESS); + REQUIRE(socket.SetRcvTimeo(1000) == Network::Errno::SUCCESS); + + std::array received{}; + const auto start = std::chrono::steady_clock::now(); + REQUIRE(socket.Recv(0, received) == std::pair{0, Network::Errno::SUCCESS}); + REQUIRE(std::chrono::steady_clock::now() - start < std::chrono::milliseconds{500}); + + QueuePacket(socket, Network::Protocol::Unspecified, std::array{1, 2, 3, 4}); + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{1, 2}); + REQUIRE(socket.Recv(0, received) == std::pair{2, Network::Errno::SUCCESS}); + REQUIRE(received == std::array{3, 4}); +} + +TEST_CASE("One room packet wakes every blocked peek reader", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + REQUIRE(socket.SetRcvTimeo(2000) == Network::Errno::SUCCESS); + + // Catch2 assertions are not thread-safe here, so the readers only record their results + std::array, 2> results{}; + std::array finished{}; + std::chrono::steady_clock::time_point queued{}; + { + std::array readers; + for (std::size_t i = 0; i < readers.size(); ++i) { + readers[i] = std::jthread([&socket, &results, &finished, i] { + std::array peeked{}; + results[i] = socket.Recv(Network::FLAG_MSG_PEEK, peeked); + finished[i] = std::chrono::steady_clock::now(); + }); + } + std::this_thread::sleep_for(std::chrono::milliseconds{50}); + queued = std::chrono::steady_clock::now(); + QueuePacket(socket, Network::Protocol::UDP, std::array{1, 2, 3, 4}); + } + + for (std::size_t i = 0; i < results.size(); ++i) { + REQUIRE(results[i] == std::pair{4, Network::Errno::SUCCESS}); + REQUIRE(finished[i] - queued < std::chrono::milliseconds{1000}); + } +} diff --git a/src/tests/core/ldn.cpp b/src/tests/core/ldn.cpp new file mode 100644 index 0000000000..4e6da2d52e --- /dev/null +++ b/src/tests/core/ldn.cpp @@ -0,0 +1,202 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include +#include +#include +#include +#include +#include + +#include "core/hle/service/ldn/lan_discovery.h" +#include "core/internal_network/network_interface.h" + +using namespace std::chrono_literals; + +namespace { + +class TestLANDiscovery : public Service::LDN::LANDiscovery { +public: + void SetConnectionTimeout(std::chrono::nanoseconds timeout) { + connection_timeout = timeout; + } + + std::chrono::nanoseconds GetConnectionTimeout() const { + return connection_timeout; + } + + bool WaitUntilConnecting() { + const auto deadline = std::chrono::steady_clock::now() + 5s; + do { + { + std::scoped_lock lock{packet_mutex}; + if (host_ip) { + // Connect has published its target and released this lock to wait for a reply. + return true; + } + } + std::this_thread::sleep_for(1ms); + } while (std::chrono::steady_clock::now() < deadline); + return false; + } +}; + +} // namespace + +TEST_CASE("LDN waits for the selected host's connection reply", "[core][ldn]") { + using namespace Service::LDN; + if (!Network::GetSelectedNetworkInterface()) { + SKIP("LDN requires a host IPv4 interface"); + } + + TestLANDiscovery station; + REQUIRE(station.Initialize([] {}).IsSuccess()); + REQUIRE(station.OpenStation().IsSuccess()); + + NetworkInfo info{}; + info.network_id.session_id = {1, 2}; + info.ldn.node_count = 1; + info.ldn.node_count_max = 8; + info.ldn.nodes[0].ipv4_address = {1, 0, 168, 192}; + + Network::LDNPacket reply{}; + reply.type = Network::LDNPacketType::SyncNetwork; + reply.local_ip = {192, 168, 0, 1}; + reply.data.resize(sizeof(info)); + std::memcpy(reply.data.data(), &info, sizeof(info)); + + // A scan result or unsolicited sync must not connect an idle station. + station.ReceivePacket(reply); + REQUIRE(station.GetState() == State::StationOpened); + + auto connecting = std::async(std::launch::async, [&] { return station.Connect(info, {}, 1); }); + REQUIRE(station.WaitUntilConnecting()); + + auto unrelated = reply; + unrelated.local_ip[3] = 2; + station.ReceivePacket(unrelated); + REQUIRE(connecting.wait_for(0ms) == std::future_status::timeout); + + auto stale = info; + stale.network_id.session_id.low++; + std::memcpy(unrelated.data.data(), &stale, sizeof(stale)); + unrelated.local_ip = reply.local_ip; + station.ReceivePacket(unrelated); + REQUIRE(connecting.wait_for(0ms) == std::future_status::timeout); + + unrelated.data.resize(1); + station.ReceivePacket(unrelated); + REQUIRE(connecting.wait_for(0ms) == std::future_status::timeout); + + station.ReceivePacket(reply); + REQUIRE(connecting.wait_for(5s) == std::future_status::ready); + REQUIRE(connecting.get().IsSuccess()); + REQUIRE(station.GetState() == State::StationConnected); + REQUIRE(station.Finalize().IsSuccess()); +} + +TEST_CASE("LDN host validates join packets and does not allocate duplicate stations", + "[core][ldn]") { + using namespace Service::LDN; + if (!Network::GetSelectedNetworkInterface()) { + SKIP("LDN requires a host IPv4 interface"); + } + + LANDiscovery host; + REQUIRE(host.Initialize([] {}).IsSuccess()); + REQUIRE(host.OpenAccessPoint().IsSuccess()); + NetworkConfig config{}; + config.node_count_max = 2; + REQUIRE(host.CreateNetwork({}, {}, config).IsSuccess()); + + NodeInfo node{}; + node.is_connected = 1; + node.mac_address.raw[5] = 1; + Network::LDNPacket request{}; + request.type = Network::LDNPacketType::Connect; + request.local_ip = {192, 168, 0, 2}; + request.data.resize(sizeof(node)); + std::memcpy(request.data.data(), &node, sizeof(node)); + host.ReceivePacket(request); + NetworkInfo info{}; + REQUIRE(host.GetNetworkInfo(info).IsSuccess()); + REQUIRE(info.ldn.node_count == 2); + + host.ReceivePacket(request); + REQUIRE(host.GetNetworkInfo(info).IsSuccess()); + REQUIRE(info.ldn.node_count == 2); + + // A third console cannot enter a two-console network. + request.local_ip[3] = 3; + host.ReceivePacket(request); + REQUIRE(host.GetNetworkInfo(info).IsSuccess()); + REQUIRE(info.ldn.node_count == 2); + + request.type = Network::LDNPacketType::Disconnect; + request.data.resize(1); + host.ReceivePacket(request); + REQUIRE(host.GetNetworkInfo(info).IsSuccess()); + REQUIRE(info.ldn.node_count == 2); + REQUIRE(host.Finalize().IsSuccess()); +} + +TEST_CASE("LDN reports a missing connection reply and ignores late replies", "[core][ldn]") { + using namespace Service::LDN; + if (!Network::GetSelectedNetworkInterface()) { + SKIP("LDN requires a host IPv4 interface"); + } + + TestLANDiscovery station; + REQUIRE(station.GetConnectionTimeout() == 5s); + station.SetConnectionTimeout(25ms); + REQUIRE(station.Initialize([] {}).IsSuccess()); + REQUIRE(station.OpenStation().IsSuccess()); + NetworkInfo info{}; + info.ldn.node_count = 1; + info.ldn.nodes[0].ipv4_address = {1, 0, 168, 192}; + REQUIRE(station.Connect(info, {}, 1) == ResultAuthenticationTimeout); + REQUIRE(station.GetState() == State::StationOpened); + + Network::LDNPacket reply{}; + reply.type = Network::LDNPacketType::SyncNetwork; + reply.local_ip = {192, 168, 0, 1}; + reply.data.resize(sizeof(info)); + std::memcpy(reply.data.data(), &info, sizeof(info)); + station.ReceivePacket(reply); + REQUIRE(station.GetState() == State::StationOpened); + REQUIRE(station.Finalize().IsSuccess()); +} + +TEST_CASE("LDN creates fresh identities for recreated networks and separate hosts", "[core][ldn]") { + using namespace Service::LDN; + if (!Network::GetSelectedNetworkInterface()) { + SKIP("LDN requires a host IPv4 interface"); + } + + LANDiscovery host; + LANDiscovery other_host; + REQUIRE(host.Initialize([] {}).IsSuccess()); + REQUIRE(other_host.Initialize([] {}).IsSuccess()); + REQUIRE(host.OpenAccessPoint().IsSuccess()); + REQUIRE(other_host.OpenAccessPoint().IsSuccess()); + NetworkConfig config{}; + config.node_count_max = 2; + REQUIRE(host.CreateNetwork({}, {}, config).IsSuccess()); + NetworkInfo first{}; + REQUIRE(host.GetNetworkInfo(first).IsSuccess()); + + REQUIRE(host.DestroyNetwork().IsSuccess()); + REQUIRE(host.CreateNetwork({}, {}, config).IsSuccess()); + NetworkInfo recreated{}; + REQUIRE(host.GetNetworkInfo(recreated).IsSuccess()); + REQUIRE(recreated.network_id.session_id != first.network_id.session_id); + + REQUIRE(other_host.CreateNetwork({}, {}, config).IsSuccess()); + NetworkInfo other{}; + REQUIRE(other_host.GetNetworkInfo(other).IsSuccess()); + REQUIRE(other.network_id.session_id != first.network_id.session_id); + REQUIRE(other.network_id.session_id != recreated.network_id.session_id); + REQUIRE(host.Finalize().IsSuccess()); + REQUIRE(other_host.Finalize().IsSuccess()); +} -- cgit v1.2.3 From 9b56bf38cb92c42abd776d95bdd61f96bf9d1e9a Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 01:15:30 +0100 Subject: network: keep fcntl modes across MSG_DONTWAIT and decompress room packets once A receive with MSG_DONTWAIT restored the blocking mode from the descriptor copy taken before the call. When fcntl(SETFL, O_NONBLOCK) ran during the receive, the fd kept reporting O_NONBLOCK while its socket blocked again. Receives now switch the mode only for MSG_DONTWAIT and read the current flags under file_descriptors_mutex at both ends, and SETFL changes the mode and the stored flags in the same critical section. OnProxyPacketReceived decompressed each room packet once per matching socket while holding file_descriptors_mutex, so every bsd call waited for it. It now copies the distinct sockets under the lock, decompresses once without it and delivers the result. Also reject fd == MAX_FD in IsFileDescriptorValid and PollImpl, which read past the table, and guard the ProxySocket blocking and receive timeout setters with packets_mutex, matching their reader. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/core/hle/service/sockets/bsd.cpp | 92 ++++++++++++++++++------------ src/core/hle/service/sockets/bsd.h | 2 + src/core/internal_network/socket_proxy.cpp | 23 ++------ src/core/internal_network/sockets.h | 1 + 4 files changed, 65 insertions(+), 53 deletions(-) diff --git a/src/core/hle/service/sockets/bsd.cpp b/src/core/hle/service/sockets/bsd.cpp index 477f32626d..e6d23b7bf6 100644 --- a/src/core/hle/service/sockets/bsd.cpp +++ b/src/core/hle/service/sockets/bsd.cpp @@ -14,6 +14,7 @@ #include "common/logging.h" #include "common/socket_types.h" +#include "common/zstd_compression.h" #include "core/core.h" #include "core/hle/kernel/k_thread.h" #include "core/hle/service/ipc_helpers.h" @@ -608,7 +609,7 @@ std::pair BSD_USA::PollImpl(std::vector& write_buffer, std::span PollFD& pollfd = fds[i]; ASSERT(False(pollfd.revents)); - if (pollfd.fd > static_cast(MAX_FD) || pollfd.fd < 0) { + if (pollfd.fd >= static_cast(MAX_FD) || pollfd.fd < 0) { LOG_ERROR(Service, "File descriptor handle={} is invalid", pollfd.fd); pollfd.revents = PollEvents{}; return {0, Errno::SUCCESS}; @@ -764,11 +765,12 @@ std::pair BSD_USA::FcntlImpl(s32 fd, FcntlCmd cmd, s32 arg) { return {descriptor->flags, Errno::SUCCESS}; case FcntlCmd::SETFL: { const bool enable = (arg & Network::FLAG_O_NONBLOCK) != 0; + // Set the mode and the flags together, so SetDontWait never sees one without the other + std::scoped_lock lock{file_descriptors_mutex}; const Errno bsd_errno = Translate(descriptor->socket->SetNonBlock(enable)); if (bsd_errno != Errno::SUCCESS) { return {-1, bsd_errno}; } - std::scoped_lock lock{file_descriptors_mutex}; if (auto& entry = file_descriptors[fd]; entry && entry->socket == descriptor->socket) { entry->flags = arg; } @@ -880,19 +882,14 @@ std::pair BSD_USA::RecvImpl(s32 fd, u32 flags, std::vector& mess // Apply flags using Network::FLAG_MSG_DONTWAIT; - using Network::FLAG_O_NONBLOCK; - if ((flags & FLAG_MSG_DONTWAIT) != 0) { - flags &= ~FLAG_MSG_DONTWAIT; - if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { - descriptor->socket->SetNonBlock(true); - } - } + const bool dont_wait = (flags & FLAG_MSG_DONTWAIT) != 0 && SetDontWait(fd, *descriptor, true); + flags &= ~FLAG_MSG_DONTWAIT; const auto [ret, bsd_errno] = Translate(descriptor->socket->Recv(flags, message)); // Restore original state - if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { - descriptor->socket->SetNonBlock(false); + if (dont_wait) { + SetDontWait(fd, *descriptor, false); } return {ret, bsd_errno}; @@ -916,20 +913,15 @@ std::pair BSD_USA::RecvFromImpl(s32 fd, u32 flags, std::vector& // Apply flags using Network::FLAG_MSG_DONTWAIT; - using Network::FLAG_O_NONBLOCK; - if ((flags & FLAG_MSG_DONTWAIT) != 0) { - flags &= ~FLAG_MSG_DONTWAIT; - if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { - descriptor->socket->SetNonBlock(true); - } - } + const bool dont_wait = (flags & FLAG_MSG_DONTWAIT) != 0 && SetDontWait(fd, *descriptor, true); + flags &= ~FLAG_MSG_DONTWAIT; const auto [ret, bsd_errno] = Translate(descriptor->socket->RecvFrom(flags, message, p_addr_in)); // Restore original state - if ((descriptor->flags & FLAG_O_NONBLOCK) == 0) { - descriptor->socket->SetNonBlock(false); + if (dont_wait) { + SetDontWait(fd, *descriptor, false); } if (p_addr_in) { @@ -1043,8 +1035,21 @@ s32 BSD_USA::FindFreeFileDescriptorHandle() noexcept { return -1; } +bool BSD_USA::SetDontWait(s32 fd, const FileDescriptor& descriptor, bool enable) { + // Read the current flags, not the copy: fcntl may have changed them during the receive + std::scoped_lock lock{file_descriptors_mutex}; + const auto& entry = file_descriptors[fd]; + const s32 current_flags = + entry && entry->socket == descriptor.socket ? entry->flags : descriptor.flags; + if ((current_flags & Network::FLAG_O_NONBLOCK) != 0) { + return false; + } + descriptor.socket->SetNonBlock(enable); + return true; +} + bool BSD_USA::IsFileDescriptorValid(s32 fd) const noexcept { - if (fd > static_cast(MAX_FD) || fd < 0) { + if (fd >= static_cast(MAX_FD) || fd < 0) { LOG_ERROR(Service, "Invalid file descriptor handle={}", fd); return false; } @@ -1064,22 +1069,37 @@ void BSD_USA::BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const } void BSD_USA::OnProxyPacketReceived(const Network::ProxyPacket& packet) { - // Runs on the room thread. The lock keeps a socket alive while a bsd thread closes it. - std::scoped_lock lock{file_descriptors_mutex}; - // A duplicated fd shares its socket, which must still get each packet only once - std::array delivered{}; - size_t num_delivered = 0; - for (auto& optional_descriptor : file_descriptors) { - if (!optional_descriptor.has_value() || !optional_descriptor->socket) { - continue; - } - Network::SocketBase* const socket = optional_descriptor->socket.get(); - const auto delivered_end = delivered.begin() + num_delivered; - if (std::find(delivered.begin(), delivered_end, socket) != delivered_end) { - continue; + // Runs on the room thread. The copies keep each socket alive if a bsd thread closes it, so + // the bsd threads do not wait for the decompression below. + std::array, MAX_FD> sockets{}; + size_t num_sockets = 0; + { + std::scoped_lock lock{file_descriptors_mutex}; + for (const auto& descriptor : file_descriptors) { + if (!descriptor.has_value() || !descriptor->socket) { + continue; + } + // A duplicated fd shares its socket, which must still get each packet only once + const auto sockets_end = sockets.begin() + num_sockets; + if (std::find(sockets.begin(), sockets_end, descriptor->socket) == sockets_end) { + sockets[num_sockets++] = descriptor->socket; + } } - delivered[num_delivered++] = socket; - socket->HandleProxyPacket(packet); + } + if (num_sockets == 0) { + return; + } + + // Decompress once for all sockets that share the port + const Network::ProxyPacket decompressed{ + .local_endpoint = packet.local_endpoint, + .remote_endpoint = packet.remote_endpoint, + .protocol = packet.protocol, + .broadcast = packet.broadcast, + .data = Common::Compression::DecompressDataZSTD(packet.data), + }; + for (size_t i = 0; i < num_sockets; ++i) { + sockets[i]->HandleProxyPacket(decompressed); } } diff --git a/src/core/hle/service/sockets/bsd.h b/src/core/hle/service/sockets/bsd.h index f908edc7d2..559830d7a1 100644 --- a/src/core/hle/service/sockets/bsd.h +++ b/src/core/hle/service/sockets/bsd.h @@ -180,6 +180,8 @@ private: bool IsFileDescriptorValid(s32 fd) const noexcept; /// Copies a descriptor under file_descriptors_mutex; nullopt if fd has no socket std::optional CopyDescriptor(s32 fd); + /// Switches a blocking socket for one MSG_DONTWAIT call; false if fd is already nonblocking + bool SetDontWait(s32 fd, const FileDescriptor& descriptor, bool enable); void BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const noexcept; diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index 1b93ce93e5..95f64b9bd0 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -43,22 +43,7 @@ void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { LOG_INFO(Network, "Received broadcast packet, but not configured for broadcast mode"); return; } - } - - // Decompress without the lock, so the game thread does not wait for it - ProxyPacket decompressed{ - .local_endpoint = packet.local_endpoint, - .remote_endpoint = packet.remote_endpoint, - .protocol = packet.protocol, - .broadcast = packet.broadcast, - .data = Common::Compression::DecompressDataZSTD(packet.data), - }; - { - std::lock_guard guard(packets_mutex); - if (closed) { - return; - } - received_packets.push(std::move(decompressed)); + received_packets.push(packet); } // A MSG_PEEK reader leaves the packet queued, so every waiter must see it packets_cv.notify_all(); @@ -304,11 +289,15 @@ Errno ProxySocket::SetSndTimeo(u32 value) { } Errno ProxySocket::SetRcvTimeo(u32 value) { - receive_timeout = value; + { + std::lock_guard guard(packets_mutex); + receive_timeout = value; + } return SetSockOpt(fd, SO_RCVTIMEO, static_cast(value)); } Errno ProxySocket::SetNonBlock(bool enable) { + std::lock_guard guard(packets_mutex); blocking = !enable; return Errno::SUCCESS; } diff --git a/src/core/internal_network/sockets.h b/src/core/internal_network/sockets.h index 0b751b6622..37cffd4651 100644 --- a/src/core/internal_network/sockets.h +++ b/src/core/internal_network/sockets.h @@ -89,6 +89,7 @@ public: virtual bool IsOpened() const = 0; + /// Queues a room packet whose data is already decompressed virtual void HandleProxyPacket(const ProxyPacket& packet) = 0; [[nodiscard]] SOCKET GetFD() const { -- cgit v1.2.3 From fdeecea52df593d6d2a1ce6e1977e3aab1bb9135 Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 01:15:30 +0100 Subject: tests: queue decompressed packets in the room socket tests HandleProxyPacket now receives data that bsd has already decompressed. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/tests/core/internal_network/socket_proxy.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tests/core/internal_network/socket_proxy.cpp b/src/tests/core/internal_network/socket_proxy.cpp index d97d7d5102..0ba5f2aa2b 100644 --- a/src/tests/core/internal_network/socket_proxy.cpp +++ b/src/tests/core/internal_network/socket_proxy.cpp @@ -8,7 +8,6 @@ #include #include "common/socket_types.h" -#include "common/zstd_compression.h" #include "core/internal_network/socket_proxy.h" namespace { @@ -33,7 +32,7 @@ void QueuePacket(Network::ProxySocket& socket, Network::Protocol protocol, .ip = {192, 168, 1, 1}, .portno = 35000}; packet.protocol = protocol; - packet.data = Common::Compression::CompressDataZSTDDefault(payload.data(), payload.size()); + packet.data.assign(payload.begin(), payload.end()); socket.HandleProxyPacket(packet); } -- cgit v1.2.3 From 0b927d171c6d918a675c25f57b2bc0121b7f889d Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 01:39:54 +0100 Subject: network: close each bsd fd once and drop unwanted or malformed room packets Two threads closing the same fd both copied the descriptor and both called Close(), so a host socket could be closed twice, and the second closesocket could hit an unrelated socket that had reused the host fd. CloseImpl now frees the slot under file_descriptors_mutex first and only that thread closes the socket; the other gets EBADF. OnProxyPacketReceived decompressed every room packet when any fd had a socket, including native sockets and proxy sockets on other ports. Sockets now report through AcceptsProxyPacket whether they want a packet, and only those are collected, so an unwanted packet is not decompressed. DecompressDataZSTD passed the frame's declared size straight to the vector constructor, so a malformed or unsized frame from a room member threw on the room thread. The new TryDecompressDataZSTD rejects invalid, unsized and oversized frames, and bsd drops such packets with a warning. DecompressDataZSTD now returns an empty vector for them instead of throwing. Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/common/zstd_compression.cpp | 20 +++++++++---- src/common/zstd_compression.h | 13 +++++++++ src/core/hle/service/sockets/bsd.cpp | 46 ++++++++++++++++++------------ src/core/hle/service/sockets/bsd.h | 2 ++ src/core/internal_network/network.cpp | 4 +++ src/core/internal_network/socket_proxy.cpp | 28 ++++++++++++------ src/core/internal_network/socket_proxy.h | 5 ++++ src/core/internal_network/sockets.h | 5 ++++ 8 files changed, 91 insertions(+), 32 deletions(-) diff --git a/src/common/zstd_compression.cpp b/src/common/zstd_compression.cpp index 7665482ee6..9bb8d7b919 100644 --- a/src/common/zstd_compression.cpp +++ b/src/common/zstd_compression.cpp @@ -35,16 +35,26 @@ std::vector CompressDataZSTDDefault(const u8* source, std::size_t source_siz } std::vector DecompressDataZSTD(std::span compressed) { - const std::size_t decompressed_size = + return TryDecompressDataZSTD(compressed, std::vector{}.max_size()) + .value_or(std::vector{}); +} + +std::optional> TryDecompressDataZSTD(std::span compressed, + std::size_t max_size) { + // The size comes from the frame header, so check it before allocating + const unsigned long long decompressed_size = ZSTD_getFrameContentSize(compressed.data(), compressed.size()); - std::vector decompressed(decompressed_size); + if (decompressed_size == ZSTD_CONTENTSIZE_ERROR || + decompressed_size == ZSTD_CONTENTSIZE_UNKNOWN || decompressed_size > max_size) { + return std::nullopt; + } + std::vector decompressed(static_cast(decompressed_size)); const std::size_t uncompressed_result_size = ZSTD_decompress( decompressed.data(), decompressed.size(), compressed.data(), compressed.size()); - if (decompressed_size != uncompressed_result_size || ZSTD_isError(uncompressed_result_size)) { - // Decompression failed - return {}; + if (ZSTD_isError(uncompressed_result_size) || uncompressed_result_size != decompressed_size) { + return std::nullopt; } return decompressed; } diff --git a/src/common/zstd_compression.h b/src/common/zstd_compression.h index 819d7d201a..c20d96adb0 100644 --- a/src/common/zstd_compression.h +++ b/src/common/zstd_compression.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include @@ -45,6 +46,18 @@ namespace Common::Compression { */ [[nodiscard]] std::vector DecompressDataZSTD(std::span compressed); +/** + * Decompresses a single Zstandard frame that declares its content size. + * + * @param compressed the compressed source memory region. + * @param max_size the largest content size to accept. + * + * @return the decompressed data, or nullopt if the frame is invalid, does not declare its size, + * or declares more than max_size bytes. + */ +[[nodiscard]] std::optional> TryDecompressDataZSTD(std::span compressed, + std::size_t max_size); + /** * Decompresses a single Zstandard frame into a caller-provided buffer of known size. * diff --git a/src/core/hle/service/sockets/bsd.cpp b/src/core/hle/service/sockets/bsd.cpp index e6d23b7bf6..730a7edcc1 100644 --- a/src/core/hle/service/sockets/bsd.cpp +++ b/src/core/hle/service/sockets/bsd.cpp @@ -965,24 +965,24 @@ std::pair BSD_USA::SendToImpl(s32 fd, u32 flags, std::span } Errno BSD_USA::CloseImpl(s32 fd) { - const auto descriptor = CopyDescriptor(fd); - if (!descriptor) { - return Errno::BADF; - } - - const Errno bsd_errno = Translate(descriptor->socket->Close()); - if (bsd_errno != Errno::SUCCESS) { - return bsd_errno; + std::shared_ptr socket; + { + // Free the slot first, so of two threads closing the same fd only one closes the socket. + // Like POSIX close(), the fd is released even if closing the socket fails. + std::scoped_lock lock{file_descriptors_mutex}; + if (!IsFileDescriptorValid(fd)) { + return Errno::BADF; + } + if (!file_descriptors[fd]->socket) { + LOG_WARNING(Service, "Uninitialized socket"); + return Errno::BADF; + } + socket = std::move(file_descriptors[fd]->socket); + file_descriptors[fd].reset(); } LOG_INFO(Service, "Close socket fd={}", fd); - - // Another thread may have closed this fd and reused the slot meanwhile - std::scoped_lock lock{file_descriptors_mutex}; - if (file_descriptors[fd] && file_descriptors[fd]->socket == descriptor->socket) { - file_descriptors[fd].reset(); - } - return bsd_errno; + return Translate(socket->Close()); } std::variant BSD_USA::DuplicateSocketImpl(s32 fd) { @@ -1070,7 +1070,8 @@ void BSD_USA::BuildErrnoResponse(HLERequestContext& ctx, Errno bsd_errno) const void BSD_USA::OnProxyPacketReceived(const Network::ProxyPacket& packet) { // Runs on the room thread. The copies keep each socket alive if a bsd thread closes it, so - // the bsd threads do not wait for the decompression below. + // the bsd threads do not wait for the decompression below. Only sockets that accept the + // packet are collected, so an unwanted packet is never decompressed. std::array, MAX_FD> sockets{}; size_t num_sockets = 0; { @@ -1081,7 +1082,8 @@ void BSD_USA::OnProxyPacketReceived(const Network::ProxyPacket& packet) { } // A duplicated fd shares its socket, which must still get each packet only once const auto sockets_end = sockets.begin() + num_sockets; - if (std::find(sockets.begin(), sockets_end, descriptor->socket) == sockets_end) { + if (std::find(sockets.begin(), sockets_end, descriptor->socket) == sockets_end && + descriptor->socket->AcceptsProxyPacket(packet)) { sockets[num_sockets++] = descriptor->socket; } } @@ -1090,13 +1092,19 @@ void BSD_USA::OnProxyPacketReceived(const Network::ProxyPacket& packet) { return; } - // Decompress once for all sockets that share the port + // Decompress once for all sockets that share the port. The room does not check the frame, so + // drop a malformed or oversized one instead of throwing on the room thread. + auto data = Common::Compression::TryDecompressDataZSTD(packet.data, MAX_ROOM_PACKET_SIZE); + if (!data) { + LOG_WARNING(Service, "Dropped a room packet with an invalid or oversized payload"); + return; + } const Network::ProxyPacket decompressed{ .local_endpoint = packet.local_endpoint, .remote_endpoint = packet.remote_endpoint, .protocol = packet.protocol, .broadcast = packet.broadcast, - .data = Common::Compression::DecompressDataZSTD(packet.data), + .data = std::move(*data), }; for (size_t i = 0; i < num_sockets; ++i) { sockets[i]->HandleProxyPacket(decompressed); diff --git a/src/core/hle/service/sockets/bsd.h b/src/core/hle/service/sockets/bsd.h index 559830d7a1..bff80d7c00 100644 --- a/src/core/hle/service/sockets/bsd.h +++ b/src/core/hle/service/sockets/bsd.h @@ -44,6 +44,8 @@ public: private: /// Maximum number of file descriptors static constexpr size_t MAX_FD = 128; + /// Largest decompressed room packet. A UDP datagram cannot be larger. + static constexpr size_t MAX_ROOM_PACKET_SIZE = 0x10000; struct FileDescriptor { std::shared_ptr socket; diff --git a/src/core/internal_network/network.cpp b/src/core/internal_network/network.cpp index d1c436aa08..806562076d 100644 --- a/src/core/internal_network/network.cpp +++ b/src/core/internal_network/network.cpp @@ -1095,6 +1095,10 @@ bool Socket::IsOpened() const { return fd != INVALID_SOCKET; } +bool Socket::AcceptsProxyPacket(const ProxyPacket& packet) { + return false; +} + void Socket::HandleProxyPacket(const ProxyPacket& packet) { LOG_WARNING(Network, "ProxyPacket received, but not in Proxy mode!"); } diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index 95f64b9bd0..f12891e8f5 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -30,17 +30,29 @@ ProxySocket::~ProxySocket() { fd = INVALID_SOCKET; } +bool ProxySocket::MatchesPacket(const ProxyPacket& packet) const { + if (protocol != packet.protocol || local_endpoint.portno != packet.remote_endpoint.portno || + closed) { + return false; + } + if (!broadcast && packet.broadcast) { + LOG_INFO(Network, "Received broadcast packet, but not configured for broadcast mode"); + return false; + } + return true; +} + +bool ProxySocket::AcceptsProxyPacket(const ProxyPacket& packet) { + std::lock_guard guard(packets_mutex); + return MatchesPacket(packet); +} + void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { { - // Runs on the room thread, while the game thread may bind, close or reconfigure the socket + // Runs on the room thread, while the game thread may bind, close or reconfigure the socket. + // Check again: the socket may have changed since AcceptsProxyPacket. std::lock_guard guard(packets_mutex); - if (protocol != packet.protocol || local_endpoint.portno != packet.remote_endpoint.portno || - closed) { - return; - } - - if (!broadcast && packet.broadcast) { - LOG_INFO(Network, "Received broadcast packet, but not configured for broadcast mode"); + if (!MatchesPacket(packet)) { return; } received_packets.push(packet); diff --git a/src/core/internal_network/socket_proxy.h b/src/core/internal_network/socket_proxy.h index 5d6e35693c..078baa7806 100644 --- a/src/core/internal_network/socket_proxy.h +++ b/src/core/internal_network/socket_proxy.h @@ -22,6 +22,8 @@ public: explicit ProxySocket() noexcept; ~ProxySocket() override; + bool AcceptsProxyPacket(const ProxyPacket& packet) override; + void HandleProxyPacket(const ProxyPacket& packet) override; Errno Initialize(Domain domain, Type type, Protocol socket_protocol) override; @@ -82,6 +84,9 @@ public: bool IsOpened() const override; private: + /// Expects packets_mutex to be held + bool MatchesPacket(const ProxyPacket& packet) const; + bool broadcast = false; bool closed = false; u32 send_timeout = 0; diff --git a/src/core/internal_network/sockets.h b/src/core/internal_network/sockets.h index 37cffd4651..29b5b0f18a 100644 --- a/src/core/internal_network/sockets.h +++ b/src/core/internal_network/sockets.h @@ -89,6 +89,9 @@ public: virtual bool IsOpened() const = 0; + /// Whether HandleProxyPacket would queue the packet; checked before decompressing it + virtual bool AcceptsProxyPacket(const ProxyPacket& packet) = 0; + /// Queues a room packet whose data is already decompressed virtual void HandleProxyPacket(const ProxyPacket& packet) = 0; @@ -164,6 +167,8 @@ public: bool IsOpened() const override; + bool AcceptsProxyPacket(const ProxyPacket& packet) override; + void HandleProxyPacket(const ProxyPacket& packet) override; private: -- cgit v1.2.3 From d8d16713b891245fd786b27d375f68d695c76d0e Mon Sep 17 00:00:00 2001 From: cirakg Date: Tue, 3 Nov 2026 01:39:55 +0100 Subject: tests: cover room packet matching and size-checked ZSTD decompression Assisted-by: Claude Opus 5.5 (investigation and implementation) --- src/tests/CMakeLists.txt | 1 + src/tests/common/zstd_compression.cpp | 59 ++++++++++++++++++++++++ src/tests/core/internal_network/socket_proxy.cpp | 37 +++++++++++++-- 3 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 src/tests/common/zstd_compression.cpp diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index a31c073765..6bf14f4cbd 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -18,6 +18,7 @@ add_executable(tests common/slot_vector.cpp common/undefined_fix.cpp common/unique_function.cpp + common/zstd_compression.cpp core/core_timing.cpp core/ldn.cpp core/internal_network/socket_proxy.cpp diff --git a/src/tests/common/zstd_compression.cpp b/src/tests/common/zstd_compression.cpp new file mode 100644 index 0000000000..6996f2e1ac --- /dev/null +++ b/src/tests/common/zstd_compression.cpp @@ -0,0 +1,59 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include + +#include + +#include "common/common_types.h" +#include "common/zstd_compression.h" + +namespace { + +std::vector Compress(const std::vector& data) { + return Common::Compression::CompressDataZSTDDefault(data.data(), data.size()); +} + +} // namespace + +TEST_CASE("ZSTD frames within the size limit decompress", "[common][zstd]") { + const std::vector data(100, 0x5a); + const auto compressed = Compress(data); + + const auto result = Common::Compression::TryDecompressDataZSTD(compressed, data.size()); + REQUIRE(result.has_value()); + REQUIRE(*result == data); + REQUIRE(Common::Compression::DecompressDataZSTD(compressed) == data); + + const auto empty = Common::Compression::TryDecompressDataZSTD(Compress({}), 0); + REQUIRE(empty.has_value()); + REQUIRE(empty->empty()); +} + +TEST_CASE("ZSTD frames over the size limit are rejected", "[common][zstd]") { + const std::vector data(100, 0x5a); + const auto compressed = Compress(data); + REQUIRE_FALSE(Common::Compression::TryDecompressDataZSTD(compressed, data.size() - 1)); +} + +TEST_CASE("Invalid ZSTD frames are rejected without throwing", "[common][zstd]") { + const std::array one_byte{0x01}; + const std::array bad_magic{0x01, 0x02, 0x03, 0x04, 0x05}; + REQUIRE_FALSE(Common::Compression::TryDecompressDataZSTD(one_byte, 0x10000)); + REQUIRE_FALSE(Common::Compression::TryDecompressDataZSTD(bad_magic, 0x10000)); + REQUIRE(Common::Compression::DecompressDataZSTD(one_byte).empty()); + + // The header declares the size, but the data after it is cut off + std::vector truncated = Compress(std::vector(100, 0x5a)); + truncated.pop_back(); + REQUIRE_FALSE(Common::Compression::TryDecompressDataZSTD(truncated, 0x10000)); +} + +TEST_CASE("ZSTD frames without a declared size are rejected", "[common][zstd]") { + // Magic number, a frame header without the content size, then one raw 3-byte last block + const std::array frame{0x28, 0xb5, 0x2f, 0xfd, 0x00, 0x00, + 0x19, 0x00, 0x00, 'a', 'b', 'c'}; + REQUIRE_FALSE(Common::Compression::TryDecompressDataZSTD(frame, 0x10000)); + REQUIRE(Common::Compression::DecompressDataZSTD(frame).empty()); +} diff --git a/src/tests/core/internal_network/socket_proxy.cpp b/src/tests/core/internal_network/socket_proxy.cpp index 0ba5f2aa2b..137de5b421 100644 --- a/src/tests/core/internal_network/socket_proxy.cpp +++ b/src/tests/core/internal_network/socket_proxy.cpp @@ -21,9 +21,7 @@ void BindSocket(Network::ProxySocket& socket, Network::Protocol protocol) { .portno = 35000}) == Network::Errno::SUCCESS); } -template -void QueuePacket(Network::ProxySocket& socket, Network::Protocol protocol, - const std::array& payload) { +Network::ProxyPacket MakePacket(Network::Protocol protocol) { Network::ProxyPacket packet{}; packet.local_endpoint = {.family = Network::Domain::INET, .ip = {192, 168, 1, 2}, @@ -32,6 +30,13 @@ void QueuePacket(Network::ProxySocket& socket, Network::Protocol protocol, .ip = {192, 168, 1, 1}, .portno = 35000}; packet.protocol = protocol; + return packet; +} + +template +void QueuePacket(Network::ProxySocket& socket, Network::Protocol protocol, + const std::array& payload) { + Network::ProxyPacket packet = MakePacket(protocol); packet.data.assign(payload.begin(), payload.end()); socket.HandleProxyPacket(packet); } @@ -228,3 +233,29 @@ TEST_CASE("One room packet wakes every blocked peek reader", "[core][sockets]") REQUIRE(finished[i] - queued < std::chrono::milliseconds{1000}); } } + +TEST_CASE("Room sockets accept only packets for their port and protocol", "[core][sockets]") { + Network::ProxySocket socket; + BindSocket(socket, Network::Protocol::UDP); + + REQUIRE(socket.AcceptsProxyPacket(MakePacket(Network::Protocol::UDP))); + REQUIRE_FALSE(socket.AcceptsProxyPacket(MakePacket(Network::Protocol::TCP))); + + Network::ProxyPacket other_port = MakePacket(Network::Protocol::UDP); + other_port.remote_endpoint.portno = 35001; + REQUIRE_FALSE(socket.AcceptsProxyPacket(other_port)); + + Network::ProxyPacket broadcast = MakePacket(Network::Protocol::UDP); + broadcast.broadcast = true; + REQUIRE_FALSE(socket.AcceptsProxyPacket(broadcast)); + REQUIRE(socket.SetBroadcast(true) == Network::Errno::SUCCESS); + REQUIRE(socket.AcceptsProxyPacket(broadcast)); + + REQUIRE(socket.Close() == Network::Errno::SUCCESS); + REQUIRE_FALSE(socket.AcceptsProxyPacket(MakePacket(Network::Protocol::UDP))); +} + +TEST_CASE("Native sockets accept no room packets", "[core][sockets]") { + Network::Socket socket; + REQUIRE_FALSE(socket.AcceptsProxyPacket(MakePacket(Network::Protocol::UDP))); +} -- cgit v1.2.3