From ef842dda2a894c4c328a8f9ee2d7875c643a4716 Mon Sep 17 00:00:00 2001 From: TuxSH <1922548+TuxSH@users.noreply.github.com> Date: Sun, 10 Apr 2022 22:19:32 +0100 Subject: [PATCH] Fix extremely obnoxious race-cond+uninit handle bug This is mostly a libctru bug (well, sort of). This can only happen to built-in sysmodules, and to processes waiting for err:f, that fail to obtain handles through svcConnectToPort first try; and only prior to 11.0. Prior to fw 11.0, kernel didn't zero-initialize output handles, and thus the output handle gets filled with junk (leaked kernel stack data) in case of failure. Libctru does not account for this, and closes such handles anyway (in srvInit, errfInit, and possibly more). The problem is that, in our case, that garbage was equal to 0x8000, actually a valid handle, in fact the first handle to be created (and not closed) in a process... a handle to KAddressArbiter. Accidentally closing this handle resulted in one or more KIPs spin-waiting and starving core1, resulting in an inability to boot. We fix this simply by replicating what recent k11 does, in kext (for svcConnectToPort). For srvGetServiceHandle, add two layers of safety. --- k11_extension/source/svc/ConnectToPort.c | 5 +++++ k11_extension/source/svc/SendSyncRequest.c | 6 ++++++ sysmodules/sm/source/services.c | 7 +++++++ 3 files changed, 18 insertions(+) diff --git a/k11_extension/source/svc/ConnectToPort.c b/k11_extension/source/svc/ConnectToPort.c index 29eb155..ada157c 100644 --- a/k11_extension/source/svc/ConnectToPort.c +++ b/k11_extension/source/svc/ConnectToPort.c @@ -42,7 +42,12 @@ Result ConnectToPortHook(Handle *out, const char *name) } res = ConnectToPort(out, name); if(res != 0) + { + // Prior to 11.0 kernel didn't zero-initialize output handles, and thus + // you could accidentaly close things like the KAddressArbiter handle by mistake... + *out = 0; return res; + } KProcessHandleTable *handleTable = handleTableOfProcess(currentCoreContext->objectContext.currentProcess); KClientSession *clientSession = (KClientSession *)KProcessHandleTable__ToKAutoObject(handleTable, *out); diff --git a/k11_extension/source/svc/SendSyncRequest.c b/k11_extension/source/svc/SendSyncRequest.c index 507b32e..60d259d 100644 --- a/k11_extension/source/svc/SendSyncRequest.c +++ b/k11_extension/source/svc/SendSyncRequest.c @@ -126,6 +126,12 @@ Result SendSyncRequestHook(Handle handle) outClientSession->syncObject.autoObject.vtable->DecrementReferenceCount(&outClientSession->syncObject.autoObject); } } + else + { + // Prior to 11.0 kernel didn't zero-initialize output handles, and thus + // you could accidentaly close things like the KAddressArbiter handle by mistake... + cmdbuf[3] = 0; + } } break; diff --git a/sysmodules/sm/source/services.c b/sysmodules/sm/source/services.c index a81af7c..f2893d3 100644 --- a/sysmodules/sm/source/services.c +++ b/sysmodules/sm/source/services.c @@ -62,7 +62,10 @@ static Result doRegisterServiceOrPort(u32 pid, Handle *serverPort, Handle client { res = svcCreatePort(&portServer, &portClient, NULL, maxSessions); if(R_FAILED(res)) + { + *serverPort = 0; return 0xD9001BFC; + } } else portClient = clientPort; @@ -165,6 +168,7 @@ Result GetServiceHandle(SessionData *sessionData, Handle *session, const char *n { Result res = checkServiceName(name, nameSize); s32 serviceId; + *session = 0; if(R_FAILED(res)) return res; @@ -178,6 +182,8 @@ Result GetServiceHandle(SessionData *sessionData, Handle *session, const char *n { Handle port = servicesInfo[serviceId].clientPort; res = svcCreateSessionToPort(session, port); + if(R_FAILED(res)) + *session = 0; if(res == (Result)0xD0401834 && !(flags & 1)) { sessionData->busyClientPortHandle = port; @@ -192,6 +198,7 @@ Result GetPort(SessionData *sessionData, Handle *port, const char *name, s32 nam { Result res = checkServiceName(name, nameSize); s32 serviceId; + *port = 0; if(R_FAILED(res)) return res;