[PATCH] services.exe: don't crash if registry contrains a service without displayname (2nd try, fixes bug #13958)

Mikołaj Zalewski mikolaj at zalewski.pl
Fri Jul 25 05:53:13 CDT 2008


---
 dlls/advapi32/tests/service.c |    9 ++++++++-
 programs/services/rpc.c       |    6 ++++--
 programs/services/services.c  |   26 ++++++++++++++++++++++----
 programs/services/services.h  |   16 ++++++++--------
 4 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/dlls/advapi32/tests/service.c b/dlls/advapi32/tests/service.c
index cd6d82c..5b3a43e 100644
--- a/dlls/advapi32/tests/service.c
+++ b/dlls/advapi32/tests/service.c
@@ -415,6 +415,7 @@ static void test_get_displayname(void)
     SC_HANDLE scm_handle, svc_handle;
     BOOL ret;
     CHAR displayname[4096];
+    CHAR keyname[4096];
     WCHAR displaynameW[2048];
     DWORD displaysize, tempsize, tempsizeW;
     static const CHAR deadbeef[] = "Deadbeef";
@@ -615,7 +616,7 @@ static void test_get_displayname(void)
     ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER,
        "Expected ERROR_INSUFFICIENT_BUFFER, got %d\n", GetLastError());
 
-    /* Get the displayname */
+    /* Get the displayname - as dislpayname was NULL, the keyname will be returned */
     SetLastError(0xdeadbeef);
     ret = GetServiceDisplayNameA(scm_handle, servicename, displayname, &displaysize);
     ok(ret, "Expected success\n");
@@ -626,6 +627,12 @@ static void test_get_displayname(void)
        GetLastError() == 0xdeadbeef       /* NT4, XP, Vista */,
        "Expected ERROR_SUCCESS, ERROR_IO_PENDING or 0xdeadbeef, got %d\n", GetLastError());
 
+    /* the keyname will also work as displayname in GetServiceKeyNameA */
+    displaysize = strlen(servicename) + 1;
+    ok(GetServiceKeyNameA(scm_handle, servicename, keyname, &displaysize), "GetServiceKeyName failed\n");
+    ok(!lstrcmpi(keyname, servicename),
+       "Expected keyname to be %s, got %s\n", servicename, keyname);
+
     /* Delete the service */
     ret = DeleteService(svc_handle);
     ok(ret, "Expected success\n");
