From 455209b43eee5cdd7c33012493e8e8789def1dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 26 Oct 2026 21:44:09 +0100 Subject: [PATCH] Net: Fix use-after-pop, the resolver after loads, inet sockaddr/poll - The apctl, NpAuth and NpMatching2 event loops kept references to the front of their deque after popping it, so callbacks could get the next event's arguments. NpMatching2 also dereferenced its optional opt param when it was null. - The resolver was left stopped after any savestate load or reboot, which broke Outrun 2006 (it relies on it starting out running). - Getpeername/Getsockname with a namelen under 2 copied a negative size. They now write the PSP's 16-byte sockaddr truncated to namelen, with sa_len 16, as pspautotests net/inet/poll shows on hardware. - Poll handed an fd that isn't open (-1) to FD_SET. It now reports POLLNVAL for it and counts it, as on hardware (net/inet/poll). Setsockopt read past a local u32 for larger options, and passed a negative optlen to the host. A zero cmsg_len looped forever. Co-Authored-By: Claude Opus 5.5 --- Core/HLE/sceNet.cpp | 3 +- Core/HLE/sceNetInet.cpp | 82 +++++++++++++++++++++++-------------- Core/HLE/sceNetResolver.cpp | 6 ++- Core/HLE/sceNp.cpp | 3 +- Core/HLE/sceNp2.cpp | 15 ++++--- pspautotests | 2 +- test.py | 1 + 7 files changed, 71 insertions(+), 41 deletions(-) diff --git a/Core/HLE/sceNet.cpp b/Core/HLE/sceNet.cpp index 583a74e77c..3370769f86 100644 --- a/Core/HLE/sceNet.cpp +++ b/Core/HLE/sceNet.cpp @@ -812,7 +812,8 @@ void __NetApctlCallbacks() // How AP works probably like this: Game use sceNetApctl function -> sceNetApctl let the hardware know and do their's thing and have a new State -> Let the game know the resulting State through Event on their handler if (!apctlEvents.empty()) { - auto& args = apctlEvents.front(); + // Copy it, the pop below destroys the element. + auto args = apctlEvents.front(); auto& oldState = args.data[0]; auto& newState = args.data[1]; auto& event = args.data[2]; diff --git a/Core/HLE/sceNetInet.cpp b/Core/HLE/sceNetInet.cpp index 3018ba868f..06a8953c49 100644 --- a/Core/HLE/sceNetInet.cpp +++ b/Core/HLE/sceNetInet.cpp @@ -154,8 +154,23 @@ static u32_le sceNetInetInetAddr(const char *hostname) { return hleLogDebug(Log::sceNet, retval); } +// The PSP fills in a whole SceNetInetSockaddr (sa_len 16) and copies as much of it as namelen +// allows, setting namelen to the bytes copied (pspautotests net/inet/poll). +static void WriteSockaddrToPSP(const SockAddrIN4 &saddr, u32 namePtr, u32 namelenPtr) { + SceNetInetSockaddr name{}; + name.sa_len = (u8)sizeof(name); + name.sa_family = saddr.addr.sa_family; + memcpy(name.sa_data, saddr.addr.sa_data, sizeof(name.sa_data)); + const s32 namelen = (s32)Memory::ReadUnchecked_U32(namelenPtr); + const u32 copyLen = (u32)std::clamp(namelen, 0, (s32)sizeof(name)); + if (Memory::IsValidRange(namePtr, copyLen)) { + Memory::Memcpy(namePtr, &name, copyLen, "InetSockaddr"); + } + Memory::WriteUnchecked_U32(copyLen, namelenPtr); +} + static int sceNetInetGetpeername(int socket, u32 namePtr, u32 namelenPtr) { - if (!Memory::IsValidAddress(namePtr) || !Memory::IsValidAddress(namelenPtr)) { + if (!Memory::IsValidAddress(namePtr) || !Memory::IsValidRange(namelenPtr, 4)) { UpdateErrnoFromHost(__KernelGetCurThread(), EFAULT, __FUNCTION__); return hleLogError(Log::sceNet, -1, "invalid arg"); } @@ -165,31 +180,21 @@ static int sceNetInetGetpeername(int socket, u32 namePtr, u32 namelenPtr) { return hleLogError(Log::sceNet, ERROR_INET_EBADF, "Bad socket #%d", socket); } - SceNetInetSockaddr* name = (SceNetInetSockaddr*)Memory::GetPointerOrException(namePtr); - int* namelen = (int*)Memory::GetPointerOrException(namelenPtr); SockAddrIN4 saddr{}; - // TODO: Should've created convertSockaddrPSP2Host (and Host2PSP too) function as it's being used pretty often, thus fixing a bug on it will be tedious when scattered all over the places - saddr.addr.sa_family = name->sa_family; - int len = std::min(*namelen > 0 ? *namelen : 0, static_cast(sizeof(saddr))); - name->sa_len = len; - memcpy(saddr.addr.sa_data, name->sa_data, sizeof(name->sa_data)); - - int retval = getpeername(inetSock->sock, (sockaddr*)&saddr, (socklen_t*)&len); + socklen_t len = sizeof(saddr); + int retval = getpeername(inetSock->sock, (sockaddr*)&saddr, &len); DEBUG_LOG(Log::sceNet, "Getpeername: Family = %s, Address = %s, Port = %d", inetSocketDomain2str(saddr.addr.sa_family).c_str(), ip2str(saddr.in.sin_addr).c_str(), ntohs(saddr.in.sin_port)); - *namelen = len; if (retval < 0) { UpdateErrnoFromHost(__KernelGetCurThread(), socket_errno, __FUNCTION__); return hleLogError(Log::sceNet, retval); } - // We shouldn't use the returned len here, because the returned len is the actual size needed, which can be larger than the inputted len - memcpy(name->sa_data, saddr.addr.sa_data, name->sa_len - (sizeof(name->sa_len) + sizeof(name->sa_family))); - name->sa_family = saddr.addr.sa_family; + WriteSockaddrToPSP(saddr, namePtr, namelenPtr); return hleLogInfo(Log::sceNet, 0); } static int sceNetInetGetsockname(int socket, u32 namePtr, u32 namelenPtr) { - if (!Memory::IsValidAddress(namePtr) || !Memory::IsValidAddress(namelenPtr)) { + if (!Memory::IsValidAddress(namePtr) || !Memory::IsValidRange(namelenPtr, 4)) { UpdateErrnoFromHost(__KernelGetCurThread(), EFAULT, __FUNCTION__); return hleLogError(Log::sceNet, -1, "invalid arg"); } @@ -199,25 +204,16 @@ static int sceNetInetGetsockname(int socket, u32 namePtr, u32 namelenPtr) { return hleLogError(Log::sceNet, ERROR_INET_EBADF, "Bad socket #%d", socket); } - SceNetInetSockaddr* name = (SceNetInetSockaddr*)Memory::GetPointerOrException(namePtr); - int* namelen = (int*)Memory::GetPointerOrException(namelenPtr); SockAddrIN4 saddr{}; - saddr.addr.sa_family = name->sa_family; - int len = std::min(*namelen > 0 ? *namelen : 0, static_cast(sizeof(saddr))); - name->sa_len = len; - memcpy(saddr.addr.sa_data, name->sa_data, sizeof(name->sa_data)); - - int retval = getsockname(inetSock->sock, (sockaddr*)&saddr, (socklen_t*)&len); + socklen_t len = sizeof(saddr); + int retval = getsockname(inetSock->sock, (sockaddr*)&saddr, &len); DEBUG_LOG(Log::sceNet, "Getsockname: Family = %s, Address = %s, Port = %d", inetSocketDomain2str(saddr.addr.sa_family).c_str(), ip2str(saddr.in.sin_addr).c_str(), ntohs(saddr.in.sin_port)); - *namelen = len; if (retval < 0) { UpdateErrnoFromHost(__KernelGetCurThread(), socket_errno, __FUNCTION__); return hleLogError(Log::sceNet, retval); } - // We shouldn't use the returned len here, because the returned len is the actual size needed, which can be larger than the inputted len - memcpy(name->sa_data, saddr.addr.sa_data, name->sa_len - (sizeof(name->sa_len) + sizeof(name->sa_family))); - name->sa_family = saddr.addr.sa_family; + WriteSockaddrToPSP(saddr, namePtr, namelenPtr); return hleLogInfo(Log::sceNet, 0); } @@ -370,7 +366,13 @@ int sceNetInetPoll(MIPSState *mips, u32 fdsPtr, u32 nfds, int timeout) { // time UpdateErrnoFromHost(__KernelGetCurThread(), EINVAL, __FUNCTION__); return hleLogError(Log::sceNet, -1, "invalid socket id"); } - SOCKET hostSocket = g_socketManager.GetHostSocketFromInetSocket(fdarray[i].fd); + fdarray[i].revents = 0; + InetSocket *inetSock; + if (!g_socketManager.GetInetSocket(fdarray[i].fd, &inetSock)) { + // Not open, reported as POLLNVAL below. + continue; + } + SOCKET hostSocket = inetSock->sock; if (hostSocket > maxHostFd) { maxHostFd = hostSocket; } @@ -378,7 +380,6 @@ int sceNetInetPoll(MIPSState *mips, u32 fdsPtr, u32 nfds, int timeout) { // time FD_SET(hostSocket, &readfds); FD_SET(hostSocket, &writefds); FD_SET(hostSocket, &exceptfds); - fdarray[i].revents = 0; } timeval tmout = { 5, 543210 }; // Workaround timeout value when timeout = NULL @@ -395,7 +396,14 @@ int sceNetInetPoll(MIPSState *mips, u32 fdsPtr, u32 nfds, int timeout) { // time retval = 0; for (int i = 0; i < (s32)nfds; i++) { - SOCKET hostSocket = g_socketManager.GetHostSocketFromInetSocket(fdarray[i].fd); + InetSocket *inetSock; + if (!g_socketManager.GetInetSocket(fdarray[i].fd, &inetSock)) { + // As on hardware (pspautotests net/inet/poll). + fdarray[i].revents = INET_POLLNVAL; + retval++; + continue; + } + SOCKET hostSocket = inetSock->sock; if ((fdarray[i].events & (INET_POLLRDNORM | INET_POLLIN)) && FD_ISSET(hostSocket, &readfds)) fdarray[i].revents |= (INET_POLLRDNORM | INET_POLLIN); //POLLIN_SET if ((fdarray[i].events & (INET_POLLWRNORM | INET_POLLOUT)) && FD_ISSET(hostSocket, &writefds)) @@ -543,8 +551,15 @@ static int sceNetInetSetsockopt(MIPSState *mips, int socket, int level, int optn tval.tv_sec = optval / 1000000; // seconds tval.tv_usec = (optval % 1000000); // microseconds retval = setsockopt(inetSock->sock, convertSockoptLevelPSP2Host(level), convertSockoptNamePSP2Host(optname, level), (char*)&tval, sizeof(tval)); - } else { + } else if (optlen >= 0 && optlen <= (int)sizeof(optval)) { retval = setsockopt(inetSock->sock, convertSockoptLevelPSP2Host(level), convertSockoptNamePSP2Host(optname, level), (char*)&optval, optlen); + } else { + // Bigger options are passed through from the game's memory. + if (!Memory::IsValidRange(optvalPtr, optlen)) { + UpdateErrnoFromHost(__KernelGetCurThread(), EFAULT, __FUNCTION__); + return hleLogError(Log::sceNet, -1, "invalid arg"); + } + retval = setsockopt(inetSock->sock, convertSockoptLevelPSP2Host(level), convertSockoptNamePSP2Host(optname, level), (const char *)Memory::GetPointerUnchecked(optvalPtr), optlen); } if (retval < 0) { UpdateErrnoFromHost(__KernelGetCurThread(), socket_errno, __FUNCTION__); @@ -1055,6 +1070,8 @@ static int sceNetInetSendmsg(int socket, u32 msghdrPtr, int flags) { if (pspMsghdr->msg_controllen >= sizeof(InetCmsghdr)) { // TODO: Creates our own CMSG_* macros (32-bit version of it, similar to the one on PSP) to avoid alignment/size issue that can lead to memory corruption/out of bound issue. for (WSACMSGHDR* cmsgptr = WSA_CMSG_FIRSTHDR(&hdr); cmsgptr != NULL; cmsgptr = WSA_CMSG_NXTHDR(&hdr, cmsgptr)) { + if (cmsgptr->cmsg_len < sizeof(WSACMSGHDR)) + break; cmsgptr->cmsg_type = convertCMsgTypePSP2Host(cmsgptr->cmsg_type, cmsgptr->cmsg_level); cmsgptr->cmsg_level = convertSockoptLevelPSP2Host(cmsgptr->cmsg_level); } @@ -1065,6 +1082,9 @@ static int sceNetInetSendmsg(int socket, u32 msghdrPtr, int flags) { // Note: Many existing implementations of CMSG_FIRSTHDR never look at msg_controllen and just return the value of cmsg_control. if (pspMsghdr->msg_controllen >= sizeof(InetCmsghdr)) { for (cmsghdr* cmsgptr = CMSG_FIRSTHDR(&hdr); cmsgptr != NULL; cmsgptr = CMSG_NXTHDR(&hdr, cmsgptr)) { + // Some CMSG_NXTHDR implementations return the same header again for a zero length. + if (cmsgptr->cmsg_len < sizeof(cmsghdr)) + break; cmsgptr->cmsg_type = convertCMsgTypePSP2Host(cmsgptr->cmsg_type, cmsgptr->cmsg_level); cmsgptr->cmsg_level = convertSockoptLevelPSP2Host(cmsgptr->cmsg_level); } diff --git a/Core/HLE/sceNetResolver.cpp b/Core/HLE/sceNetResolver.cpp index e91a060eb7..cf2a78c903 100644 --- a/Core/HLE/sceNetResolver.cpp +++ b/Core/HLE/sceNetResolver.cpp @@ -71,13 +71,15 @@ static int sceNetResolverInit() { return hleLogInfo(Log::sceNet, 0); } +// Also called on savestate load, so go back to the boot state, where it's running. void __NetResolverShutdown() { g_netResolvers.clear(); - g_netResolverInitialized = false; + g_netResolverInitialized = true; } static int sceNetResolverTerm() { - __NetResolverShutdown(); + g_netResolvers.clear(); + g_netResolverInitialized = false; return hleLogInfo(Log::sceNet, 0); } diff --git a/Core/HLE/sceNp.cpp b/Core/HLE/sceNp.cpp index fc174bc697..822ca675fc 100644 --- a/Core/HLE/sceNp.cpp +++ b/Core/HLE/sceNp.cpp @@ -125,7 +125,8 @@ bool NpAuthProcessEvents() { return false; } - auto& args = npAuthEvents.front(); + // Copy it, the pop below destroys the element. + auto args = npAuthEvents.front(); auto& id = args.data[0]; auto& result = args.data[1]; auto& argAddr = args.data[2]; diff --git a/Core/HLE/sceNp2.cpp b/Core/HLE/sceNp2.cpp index b6059cbaea..b597440207 100644 --- a/Core/HLE/sceNp2.cpp +++ b/Core/HLE/sceNp2.cpp @@ -54,7 +54,8 @@ bool NpMatching2ProcessEvents() { return false; } - auto& args = npMatching2Events.front(); + // Copy it, the pop below destroys the element. + auto args = npMatching2Events.front(); auto& event = args.data[0]; auto& stat = args.data[1]; auto& serverIdPtr = args.data[2]; @@ -72,10 +73,14 @@ bool NpMatching2ProcessEvents() { } // Per npMatching2 function callback - u32* inStruct = (u32*)Memory::GetPointerOrException(inStructPtr); - if (Memory::IsValidAddress(inStruct[0])) { - DEBUG_LOG(Log::sceNet, "NpMatching2Callback [ServerID=%i][EventID=%04x][State=%04x][FuncAddr=%08x][ArgsPtr=%08x]", *(u32*)Memory::GetPointerOrException(serverIdPtr), event, stat, inStruct[0], inStruct[1]); - hleEnqueueCall(inStruct[0], 7, args.data); + // The opt param is optional. + if (Memory::IsValid4AlignedRange(inStructPtr, 8)) { + const u32 funcAddr = Memory::ReadUnchecked_U32(inStructPtr); + if (Memory::IsValidAddress(funcAddr)) { + const u32 serverId = Memory::IsValid4AlignedAddress(serverIdPtr) ? Memory::ReadUnchecked_U32(serverIdPtr) : 0; + DEBUG_LOG(Log::sceNet, "NpMatching2Callback [ServerID=%i][EventID=%04x][State=%04x][FuncAddr=%08x][ArgsPtr=%08x]", serverId, event, stat, funcAddr, Memory::ReadUnchecked_U32(inStructPtr + 4)); + hleEnqueueCall(funcAddr, 7, args.data); + } } return true; } diff --git a/pspautotests b/pspautotests index 88e64dd037..fab33ea469 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 88e64dd03767cd69e32e2197afb84f38cf7aba20 +Subproject commit fab33ea469ae6d82980064f3e29299e6920bf2e4 diff --git a/test.py b/test.py index 8ec831539f..f8cc384c5e 100755 --- a/test.py +++ b/test.py @@ -656,6 +656,7 @@ tests_next = [ "modules/unloadfill/unloadfill", "net/http/http", "net/inet/bind", + "net/inet/poll", "net/primary/ether", "sysmem/partition", # These two mbx tests only appeared to work because they papered over bugs