driver: socket: defer WskInit until sockets are actually created

It turns out that MINIPORT_INITIALIZE can actually delay system load,
and we currently have reports of people's systems hanging indefinitely.
WskCaptureNPI is known to deadlock if called too early in boot (say,
from DriverEntry of a PnP driver), but it was thought that
MINIPORT_INITIALIZE was sufficiently late that it was okay. Perhaps that
assumption is incorrect. In case it is, this patch moves WSK
initialization to when sockets are created, which always happens in the
context of a user thread, which naturally happens late in boot and can
block.

We know empirically that MINIPORT_INITIALIZE can block system boot, by
adding a `KeDelayExecutionThread(KernelMode, FALSE, &(LARGE_INTEGER){
.QuadPart = -SEC_TO_SYS_TIME_UNITS(300) });` to the top and noting that
boot takes 5 minutes longer. So the theory that the assumption is
incorrect is at least plausible.

All this commit does is move the call to WskInit() from InitializeEx in
device.c to SocketInit() in socket.c. The diff looks more verbose than
it is because making WskInit static and removing its forward declaration
required shuffling some functions around in socket.c, but no code
changed during that shuffle.

Reported-by: Oliver Freyermuth <freyermuth@physik.uni-bonn.de>
Reported-by: Joshua Sjoding <joshua.sjoding@scjalliance.com>
Reported-by: John-Paul Andreini <jandreini@geonerco.com>
Reported-by: Arlo Clauser <Arlo@starcubedesign.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

3 files changed, 69 insertions, 80 deletions

diff --git a/driver/device.c b/driver/device.c
index a7a14f9..2890e41 100644
--- a/driver/device.c
+++ b/driver/device.c
@@ -544,10 +544,7 @@ InitializeEx(
     NDIS_HANDLE MiniportDriverContext,
     PNDIS_MINIPORT_INIT_PARAMETERS MiniportInitParameters)
 {
-    NTSTATUS Status = WskInit();
-    if (!NT_SUCCESS(Status))
-        return Status;
-
+    NTSTATUS Status;
     WG_DEVICE *Wg = MemAllocateAndZero(sizeof(*Wg));
     if (!Wg)
         return NDIS_STATUS_RESOURCES;
diff --git a/driver/socket.c b/driver/socket.c
index 2a56196..cd6ee1e 100644
--- a/driver/socket.c
+++ b/driver/socket.c
@@ -828,67 +828,6 @@ cleanupSocket:
     return Status;
 }
 
-_Use_decl_annotations_
-NTSTATUS
-SocketInit(WG_DEVICE *Wg, UINT16 Port)
-{
-    NTSTATUS Status;
-    SOCKADDR_IN Sa4 = { .sin_family = AF_INET, .sin_addr.s_addr = Htonl(INADDR_ANY), .sin_port = Htons(Port) };
-    SOCKADDR_IN6 Sa6 = { .sin6_family = AF_INET6, .sin6_addr = IN6ADDR_ANY_INIT };
-    SOCKET *New4 = NULL, *New6 = NULL;
-    LONG Retries = 0;
-
-retry:
-    if (WskHasIpv4Transport)
-    {
-        Status = CreateAndBindSocket(Wg, (SOCKADDR *)&Sa4, &New4);
-        if (!NT_SUCCESS(Status))
-            goto out;
-    }
-
-    if (WskHasIpv6Transport)
-    {
-        Sa6.sin6_port = Sa4.sin_port;
-        Status = CreateAndBindSocket(Wg, (SOCKADDR *)&Sa6, &New6);
-        if (!NT_SUCCESS(Status))
-        {
-            CloseSocket(New4);
-            New4 = NULL;
-            if (Status == STATUS_ADDRESS_ALREADY_EXISTS && !Port && Retries++ < 100)
-                goto retry;
-            goto out;
-        }
-    }
-
-    SocketReinit(
-        Wg,
-        New4,
-        New6,
-        WskHasIpv4Transport   ? Ntohs(Sa4.sin_port)
-        : WskHasIpv6Transport ? Ntohs(Sa6.sin6_port)
-                              : Port);
-    Status = STATUS_SUCCESS;
-out:
-    return Status;
-}
-
-_Use_decl_annotations_
-VOID
-SocketReinit(WG_DEVICE *Wg, SOCKET *New4, SOCKET *New6, UINT16 Port)
-{
-    MuAcquirePushLockExclusive(&Wg->SocketUpdateLock);
-    SOCKET *Old4 = RcuDereferenceProtected(SOCKET, Wg->Sock4, &Wg->SocketUpdateLock);
-    SOCKET *Old6 = RcuDereferenceProtected(SOCKET, Wg->Sock6, &Wg->SocketUpdateLock);
-    RcuAssignPointer(Wg->Sock4, New4);
-    RcuAssignPointer(Wg->Sock6, New6);
-    if (New4 || New6)
-        Wg->IncomingPort = Port;
-    MuReleasePushLockExclusive(&Wg->SocketUpdateLock);
-    RcuSynchronize();
-    CloseSocket(Old4);
-    CloseSocket(Old6);
-}
-
 static VOID
 RouteNotification(
     _In_ VOID *CallerContext,
@@ -898,9 +837,8 @@ RouteNotification(
     InterlockedAdd((LONG *)CallerContext, 2);
 }
 
-_Use_decl_annotations_
-NTSTATUS
-WskInit(VOID)
+_IRQL_requires_max_(PASSIVE_LEVEL)
+static NTSTATUS WskInit(VOID)
 {
     NTSTATUS Status = ReadNoFence(&WskInitStatus);
     if (Status != STATUS_RETRY)
@@ -987,14 +925,7 @@ WskInit(VOID)
     /* Ignore return value, as MSDN says eventually this will be removed. */
     ULONG NoTdi = WSK_TDI_BEHAVIOR_BYPASS_TDI;
     WskProviderNpi.Dispatch->WskControlClient(
-        WskProviderNpi.Client,
-        WSK_TDI_BEHAVIOR,
-        sizeof(NoTdi),
-        &NoTdi,
-        0,
-        NULL,
-        NULL,
-        NULL);
+        WskProviderNpi.Client, WSK_TDI_BEHAVIOR, sizeof(NoTdi), &NoTdi, 0, NULL, NULL, NULL);
 
     Status = NotifyRouteChange2(AF_INET, RouteNotification, &RoutingGenerationV4, FALSE, &RouteNotifierV4);
     if (!NT_SUCCESS(Status))
@@ -1035,3 +966,68 @@ VOID WskUnload(VOID)
 out:
     MuReleasePushLockExclusive(&WskIsIniting);
 }
