From 1dfbed9c3dbd135c7bf51090b9cadde6d2642340 Mon Sep 17 00:00:00 2001 From: Eric Kohl Date: Sat, 24 Feb 2018 11:14:05 +0100 Subject: [PATCH] [SERVICES][ADVAPI32] Fix the broken service stop sequence services\database.c: - Refactor ScmControlService() so that it can be used to send the dispatcher loop stop command. - Separate the code to decrement the image run counter from the service image cleanup code. services\rpcserver.c: - RSetServiceStatus(): Stop the dispatcher loop when the image run counter is zero and remove the service image after that. advapi32\service\sctrl.c: - Do not terminate the service dispatcher loop when the last service is being stopped. Wait for an explicit dispatcher stop command (empty service name). CORE-12413 --- base/system/services/database.c | 105 +++++++++++++++-------------- base/system/services/rpcserver.c | 28 +++++++- base/system/services/services.h | 5 +- dll/win32/advapi32/service/sctrl.c | 70 +++++++++---------- 4 files changed, 121 insertions(+), 87 deletions(-) diff --git a/base/system/services/database.c b/base/system/services/database.c index d31d353a87d..d28a98c0826 100644 --- a/base/system/services/database.c +++ b/base/system/services/database.c @@ -432,41 +432,34 @@ done: } -static VOID -ScmDereferenceServiceImage(PSERVICE_IMAGE pServiceImage) +VOID +ScmRemoveServiceImage(PSERVICE_IMAGE pServiceImage) { - DPRINT1("ScmDereferenceServiceImage() called\n"); + DPRINT1("ScmRemoveServiceImage() called\n"); - pServiceImage->dwImageRunCount--; + /* FIXME: Terminate the process */ - if (pServiceImage->dwImageRunCount == 0) - { - DPRINT1("dwImageRunCount == 0\n"); + /* Remove the service image from the list */ + RemoveEntryList(&pServiceImage->ImageListEntry); - /* FIXME: Terminate the process */ + /* Close the process handle */ + if (pServiceImage->hProcess != INVALID_HANDLE_VALUE) + CloseHandle(pServiceImage->hProcess); - /* Remove the service image from the list */ - RemoveEntryList(&pServiceImage->ImageListEntry); + /* Close the control pipe */ + if (pServiceImage->hControlPipe != INVALID_HANDLE_VALUE) + CloseHandle(pServiceImage->hControlPipe); - /* Close the process handle */ - if (pServiceImage->hProcess != INVALID_HANDLE_VALUE) - CloseHandle(pServiceImage->hProcess); + /* Unload the user profile */ + if (pServiceImage->hProfile != NULL) + UnloadUserProfile(pServiceImage->hToken, pServiceImage->hProfile); - /* Close the control pipe */ - if (pServiceImage->hControlPipe != INVALID_HANDLE_VALUE) - CloseHandle(pServiceImage->hControlPipe); + /* Close the logon token */ + if (pServiceImage->hToken != NULL) + CloseHandle(pServiceImage->hToken); - /* Unload the user profile */ - if (pServiceImage->hProfile != NULL) - UnloadUserProfile(pServiceImage->hToken, pServiceImage->hProfile); - - /* Close the logon token */ - if (pServiceImage->hToken != NULL) - CloseHandle(pServiceImage->hToken); - - /* Release the service image */ - HeapFree(GetProcessHeap(), 0, pServiceImage); - } + /* Release the service image */ + HeapFree(GetProcessHeap(), 0, pServiceImage); } @@ -618,7 +611,15 @@ ScmDeleteServiceRecord(PSERVICE lpService) /* Dereference the service image */ if (lpService->lpImage) - ScmDereferenceServiceImage(lpService->lpImage); + { + lpService->lpImage->dwImageRunCount--; + + if (lpService->lpImage->dwImageRunCount == 0) + { + ScmRemoveServiceImage(lpService->lpImage); + lpService->lpImage = NULL; + } + } /* Decrement the group reference counter */ ScmSetServiceGroup(lpService, NULL); @@ -1095,7 +1096,9 @@ ScmGetBootAndSystemDriverState(VOID) DWORD -ScmControlService(PSERVICE Service, +ScmControlService(HANDLE hControlPipe, + PWSTR pServiceName, + SERVICE_STATUS_HANDLE hServiceStatus, DWORD dwControl) { PSCM_CONTROL_PACKET ControlPacket; @@ -1116,7 +1119,7 @@ ScmControlService(PSERVICE Service, /* Calculate the total length of the start command line */ PacketSize = sizeof(SCM_CONTROL_PACKET); - PacketSize += (DWORD)((wcslen(Service->lpServiceName) + 1) * sizeof(WCHAR)); + PacketSize += (DWORD)((wcslen(pServiceName) + 1) * sizeof(WCHAR)); ControlPacket = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, @@ -1129,17 +1132,17 @@ ScmControlService(PSERVICE Service, ControlPacket->dwSize = PacketSize; ControlPacket->dwControl = dwControl; - ControlPacket->hServiceStatus = (SERVICE_STATUS_HANDLE)Service; + ControlPacket->hServiceStatus = hServiceStatus; ControlPacket->dwServiceNameOffset = sizeof(SCM_CONTROL_PACKET); Ptr = (PWSTR)((PBYTE)ControlPacket + ControlPacket->dwServiceNameOffset); - wcscpy(Ptr, Service->lpServiceName); + wcscpy(Ptr, pServiceName); ControlPacket->dwArgumentsCount = 0; ControlPacket->dwArgumentsOffset = 0; - bResult = WriteFile(Service->lpImage->hControlPipe, + bResult = WriteFile(hControlPipe, ControlPacket, PacketSize, &dwWriteCount, @@ -1153,13 +1156,13 @@ ScmControlService(PSERVICE Service, { DPRINT("dwError: ERROR_IO_PENDING\n"); - dwError = WaitForSingleObject(Service->lpImage->hControlPipe, + dwError = WaitForSingleObject(hControlPipe, PipeTimeout); DPRINT("WaitForSingleObject() returned %lu\n", dwError); if (dwError == WAIT_TIMEOUT) { - bResult = CancelIo(Service->lpImage->hControlPipe); + bResult = CancelIo(hControlPipe); if (bResult == FALSE) { DPRINT1("CancelIo() failed (Error: %lu)\n", GetLastError()); @@ -1170,7 +1173,7 @@ ScmControlService(PSERVICE Service, } else if (dwError == WAIT_OBJECT_0) { - bResult = GetOverlappedResult(Service->lpImage->hControlPipe, + bResult = GetOverlappedResult(hControlPipe, &Overlapped, &dwWriteCount, TRUE); @@ -1193,7 +1196,7 @@ ScmControlService(PSERVICE Service, /* Read the reply */ Overlapped.hEvent = (HANDLE) NULL; - bResult = ReadFile(Service->lpImage->hControlPipe, + bResult = ReadFile(hControlPipe, &ReplyPacket, sizeof(SCM_REPLY_PACKET), &dwReadCount, @@ -1207,13 +1210,13 @@ ScmControlService(PSERVICE Service, { DPRINT("dwError: ERROR_IO_PENDING\n"); - dwError = WaitForSingleObject(Service->lpImage->hControlPipe, + dwError = WaitForSingleObject(hControlPipe, PipeTimeout); DPRINT("WaitForSingleObject() returned %lu\n", dwError); if (dwError == WAIT_TIMEOUT) { - bResult = CancelIo(Service->lpImage->hControlPipe); + bResult = CancelIo(hControlPipe); if (bResult == FALSE) { DPRINT1("CancelIo() failed (Error: %lu)\n", GetLastError()); @@ -1224,7 +1227,7 @@ ScmControlService(PSERVICE Service, } else if (dwError == WAIT_OBJECT_0) { - bResult = GetOverlappedResult(Service->lpImage->hControlPipe, + bResult = GetOverlappedResult(hControlPipe, &Overlapped, &dwReadCount, TRUE); @@ -1255,13 +1258,6 @@ Done: dwError = ReplyPacket.dwError; } - if (dwError == ERROR_SUCCESS && - dwControl == SERVICE_CONTROL_STOP) - { - ScmDereferenceServiceImage(Service->lpImage); - Service->lpImage = NULL; - } - LeaveCriticalSection(&ControlServiceCriticalSection); DPRINT("ScmControlService() done\n"); @@ -1831,8 +1827,12 @@ ScmLoadService(PSERVICE Service, } else { - ScmDereferenceServiceImage(Service->lpImage); - Service->lpImage = NULL; + Service->lpImage->dwImageRunCount--; + if (Service->lpImage->dwImageRunCount == 0) + { + ScmRemoveServiceImage(Service->lpImage); + Service->lpImage = NULL; + } } } } @@ -2168,8 +2168,11 @@ ScmAutoShutdownServices(VOID) CurrentService->Status.dwCurrentState == SERVICE_START_PENDING) { /* shutdown service */ - DPRINT("Shutdown service: %S\n", CurrentService->szServiceName); - ScmControlService(CurrentService, SERVICE_CONTROL_SHUTDOWN); + DPRINT("Shutdown service: %S\n", CurrentService->lpServiceName); + ScmControlService(CurrentService->lpImage->hControlPipe, + CurrentService->lpServiceName, + (SERVICE_STATUS_HANDLE)CurrentService, + SERVICE_CONTROL_SHUTDOWN); } ServiceEntry = ServiceEntry->Flink; diff --git a/base/system/services/rpcserver.c b/base/system/services/rpcserver.c index a4718aa15f2..0a518d06adb 100644 --- a/base/system/services/rpcserver.c +++ b/base/system/services/rpcserver.c @@ -1228,7 +1228,9 @@ RControlService( } /* Send control code to the service */ - dwError = ScmControlService(lpService, + dwError = ScmControlService(lpService->lpImage->hControlPipe, + lpService->lpServiceName, + (SERVICE_STATUS_HANDLE)lpService, dwControl); /* Return service status information */ @@ -1725,6 +1727,28 @@ RSetServiceStatus( /* Restore the previous service type */ lpService->Status.dwServiceType = dwPreviousType; + /* Handle a stopped service */ + if ((lpServiceStatus->dwServiceType & SERVICE_WIN32) && + (lpServiceStatus->dwCurrentState == SERVICE_STOPPED)) + { + /* Decrement the image run counter */ + lpService->lpImage->dwImageRunCount--; + + /* If we just stopped the last running service... */ + if (lpService->lpImage->dwImageRunCount == 0) + { + /* Stop the dispatcher thread */ + ScmControlService(lpService->lpImage->hControlPipe, + L"", + (SERVICE_STATUS_HANDLE)lpService, + SERVICE_CONTROL_STOP); + + /* Remove the service image */ + ScmRemoveServiceImage(lpService->lpImage); + lpService->lpImage = NULL; + } + } + /* Unlock the service database */ ScmUnlockDatabase(); @@ -1773,6 +1797,8 @@ RSetServiceStatus( lpLogStrings); } + + DPRINT("Set %S to %lu\n", lpService->lpDisplayName, lpService->Status.dwCurrentState); DPRINT("RSetServiceStatus() done\n"); diff --git a/base/system/services/services.h b/base/system/services/services.h index 0f038499670..3592ba2560b 100644 --- a/base/system/services/services.h +++ b/base/system/services/services.h @@ -164,6 +164,7 @@ DWORD ScmStartService(PSERVICE Service, DWORD argc, LPWSTR *argv); +VOID ScmRemoveServiceImage(PSERVICE_IMAGE pServiceImage); PSERVICE ScmGetServiceEntryByName(LPCWSTR lpServiceName); PSERVICE ScmGetServiceEntryByDisplayName(LPCWSTR lpDisplayName); PSERVICE ScmGetServiceEntryByResumeCount(DWORD dwResumeCount); @@ -174,7 +175,9 @@ DWORD ScmCreateNewServiceRecord(LPCWSTR lpServiceName, VOID ScmDeleteServiceRecord(PSERVICE lpService); DWORD ScmMarkServiceForDelete(PSERVICE pService); -DWORD ScmControlService(PSERVICE Service, +DWORD ScmControlService(HANDLE hControlPipe, + PWSTR pServiceName, + SERVICE_STATUS_HANDLE hServiceStatus, DWORD dwControl); BOOL ScmLockDatabaseExclusive(VOID); diff --git a/dll/win32/advapi32/service/sctrl.c b/dll/win32/advapi32/service/sctrl.c index 4a0ae5665d9..fe2469732fe 100644 --- a/dll/win32/advapi32/service/sctrl.c +++ b/dll/win32/advapi32/service/sctrl.c @@ -544,7 +544,7 @@ ScServiceDispatcher(HANDLE hPipe, { DWORD Count; BOOL bResult; - DWORD dwRunningServices = 0; + BOOL bRunning = TRUE; LPWSTR lpServiceName; PACTIVE_SERVICE lpService; SCM_REPLY_PACKET ReplyPacket; @@ -555,7 +555,7 @@ ScServiceDispatcher(HANDLE hPipe, if (ControlPacket == NULL || dwBufferSize < sizeof(SCM_CONTROL_PACKET)) return FALSE; - while (TRUE) + while (bRunning) { /* Read command from the control pipe */ bResult = ReadFile(hPipe, @@ -572,39 +572,44 @@ ScServiceDispatcher(HANDLE hPipe, lpServiceName = (LPWSTR)((PBYTE)ControlPacket + ControlPacket->dwServiceNameOffset); TRACE("Service: %S\n", lpServiceName); - if (ControlPacket->dwControl == SERVICE_CONTROL_START_OWN) - lpActiveServices[0].bOwnProcess = TRUE; - - lpService = ScLookupServiceByServiceName(lpServiceName); - if (lpService != NULL) + if (lpServiceName[0] == UNICODE_NULL) { - /* Execute command */ - switch (ControlPacket->dwControl) - { - case SERVICE_CONTROL_START_SHARE: - case SERVICE_CONTROL_START_OWN: - TRACE("Start command - received SERVICE_CONTROL_START\n"); - dwError = ScStartService(lpService, ControlPacket); - if (dwError == ERROR_SUCCESS) - dwRunningServices++; - break; - - case SERVICE_CONTROL_STOP: - TRACE("Stop command - received SERVICE_CONTROL_STOP\n"); - dwError = ScControlService(lpService, ControlPacket); - if (dwError == ERROR_SUCCESS) - dwRunningServices--; - break; - - default: - TRACE("Command %lu received", ControlPacket->dwControl); - dwError = ScControlService(lpService, ControlPacket); - break; - } + ERR("Stop dispatcher thread\n"); + bRunning = FALSE; + dwError = ERROR_SUCCESS; } else { - dwError = ERROR_SERVICE_DOES_NOT_EXIST; + if (ControlPacket->dwControl == SERVICE_CONTROL_START_OWN) + lpActiveServices[0].bOwnProcess = TRUE; + + lpService = ScLookupServiceByServiceName(lpServiceName); + if (lpService != NULL) + { + /* Execute command */ + switch (ControlPacket->dwControl) + { + case SERVICE_CONTROL_START_SHARE: + case SERVICE_CONTROL_START_OWN: + TRACE("Start command - received SERVICE_CONTROL_START\n"); + dwError = ScStartService(lpService, ControlPacket); + break; + + case SERVICE_CONTROL_STOP: + TRACE("Stop command - received SERVICE_CONTROL_STOP\n"); + dwError = ScControlService(lpService, ControlPacket); + break; + + default: + TRACE("Command %lu received", ControlPacket->dwControl); + dwError = ScControlService(lpService, ControlPacket); + break; + } + } + else + { + dwError = ERROR_SERVICE_DOES_NOT_EXIST; + } } ReplyPacket.dwError = dwError; @@ -620,9 +625,6 @@ ScServiceDispatcher(HANDLE hPipe, ERR("Pipe write failed (Error: %lu)\n", GetLastError()); return FALSE; } - - if (dwRunningServices == 0) - break; } return TRUE;