[ros-diffs] [ekohl] 45711: [SERVICES] - Generate unique service status handles. Services could set the status information of another service because the status handles were not guaranteed to be unique for all services. - Lock and unlock the service database when getting or setting service status information.

Author: ekohl
Date: Sat Feb 27 22:47:59 2010
New Revision: 45711

URL: http://svn.reactos.org/svn/reactos?rev=45711&view=rev
Log:
[SERVICES]
- Generate unique service status handles. Services could set the status information of another service because the status handles were not guaranteed to be unique for all services.
- Lock and unlock the service database when getting or setting service status information.

Modified:
    trunk/reactos/base/system/services/database.c
    trunk/reactos/base/system/services/rpcserver.c
    trunk/reactos/base/system/services/services.h
    trunk/reactos/dll/win32/advapi32/service/sctrl.c
    trunk/reactos/include/reactos/services/services.h

Modified: trunk/reactos/base/system/services/database.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/databa...
==============================================================================
--- trunk/reactos/base/system/services/database.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/database.c [iso-8859-1] Sat Feb 27 22:47:59 2010
@@ -107,37 +107,6 @@
                                            SERVICE,
                                            ServiceListEntry);
         if (CurrentService->dwResumeCount > dwResumeCount)
-        {
-            DPRINT("Found service: '%S'\n", CurrentService->lpDisplayName);
-            return CurrentService;
-        }
-
-        ServiceEntry = ServiceEntry->Flink;
-    }
-
-    DPRINT("Couldn't find a matching service\n");
-
-    return NULL;
-}
-
-
-PSERVICE
-ScmGetServiceEntryByClientHandle(HANDLE Handle)
-{
-    PLIST_ENTRY ServiceEntry;
-    PSERVICE CurrentService;
-
-    DPRINT("ScmGetServiceEntryByClientHandle() called\n");
-    DPRINT("looking for %p\n", Handle);
-
-    ServiceEntry = ServiceListHead.Flink;
-    while (ServiceEntry != &ServiceListHead)
-    {
-        CurrentService = CONTAINING_RECORD(ServiceEntry,
-                                           SERVICE,
-                                           ServiceListEntry);
-
-        if (CurrentService->hClient == Handle)
         {
             DPRINT("Found service: '%S'\n", CurrentService->lpDisplayName);
             return CurrentService;
@@ -728,8 +697,8 @@
         return ERROR_NOT_ENOUGH_MEMORY;

ControlPacket->dwControl = dwControl;
-    ControlPacket->hClient = Service->hClient;
     ControlPacket->dwSize = TotalLength;
+    ControlPacket->hServiceStatus = (SERVICE_STATUS_HANDLE)Service;
     wcscpy(&ControlPacket->szArguments[0], Service->lpServiceName);

/* Send the control packet */
@@ -793,7 +762,7 @@
         return ERROR_NOT_ENOUGH_MEMORY;

ControlPacket->dwControl = SERVICE_CONTROL_START;
-    ControlPacket->hClient = Service->hClient;
+    ControlPacket->hServiceStatus = (SERVICE_STATUS_HANDLE)Service;
     ControlPacket->dwSize = TotalLength;
     Ptr = &ControlPacket->szArguments[0];
     wcscpy(Ptr, Service->lpServiceName);
@@ -850,6 +819,7 @@
     WCHAR NtControlPipeName[MAX_PATH + 1];
     HKEY hServiceCurrentKey = INVALID_HANDLE_VALUE;
     DWORD KeyDisposition;
+    DWORD dwProcessId;

RtlInitUnicodeString(&ImagePath, NULL);

@@ -991,7 +961,7 @@

/* Read SERVICE_STATUS_HANDLE from pipe */
         if (!ReadFile(Service->ControlPipeHandle,
-                      (LPVOID)&Service->hClient,
+                      (LPVOID)&dwProcessId,
                       sizeof(DWORD),
                       &dwRead,
                       NULL))
@@ -1002,7 +972,7 @@
         }
         else
         {
-            DPRINT("Received service status %lu\n", Service->hClient);
+            DPRINT("Received service process ID %lu\n", dwProcessId);

/* Send start command */
             dwError = ScmSendStartCommand(Service, argc, argv);
@@ -1244,4 +1214,25 @@
     DPRINT("ScmGetBootAndSystemDriverState() done\n");
 }

+
+BOOL
+ScmLockDatabaseExclusive(VOID)
+{
+    return RtlAcquireResourceExclusive(&DatabaseLock, TRUE);
+}
+
+
+BOOL
+ScmLockDatabaseShared(VOID)
+{
+    return RtlAcquireResourceShared(&DatabaseLock, TRUE);
+}
+
+
+VOID
+ScmUnlockDatabase(VOID)
+{
+    RtlReleaseResource(&DatabaseLock);
+}
+
 /* EOF */

Modified: trunk/reactos/base/system/services/rpcserver.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/rpcser...
==============================================================================
--- trunk/reactos/base/system/services/rpcserver.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/rpcserver.c [iso-8859-1] Sat Feb 27 22:47:59 2010
@@ -978,10 +978,14 @@
         return ERROR_INVALID_HANDLE;
     }

+    ScmLockDatabaseShared();
+
     /* Return service status information */
     RtlCopyMemory(lpServiceStatus,
                   &lpService->Status,
                   sizeof(SERVICE_STATUS));
+
+    ScmUnlockDatabase();

return ERROR_SUCCESS;
 }