+
+_Use_decl_annotations_
+NTSTATUS
+SocketInit(WG_DEVICE *Wg, UINT16 Port)
+{
+    NTSTATUS Status;
+    SOCKADDR_IN Sa4 = { .sin_family = AF_INET, .sin_addr.s_addr = Htonl(INADDR_ANY), .sin_port = Htons(Port) };
+    SOCKADDR_IN6 Sa6 = { .sin6_family = AF_INET6, .sin6_addr = IN6ADDR_ANY_INIT };
+    SOCKET *New4 = NULL, *New6 = NULL;
+    LONG Retries = 0;
+
+    Status = WskInit();
+    if (!NT_SUCCESS(Status))
+        goto out;
+
+retry:
+    if (WskHasIpv4Transport)
+    {
+        Status = CreateAndBindSocket(Wg, (SOCKADDR *)&Sa4, &New4);
+        if (!NT_SUCCESS(Status))
+            goto out;
+    }
+
+    if (WskHasIpv6Transport)
+    {
+        Sa6.sin6_port = Sa4.sin_port;
+        Status = CreateAndBindSocket(Wg, (SOCKADDR *)&Sa6, &New6);
+        if (!NT_SUCCESS(Status))
+        {
+            CloseSocket(New4);
+            New4 = NULL;
+            if (Status == STATUS_ADDRESS_ALREADY_EXISTS && !Port && Retries++ < 100)
+                goto retry;
+            goto out;
+        }
+    }
+
+    SocketReinit(
+        Wg,
+        New4,
+        New6,
+        WskHasIpv4Transport   ? Ntohs(Sa4.sin_port)
+        : WskHasIpv6Transport ? Ntohs(Sa6.sin6_port)
+                              : Port);
+    Status = STATUS_SUCCESS;
+out:
+    return Status;
+}
+
+_Use_decl_annotations_
+VOID
+SocketReinit(WG_DEVICE *Wg, SOCKET *New4, SOCKET *New6, UINT16 Port)
+{
+    MuAcquirePushLockExclusive(&Wg->SocketUpdateLock);
+    SOCKET *Old4 = RcuDereferenceProtected(SOCKET, Wg->Sock4, &Wg->SocketUpdateLock);
+    SOCKET *Old6 = RcuDereferenceProtected(SOCKET, Wg->Sock6, &Wg->SocketUpdateLock);
+    RcuAssignPointer(Wg->Sock4, New4);
+    RcuAssignPointer(Wg->Sock6, New6);
+    if (New4 || New6)
+        Wg->IncomingPort = Port;
+    MuReleasePushLockExclusive(&Wg->SocketUpdateLock);
+    RcuSynchronize();
+    CloseSocket(Old4);
+    CloseSocket(Old6);
+}
diff --git a/driver/socket.h b/driver/socket.h
index 223c3ec..f7a3bf3 100644
--- a/driver/socket.h
+++ b/driver/socket.h
@@ -60,8 +60,4 @@ SocketReinit(
     _In_ UINT16 Port);
 
 _IRQL_requires_max_(PASSIVE_LEVEL)
-NTSTATUS
-WskInit(VOID);
-
-_IRQL_requires_max_(PASSIVE_LEVEL)
 VOID WskUnload(VOID);