[arty] 22684: Fixed reference counting in CmiConnectHive and CmiDisconnectHive. No longer need hacks to check reference counts. Deleted a ton of wierd code. Fixed bug where we allocated uninitialized memory for child nodes we never populated. Now reference counting mirrors pointers exactly: - Hold one reference for the parent key pointer - Hold one reference for the list entry in the connected hive list
Author: arty Date: Thu Jun 29 07:48:43 2006 New Revision: 22684
URL: http://svn.reactos.org/svn/reactos?rev=22684&view=rev Log: Fixed reference counting in CmiConnectHive and CmiDisconnectHive. No longer need hacks to check reference counts. Deleted a ton of wierd code. Fixed bug where we allocated uninitialized memory for child nodes we never populated. Now reference counting mirrors pointers exactly: - Hold one reference for the parent key pointer - Hold one reference for the list entry in the connected hive list
Modified: trunk/reactos/ntoskrnl/cm/cm.h trunk/reactos/ntoskrnl/cm/import.c trunk/reactos/ntoskrnl/cm/registry.c trunk/reactos/ntoskrnl/cm/regobj.c
Modified: trunk/reactos/ntoskrnl/cm/cm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/cm.h?rev=22684&... ============================================================================== --- trunk/reactos/ntoskrnl/cm/cm.h (original) +++ trunk/reactos/ntoskrnl/cm/cm.h Thu Jun 29 07:48:43 2006 @@ -358,6 +358,9 @@
/* Time stamp for the last access by the parse routine */ ULONG TimeStamp; + + /* List entry for connected hives */ + LIST_ENTRY HiveList; } KEY_OBJECT, *PKEY_OBJECT;
/* Bits 31-22 (top 10 bits) of the cell index is the directory index */
Modified: trunk/reactos/ntoskrnl/cm/import.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/import.c?rev=22... ============================================================================== --- trunk/reactos/ntoskrnl/cm/import.c (original) +++ trunk/reactos/ntoskrnl/cm/import.c Thu Jun 29 07:48:43 2006 @@ -142,6 +142,8 @@ /* Acquire hive list lock exclusively */ KeEnterCriticalRegion(); ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE); + + DPRINT1("Adding new hive\n");
/* Add the new hive to the hive list */ InsertTailList(&CmiHiveListHead, &Hive->HiveList); @@ -195,7 +197,6 @@ if (!NT_SUCCESS(Status)) { DPRINT1 ("CmiConnectHive(%wZ) failed (Status %lx)\n", &KeyName, Status); -// CmiRemoveRegistryHive(RegistryHive); return FALSE; }
Modified: trunk/reactos/ntoskrnl/cm/registry.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/registry.c?rev=... ============================================================================== --- trunk/reactos/ntoskrnl/cm/registry.c (original) +++ trunk/reactos/ntoskrnl/cm/registry.c Thu Jun 29 07:48:43 2006 @@ -36,6 +36,7 @@
KTIMER CmiWorkerTimer; LIST_ENTRY CmiKeyObjectListHead; +LIST_ENTRY CmiConnectedHiveList; ULONG CmiTimer = 0;
volatile BOOLEAN CmiHiveSyncEnabled = FALSE; @@ -385,6 +386,7 @@
/* Initialize the key object list */ InitializeListHead(&CmiKeyObjectListHead); + InitializeListHead(&CmiConnectedHiveList);
/* Initialize the worker timer */ KeInitializeTimerEx(&CmiWorkerTimer, SynchronizationTimer); @@ -694,92 +696,94 @@ return Status; }
- NTSTATUS CmiConnectHive(IN POBJECT_ATTRIBUTES KeyObjectAttributes, IN PREGISTRY_HIVE RegistryHive) { - UNICODE_STRING RemainingPath; - PKEY_OBJECT ParentKey; - PKEY_OBJECT NewKey; - NTSTATUS Status; - PWSTR SubName; - UNICODE_STRING ObjectName; - OBJECT_CREATE_INFORMATION ObjectCreateInfo; - - DPRINT("CmiConnectHive(%p, %p) called.\n", - KeyObjectAttributes, RegistryHive); - - /* Capture all the info */ - DPRINT("Capturing Create Info\n"); - Status = ObpCaptureObjectAttributes(KeyObjectAttributes, - KernelMode, - FALSE, - &ObjectCreateInfo, - &ObjectName); - if (!NT_SUCCESS(Status)) - { + UNICODE_STRING RemainingPath; + PKEY_OBJECT ParentKey; + PKEY_OBJECT NewKey; + NTSTATUS Status; + PWSTR SubName; + UNICODE_STRING ObjectName; + OBJECT_CREATE_INFORMATION ObjectCreateInfo; + + DPRINT("CmiConnectHive(%p, %p) called.\n", + KeyObjectAttributes, RegistryHive); + + /* Capture all the info */ + DPRINT("Capturing Create Info\n"); + Status = ObpCaptureObjectAttributes(KeyObjectAttributes, + KernelMode, + FALSE, + &ObjectCreateInfo, + &ObjectName); + + if (!NT_SUCCESS(Status)) + { DPRINT("ObpCaptureObjectAttributes() failed (Status %lx)\n", Status); return Status; - } - - Status = CmFindObject(&ObjectCreateInfo, - &ObjectName, - (PVOID*)&ParentKey, - &RemainingPath, - CmiKeyType, - NULL, - NULL); - ObpReleaseCapturedAttributes(&ObjectCreateInfo); - if (ObjectName.Buffer) ObpReleaseCapturedName(&ObjectName); - if (!NT_SUCCESS(Status)) - { - return Status; - } - - DPRINT ("RemainingPath %wZ\n", &RemainingPath); - - if ((RemainingPath.Buffer == NULL) || (RemainingPath.Buffer[0] == 0)) - { - ObDereferenceObject (ParentKey); - RtlFreeUnicodeString(&RemainingPath); - return STATUS_OBJECT_NAME_COLLISION; - } - - /* Ignore leading backslash */ - SubName = RemainingPath.Buffer; - if (*SubName == L'\') - SubName++; - - /* If RemainingPath contains \ we must return error - because CmiConnectHive() can not create trees */ - if (wcschr (SubName, L'\') != NULL) - { - ObDereferenceObject (ParentKey); - RtlFreeUnicodeString(&RemainingPath); - return STATUS_OBJECT_NAME_NOT_FOUND; - } - - DPRINT("RemainingPath %wZ ParentKey %p\n", - &RemainingPath, ParentKey); - - Status = ObCreateObject(KernelMode, + } + + Status = CmFindObject(&ObjectCreateInfo, + &ObjectName, + (PVOID*)&ParentKey, + &RemainingPath, CmiKeyType, NULL, - KernelMode, - NULL, - sizeof(KEY_OBJECT), - 0, - 0, - (PVOID*)&NewKey); - - if (!NT_SUCCESS(Status)) - { - DPRINT1 ("ObCreateObject() failed (Status %lx)\n", Status); - ObDereferenceObject (ParentKey); - RtlFreeUnicodeString(&RemainingPath); - return Status; - } + NULL); + /* Yields a new reference */ + ObpReleaseCapturedAttributes(&ObjectCreateInfo); + + if (ObjectName.Buffer) ObpReleaseCapturedName(&ObjectName); + if (!NT_SUCCESS(Status)) + { + return Status; + } + + DPRINT ("RemainingPath %wZ\n", &RemainingPath); + + if ((RemainingPath.Buffer == NULL) || (RemainingPath.Buffer[0] == 0)) + { + ObDereferenceObject (ParentKey); + RtlFreeUnicodeString(&RemainingPath); + return STATUS_OBJECT_NAME_COLLISION; + } + + /* Ignore leading backslash */ + SubName = RemainingPath.Buffer; + if (*SubName == L'\') + SubName++; + + /* If RemainingPath contains \ we must return error + because CmiConnectHive() can not create trees */ + if (wcschr (SubName, L'\') != NULL) + { + ObDereferenceObject (ParentKey); + RtlFreeUnicodeString(&RemainingPath); + return STATUS_OBJECT_NAME_NOT_FOUND; + } + + DPRINT("RemainingPath %wZ ParentKey %p\n", + &RemainingPath, ParentKey); + + Status = ObCreateObject(KernelMode, + CmiKeyType, + NULL, + KernelMode, + NULL, + sizeof(KEY_OBJECT), + 0, + 0, + (PVOID*)&NewKey); + + if (!NT_SUCCESS(Status)) + { + DPRINT1 ("ObCreateObject() failed (Status %lx)\n", Status); + ObDereferenceObject (ParentKey); + RtlFreeUnicodeString(&RemainingPath); + return Status; + } DPRINT("Inserting Key into Object Tree\n"); Status = ObInsertObject((PVOID)NewKey, NULL, @@ -787,56 +791,42 @@ 0, NULL, NULL); -DPRINT("Status %x\n", Status); - NewKey->RegistryHive = RegistryHive; - NewKey->KeyCellOffset = RegistryHive->HiveHeader->RootKeyOffset; - NewKey->KeyCell = CmiGetCell (RegistryHive, NewKey->KeyCellOffset, NULL); - NewKey->Flags = 0; - NewKey->NumberOfSubKeys = 0; - InsertTailList(&CmiKeyObjectListHead, &NewKey->ListEntry); - if (NewKey->KeyCell->NumberOfSubKeys != 0) - { - NewKey->SubKeys = ExAllocatePool(NonPagedPool, - NewKey->KeyCell->NumberOfSubKeys * sizeof(ULONG)); - if (NewKey->SubKeys == NULL) - { - DPRINT("ExAllocatePool() failed\n"); - ObDereferenceObject (NewKey); - ObDereferenceObject (ParentKey); - RtlFreeUnicodeString(&RemainingPath); - return STATUS_INSUFFICIENT_RESOURCES; - } - } - else - { - NewKey->SubKeys = NULL; - } - - DPRINT ("SubName %S\n", SubName); - - Status = RtlpCreateUnicodeString(&NewKey->Name, - SubName, NonPagedPool); - RtlFreeUnicodeString(&RemainingPath); - if (!NT_SUCCESS(Status)) - { - DPRINT1("RtlpCreateUnicodeString() failed (Status %lx)\n", Status); - if (NewKey->SubKeys != NULL) - { - ExFreePool (NewKey->SubKeys); - } - ObDereferenceObject (NewKey); - ObDereferenceObject (ParentKey); - return STATUS_INSUFFICIENT_RESOURCES; - } - - CmiAddKeyToList (ParentKey, NewKey); - ObDereferenceObject (ParentKey); - - VERIFY_KEY_OBJECT(NewKey); - - /* Note: Do not dereference NewKey here! */ - - return STATUS_SUCCESS; + DPRINT("Status %x\n", Status); + NewKey->RegistryHive = RegistryHive; + NewKey->KeyCellOffset = RegistryHive->HiveHeader->RootKeyOffset; + NewKey->KeyCell = CmiGetCell (RegistryHive, NewKey->KeyCellOffset, NULL); + NewKey->Flags = 0; + NewKey->NumberOfSubKeys = 0; + NewKey->SubKeys = NULL; + InsertTailList(&CmiKeyObjectListHead, &NewKey->ListEntry); + InsertTailList(&CmiConnectedHiveList, &NewKey->HiveList); + + DPRINT ("SubName %S\n", SubName); + + Status = RtlpCreateUnicodeString(&NewKey->Name, + SubName, NonPagedPool); + RtlFreeUnicodeString(&RemainingPath); + if (!NT_SUCCESS(Status)) + { + DPRINT1("RtlpCreateUnicodeString() failed (Status %lx)\n", Status); + if (NewKey->SubKeys != NULL) + { + ExFreePool (NewKey->SubKeys); + } + ObDereferenceObject (NewKey); + ObDereferenceObject (ParentKey); + return STATUS_INSUFFICIENT_RESOURCES; + } + + CmiAddKeyToList (ParentKey, NewKey); + + VERIFY_KEY_OBJECT(NewKey); + + /* We're holding a pointer to the parent key .. We must keep it + * referenced */ + /* Note: Do not dereference NewKey here! */ + + return STATUS_SUCCESS; }
@@ -845,9 +835,8 @@ OUT PREGISTRY_HIVE *RegistryHive) { PKEY_OBJECT KeyObject; - PREGISTRY_HIVE Hive; HANDLE KeyHandle; - NTSTATUS Status; + NTSTATUS Status = STATUS_OBJECT_NAME_NOT_FOUND; PLIST_ENTRY CurrentEntry; PKEY_OBJECT CurrentKey;
@@ -875,68 +864,47 @@ (PVOID*)&KeyObject, NULL); ZwClose (KeyHandle); + if (!NT_SUCCESS(Status)) { DPRINT1 ("ObReferenceObjectByName() failed (Status %lx)\n", Status); return Status; } DPRINT ("KeyObject %p Hive %p\n", KeyObject, KeyObject->RegistryHive); - - if (!(KeyObject->KeyCell->Flags & REG_KEY_ROOT_CELL)) - { - DPRINT1 ("Key is not the Hive-Root-Key\n"); - ObDereferenceObject (KeyObject); - return STATUS_INVALID_PARAMETER; - }
/* Acquire registry lock exclusively */ KeEnterCriticalRegion(); ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
- CurrentEntry = CmiKeyObjectListHead.Flink; - while (CurrentEntry != &CmiKeyObjectListHead) - { - CurrentKey = CONTAINING_RECORD(CurrentEntry, KEY_OBJECT, ListEntry); - if (1 == ObGetObjectPointerCount(CurrentKey) && - !(CurrentKey->Flags & KO_MARKED_FOR_DELETE)) - { - ObDereferenceObject(CurrentKey); - CurrentEntry = CmiKeyObjectListHead.Flink; - } - else - { - CurrentEntry = CurrentEntry->Flink; - } + /* Find out if we represent a connected hive. */ + for( CurrentEntry = CmiConnectedHiveList.Flink; + CurrentEntry != &CmiConnectedHiveList; + CurrentEntry = CurrentEntry->Flink ) { + CurrentKey = CONTAINING_RECORD(CurrentEntry, KEY_OBJECT, HiveList); + if( CurrentKey == KeyObject ) { + /* Remove the connected hive from the connected hive list */ + RemoveEntryList(CurrentEntry); + /* found ourselves in the connected hive list */ + *RegistryHive = KeyObject->RegistryHive; + Status = STATUS_SUCCESS; + + /* Release references captured in CmiConnectHive */ + ObDereferenceObject (KeyObject->ParentKey); + ObDereferenceObject (KeyObject); + break; + } } - - if (ObGetObjectHandleCount (KeyObject) != 0 || - ObGetObjectPointerCount (KeyObject) != 2) - { - DPRINT1 ("Hive is still in use (hc %d, rc %d)\n", ObGetObjectHandleCount (KeyObject), ObGetObjectPointerCount (KeyObject)); - ObDereferenceObject (KeyObject); - - /* Release registry lock */ - ExReleaseResourceLite (&CmiRegistryLock); - KeLeaveCriticalRegion(); - - return STATUS_UNSUCCESSFUL; - } - - Hive = KeyObject->RegistryHive; - - /* Dereference KeyObject twice to delete it */ - ObDereferenceObject (KeyObject); - ObDereferenceObject (KeyObject); - - *RegistryHive = Hive; - + /* Release registry lock */ ExReleaseResourceLite (&CmiRegistryLock); KeLeaveCriticalRegion();
+ /* Release reference above */ + ObDereferenceObject (KeyObject); + DPRINT ("CmiDisconnectHive() done\n");
- return STATUS_SUCCESS; + return Status; }
Modified: trunk/reactos/ntoskrnl/cm/regobj.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/regobj.c?rev=22... ============================================================================== --- trunk/reactos/ntoskrnl/cm/regobj.c (original) +++ trunk/reactos/ntoskrnl/cm/regobj.c Thu Jun 29 07:48:43 2006 @@ -462,7 +462,7 @@ ExReleaseResourceLite(&CmiRegistryLock); KeLeaveCriticalRegion();
- DPRINT("CmiObjectParse: %s\n", FoundObject->Name); + //DPRINT("CmiObjectParse: %s\n", FoundObject->Name);
*Path = EndPtr;
@@ -492,9 +492,9 @@ KeEnterCriticalRegion(); ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
- //if (!NT_SUCCESS(CmiRemoveKeyFromList(KeyObject))) - { - // DPRINT1("Key not found in parent list ???\n"); + if (!NT_SUCCESS(CmiRemoveKeyFromList(KeyObject))) + { + DPRINT1("Key not found in parent list ???\n"); }
RemoveEntryList(&KeyObject->ListEntry); @@ -526,7 +526,7 @@
if (KeyObject->NumberOfSubKeys) { - //KEBUGCHECK(REGISTRY_ERROR); + KEBUGCHECK(REGISTRY_ERROR); }
if (KeyObject->SizeOfSubKeys)
-
arty＠svn.reactos.org