@@ -1030,7 +1034,7 @@
         return ERROR_INVALID_HANDLE;
     }

-    lpService = ScmGetServiceEntryByClientHandle((HANDLE)hServiceStatus);
+    lpService = (PSERVICE)hServiceStatus;
     if (lpService == NULL)
     {
         DPRINT("lpService == NULL!\n");
@@ -1059,10 +1063,13 @@
         return ERROR_INVALID_DATA;
     }

+    ScmLockDatabaseExclusive();

RtlCopyMemory(&lpService->Status,
                   lpServiceStatus,
                   sizeof(SERVICE_STATUS));
+
+    ScmUnlockDatabase();

DPRINT("Set %S to %lu\n", lpService->lpDisplayName, lpService->Status.dwCurrentState);
     DPRINT("RSetServiceStatus() done\n");

Modified: trunk/reactos/base/system/services/services.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/servic...
==============================================================================
--- trunk/reactos/base/system/services/services.h [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/services.h [iso-8859-1] Sat Feb 27 22:47:59 2010
@@ -42,7 +42,6 @@
     DWORD dwResumeCount;
     DWORD dwRefCount;

-    CLIENT_HANDLE hClient;
     SERVICE_STATUS Status;
     DWORD dwStartType;
     DWORD dwErrorControl;
@@ -112,7 +111,6 @@
 PSERVICE ScmGetServiceEntryByName(LPCWSTR lpServiceName);
 PSERVICE ScmGetServiceEntryByDisplayName(LPCWSTR lpDisplayName);
 PSERVICE ScmGetServiceEntryByResumeCount(DWORD dwResumeCount);
-PSERVICE ScmGetServiceEntryByClientHandle(HANDLE Handle);
 DWORD ScmCreateNewServiceRecord(LPCWSTR lpServiceName,
                                 PSERVICE *lpServiceRecord);
 VOID ScmDeleteServiceRecord(PSERVICE lpService);
@@ -121,6 +119,11 @@
 DWORD ScmControlService(PSERVICE Service,
                         DWORD dwControl,
                         LPSERVICE_STATUS lpServiceStatus);
+
+BOOL ScmLockDatabaseExclusive(VOID);
+BOOL ScmLockDatabaseShared(VOID);
+VOID ScmUnlockDatabase(VOID);
+

/* driver.c */

Modified: trunk/reactos/dll/win32/advapi32/service/sctrl.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/service/...
==============================================================================
--- trunk/reactos/dll/win32/advapi32/service/sctrl.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/advapi32/service/sctrl.c [iso-8859-1] Sat Feb 27 22:47:59 2010
@@ -22,7 +22,7 @@

typedef struct _ACTIVE_SERVICE
 {
-    CLIENT_HANDLE hService;
+    SERVICE_STATUS_HANDLE hServiceStatus;
     UNICODE_STRING ServiceName;
     union
     {
@@ -32,7 +32,6 @@
     LPHANDLER_FUNCTION HandlerFunction;
     LPHANDLER_FUNCTION_EX HandlerFunctionEx;
     LPVOID HandlerContext;
-    SERVICE_STATUS ServiceStatus;
     BOOL bUnicode;
     LPWSTR Arguments;
 } ACTIVE_SERVICE, *PACTIVE_SERVICE;
@@ -199,6 +198,7 @@
     NTSTATUS Status;
     WCHAR NtControlPipeName[MAX_PATH + 1];
     RTL_QUERY_REGISTRY_TABLE QueryTable[2];
+    DWORD dwProcessId;

/* Get the service number and create the named pipe */
     RtlZeroMemory(&QueryTable,
@@ -249,37 +249,34 @@
         return ERROR_FAILED_SERVICE_CONTROLLER_CONNECT;
     }

-    /* Share the SERVICE_HANDLE handle with the SCM */
+    /* Pass the ProcessId to the SCM */
+    dwProcessId = GetCurrentProcessId();
     WriteFile(*hPipe,
-              (DWORD *)&lpActiveServices->hService,
-              sizeof(CLIENT_HANDLE),
+              &dwProcessId,
+              sizeof(DWORD),
               &dwBytesWritten,
               NULL);

-    TRACE("Sent SERVICE_HANDLE %lu\n", lpActiveServices->hService);
+    TRACE("Sent Process ID %lu\n", dwProcessId);
+

return ERROR_SUCCESS;
 }

static DWORD
-ScStartService(PSCM_CONTROL_PACKET ControlPacket)
-{
-    PACTIVE_SERVICE lpService;
+ScStartService(PACTIVE_SERVICE lpService,
+               PSCM_CONTROL_PACKET ControlPacket)
+{
     HANDLE ThreadHandle;
     DWORD ThreadId;

TRACE("ScStartService() called\n");
-    TRACE("client handle: %lu\n", ControlPacket->hClient);
     TRACE("Size: %lu\n", ControlPacket->dwSize);
     TRACE("Service: %S\n", &ControlPacket->szArguments[0]);

-    lpService = (PACTIVE_SERVICE)ControlPacket->hClient;
-    if (lpService == NULL)
-    {
-        TRACE("Service not found\n");
-        return ERROR_SERVICE_DOES_NOT_EXIST;
-    }
+    /* Set the service status handle */
+    lpService->hServiceStatus = ControlPacket->hServiceStatus;

lpService->Arguments = HeapAlloc(GetProcessHeap(),
                                      HEAP_ZERO_MEMORY,
@@ -309,20 +306,12 @@

static DWORD
-ScControlService(PSCM_CONTROL_PACKET ControlPacket)
-{
-    PACTIVE_SERVICE lpService;
-
+ScControlService(PACTIVE_SERVICE lpService,
+                 PSCM_CONTROL_PACKET ControlPacket)
+{
     TRACE("ScControlService() called\n");
     TRACE("Size: %lu\n", ControlPacket->dwSize);
     TRACE("Service: %S\n", &ControlPacket->szArguments[0]);
-
-    lpService = (PACTIVE_SERVICE)ControlPacket->hClient;
-    if (lpService == NULL)
-    {
-        TRACE("Service not found\n");
-        return ERROR_SERVICE_DOES_NOT_EXIST;
-    }

if (lpService->HandlerFunction)
     {
@@ -356,6 +345,8 @@
     DWORD Count;
     BOOL bResult;
     DWORD dwRunningServices = 0;
+    LPWSTR lpServiceName;
+    PACTIVE_SERVICE lpService;

TRACE("ScDispatcherLoop() called\n");

@@ -379,25 +370,32 @@
             return FALSE;
         }

-        /* Execute command */
-        switch (ControlPacket->dwControl)
-        {
-            case SERVICE_CONTROL_START:
-                TRACE("Start command - recieved SERVICE_CONTROL_START\n");
-                if (ScStartService(ControlPacket) == ERROR_SUCCESS)
-                    dwRunningServices++;
-                break;
-
-            case SERVICE_CONTROL_STOP:
-                TRACE("Stop command - recieved SERVICE_CONTROL_STOP\n");
-                if (ScControlService(ControlPacket) == ERROR_SUCCESS)
-                    dwRunningServices--;
-                break;
-
-            default:
-                TRACE("Command %lu received", ControlPacket->dwControl);
-                ScControlService(ControlPacket);
-                continue;
+        lpServiceName = &ControlPacket->szArguments[0];
+        TRACE("Service: %S\n", lpServiceName);
+
+        lpService = ScLookupServiceByServiceName(lpServiceName);
+        if (lpService != NULL)
+        {
+            /* Execute command */
+            switch (ControlPacket->dwControl)
+            {
+                case SERVICE_CONTROL_START:
+                    TRACE("Start command - recieved SERVICE_CONTROL_START\n");
+                    if (ScStartService(lpService, ControlPacket) == ERROR_SUCCESS)
+                        dwRunningServices++;
+                    break;
+
+                case SERVICE_CONTROL_STOP:
+                    TRACE("Stop command - recieved SERVICE_CONTROL_STOP\n");
+                    if (ScControlService(lpService, ControlPacket) == ERROR_SUCCESS)
+                        dwRunningServices--;
+                    break;
+
+                default:
+                    TRACE("Command %lu received", ControlPacket->dwControl);
+                    ScControlService(lpService, ControlPacket);
+                    continue;
+            }
         }

if (dwRunningServices == 0)
@@ -461,9 +459,9 @@
     Service->HandlerFunction = lpHandlerProc;
     Service->HandlerFunctionEx = NULL;

-    TRACE("RegisterServiceCtrlHandler returning %lu\n", Service->hService);
-
-    return (SERVICE_STATUS_HANDLE)Service->hService;
+    TRACE("RegisterServiceCtrlHandler returning %lu\n", Service->hServiceStatus);
+
+    return Service->hServiceStatus;
 }

@@ -520,9 +518,9 @@
     Service->HandlerFunctionEx = lpHandlerProc;
     Service->HandlerContext = lpContext;

-    TRACE("RegisterServiceCtrlHandlerEx returning %lu\n", Service->hService);
-
-    return (SERVICE_STATUS_HANDLE)Service->hService;
+    TRACE("RegisterServiceCtrlHandlerEx returning %lu\n", Service->hServiceStatus);
+
+    return Service->hServiceStatus;
 }

@@ -683,7 +681,7 @@
         RtlCreateUnicodeStringFromAsciiz(&lpActiveServices[i].ServiceName,
                                          lpServiceStartTable[i].lpServiceName);
         lpActiveServices[i].Main.lpFuncA = lpServiceStartTable[i].lpServiceProc;
-        lpActiveServices[i].hService = (CLIENT_HANDLE)&lpActiveServices[i];
+        lpActiveServices[i].hServiceStatus = 0;
         lpActiveServices[i].bUnicode = FALSE;
     }

@@ -773,7 +771,7 @@
         RtlCreateUnicodeString(&lpActiveServices[i].ServiceName,
                                lpServiceStartTable[i].lpServiceName);
         lpActiveServices[i].Main.lpFuncW = lpServiceStartTable[i].lpServiceProc;
-        lpActiveServices[i].hService = (CLIENT_HANDLE)&lpActiveServices[i];
+        lpActiveServices[i].hServiceStatus = 0;
         lpActiveServices[i].bUnicode = TRUE;
     }

Modified: trunk/reactos/include/reactos/services/services.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/reactos/services/se...
==============================================================================
--- trunk/reactos/include/reactos/services/services.h [iso-8859-1] (original)
+++ trunk/reactos/include/reactos/services/services.h [iso-8859-1] Sat Feb 27 22:47:59 2010
@@ -11,12 +11,10 @@

#define SERVICE_CONTROL_START 0

-DECLARE_HANDLE(CLIENT_HANDLE);
-
 typedef struct _SCM_CONTROL_PACKET
 {
     DWORD dwControl;
-    CLIENT_HANDLE hClient;
+    SERVICE_STATUS_HANDLE hServiceStatus;
     DWORD dwSize;
     WCHAR szArguments[1];
 } SCM_CONTROL_PACKET, *PSCM_CONTROL_PACKET;