diff --git a/programs/services/rpc.c b/programs/services/rpc.c
index ed353d4..936b7d6 100644
--- a/programs/services/rpc.c
+++ b/programs/services/rpc.c
@@ -203,7 +203,7 @@ DWORD svcctl_GetServiceDisplayNameW(
     {
         LPCWSTR name;
         service_lock_shared(entry);
-        name = get_display_name(entry);
+        name = entry->config.lpDisplayName;
         *cchLength = strlenW(name);
         if (*cchLength < cchBufSize)
         {
@@ -377,6 +377,7 @@ DWORD svcctl_CreateServiceW(
     else
         entry->config.dwTagId = 0;
 
+    service_set_defaults(entry);
     /* other fields NULL*/
 
     if (!validate_service_config(entry))
@@ -395,7 +396,7 @@ DWORD svcctl_CreateServiceW(
         return ERROR_SERVICE_EXISTS;
     }
 
-    if (scmdatabase_find_service_by_displayname(manager->db, get_display_name(entry)))
+    if (scmdatabase_find_service_by_displayname(manager->db, entry->config.lpDisplayName))
     {
         scmdatabase_unlock(manager->db);
         free_service_entry(entry);
@@ -570,6 +571,7 @@ DWORD svcctl_ChangeServiceConfigW(
     {
         HeapFree(GetProcessHeap(), 0, service->service_entry->config.lpDisplayName);
         new_entry.config.lpDisplayName = strdupW(lpDisplayName);
+        new_entry.flags &= ~SERVICE_IMPLICIT_DISPLAYNAME;
     }
 
     *service->service_entry = new_entry;
diff --git a/programs/services/services.c b/programs/services/services.c
index 36ed117..47b582c 100644
--- a/programs/services/services.c
+++ b/programs/services/services.c
@@ -91,6 +91,18 @@ void free_service_entry(struct service_entry *entry)
     HeapFree(GetProcessHeap(), 0, entry);
 }
 
+void service_set_defaults(struct service_entry *entry)
+{
+    if (entry->config.lpServiceStartName == NULL)
+        entry->config.lpServiceStartName = strdupW(SZ_LOCAL_SYSTEM);
+
+    if (entry->config.lpDisplayName == NULL)
+    {
+        entry->flags |= SERVICE_IMPLICIT_DISPLAYNAME;
+        entry->config.lpDisplayName = strdupW(entry->name);
+    }
+}
+
 static DWORD load_service_config(HKEY hKey, struct service_entry *entry)
 {
     DWORD err;
@@ -153,12 +165,14 @@ DWORD save_service_config(struct service_entry *entry)
 {
     DWORD err;
     HKEY hKey = NULL;
+    LPWSTR saveDisplayName;
 
     err = RegCreateKeyW(entry->db->root_key, entry->name, &hKey);
     if (err != ERROR_SUCCESS)
         goto cleanup;
 
-    if ((err = reg_set_string_value(hKey, SZ_DISPLAY_NAME, entry->config.lpDisplayName)) != 0)
+    saveDisplayName = (entry->flags & SERVICE_IMPLICIT_DISPLAYNAME ? NULL : entry->config.lpDisplayName);
+    if ((err = reg_set_string_value(hKey, SZ_DISPLAY_NAME, saveDisplayName)) != 0)
         goto cleanup;
     if ((err = reg_set_string_value(hKey, SZ_IMAGE_PATH, entry->config.lpBinaryPathName)) != 0)
         goto cleanup;
@@ -273,7 +287,7 @@ BOOL validate_service_name(LPCWSTR name)
     return (name && name[0] && !strchrW(name, '/') && !strchrW(name, '\\'));
 }
 
-BOOL validate_service_config(struct service_entry *entry)
+BOOL validate_service_config(const struct service_entry *entry)
 {
     if (entry->config.dwServiceType & SERVICE_WIN32 && (entry->config.lpBinaryPathName == NULL || !entry->config.lpBinaryPathName[0]))
     {
@@ -318,8 +332,11 @@ BOOL validate_service_config(struct service_entry *entry)
         return FALSE;
     }
 
-    if (entry->config.lpServiceStartName == NULL)
-        entry->config.lpServiceStartName = strdupW(SZ_LOCAL_SYSTEM);
+    if (!entry->config.lpDisplayName)
+    {
+        WINE_ERR("No display name - should be set by service_set_defaults\n");
+        return FALSE;
+    }
 
     return TRUE;
 }
@@ -416,6 +433,7 @@ static DWORD scmdatabase_load_services(struct scmdatabase *db)
         if (err == ERROR_SUCCESS)
         {
             err = load_service_config(hServiceKey, entry);
+            service_set_defaults(entry);
             RegCloseKey(hServiceKey);
         }
 
diff --git a/programs/services/services.h b/programs/services/services.h
index fd99bf9..0a384db 100644
--- a/programs/services/services.h
+++ b/programs/services/services.h
@@ -31,14 +31,18 @@ struct scmdatabase
     CRITICAL_SECTION cs;
 };
 
+/* values of service_entry.flags */
+#define SERVICE_IMPLICIT_DISPLAYNAME 1
+
 struct service_entry
 {
     struct list entry;
     struct scmdatabase *db;
     LONG ref_count;                    /* number of references - if goes to zero and the service is deleted the structure will be freed */
-    LPWSTR name;
+    LPWSTR name;                       /* not null, const */
+    DWORD flags;
     SERVICE_STATUS_PROCESS status;
-    QUERY_SERVICE_CONFIGW config;
+    QUERY_SERVICE_CONFIGW config;      /* config.lpDisplayName not null*/
     LPWSTR description;
     LPWSTR dependOnServices;
     LPWSTR dependOnGroups;
@@ -66,8 +70,9 @@ void scmdatabase_unlock(struct scmdatabase *db);
 /* Service functions */
 
 DWORD service_create(LPCWSTR name, struct service_entry **entry);
+void service_set_defaults(struct service_entry *entry);
 BOOL validate_service_name(LPCWSTR name);
-BOOL validate_service_config(struct service_entry *entry);
+BOOL validate_service_config(const struct service_entry *entry);
 DWORD save_service_config(struct service_entry *entry);
 void free_service_entry(struct service_entry *entry);
 void release_service(struct service_entry *service);
@@ -90,11 +95,6 @@ DWORD load_reg_string(HKEY hKey, LPCWSTR szValue, BOOL bExpand, LPWSTR *output);
 DWORD load_reg_multisz(HKEY hKey, LPCWSTR szValue, LPWSTR *output);
 DWORD load_reg_dword(HKEY hKey, LPCWSTR szValue, DWORD *output);
 
-static inline LPCWSTR get_display_name(struct service_entry *service)
-{
-    return service->config.lpDisplayName ? service->config.lpDisplayName : service->name;
-}
-
 static inline BOOL is_marked_for_delete(struct service_entry *service)
 {
     return service->entry.next == NULL;
-- 
1.5.4


--------------090400000700080704040606--



More information about the wine-patches mailing list