From d02a403c944fcb135be5e919e39db44289f7f914 Mon Sep 17 00:00:00 2001 From: Alex Ionescu Date: Thu, 29 Jun 2006 05:05:27 +0000 Subject: [PATCH] - This patch finally enables closing handles for real, when NtClose is called. This means that File handles, processes, and all other NT Objects finally die (no more "file in use" and unkillable processes). On the other hand, this makes the registry code unhappy and unravelled a bug in ObDuplicateObject. - Booting/installing still works, but the system will possibly be unstable. However I'm choosing to commit this because it shows correct Ob behavior and will allow Art to fix Cm's referencing properly. - Implement ObCheckObjectAccess and call it to perform access verification checks. - Properly create devices, processes and controllers with an extra reference count, so that when the code closes their handle they don't die. - Check for invalid uses of ObfDereferenceObject for our current debugging purposes. - Add SEH to NtQueryObject. svn path=/trunk/; revision=22685 --- reactos/ntoskrnl/include/internal/ob.h | 12 ++- reactos/ntoskrnl/io/controller.c | 4 +- reactos/ntoskrnl/io/device.c | 4 +- reactos/ntoskrnl/ob/obdir.c | 2 +- reactos/ntoskrnl/ob/obhandle.c | 31 +++++--- reactos/ntoskrnl/ob/oblife.c | 49 ++++++++++-- reactos/ntoskrnl/ob/obref.c | 21 ++++- reactos/ntoskrnl/ob/obsecure.c | 102 ++++++++++++++++++++++++- reactos/ntoskrnl/ps/process.c | 4 +- 9 files changed, 205 insertions(+), 24 deletions(-) diff --git a/reactos/ntoskrnl/include/internal/ob.h b/reactos/ntoskrnl/include/internal/ob.h index ddbfbd7cadc..65f97ca293e 100644 --- a/reactos/ntoskrnl/include/internal/ob.h +++ b/reactos/ntoskrnl/include/internal/ob.h @@ -214,7 +214,7 @@ ObpDeleteNameCheck( ); // -// Security descriptor cache functions +// Security functions // NTSTATUS NTAPI @@ -247,6 +247,16 @@ ObpDereferenceCachedSecurityDescriptor( IN PSECURITY_DESCRIPTOR SecurityDescriptor ); +BOOLEAN +NTAPI +ObCheckObjectAccess( + IN PVOID Object, + IN OUT PACCESS_STATE AccessState, + IN BOOLEAN Unknown, + IN KPROCESSOR_MODE AccessMode, + OUT PNTSTATUS ReturnedStatus +); + // // Executive Fast Referencing Functions // diff --git a/reactos/ntoskrnl/io/controller.c b/reactos/ntoskrnl/io/controller.c index 457dc8dad12..2a2a2d576ce 100644 --- a/reactos/ntoskrnl/io/controller.c +++ b/reactos/ntoskrnl/io/controller.c @@ -100,8 +100,8 @@ IoCreateController(ULONG Size) Status = ObInsertObject(Controller, NULL, FILE_READ_DATA | FILE_WRITE_DATA, - 0, - NULL, + 1, + (PVOID*)&Controller, &Handle); if (!NT_SUCCESS(Status)) return NULL; diff --git a/reactos/ntoskrnl/io/device.c b/reactos/ntoskrnl/io/device.c index 5574c063180..bfd7a23c934 100644 --- a/reactos/ntoskrnl/io/device.c +++ b/reactos/ntoskrnl/io/device.c @@ -566,8 +566,8 @@ IoCreateDevice(PDRIVER_OBJECT DriverObject, Status = ObInsertObject(CreatedDeviceObject, NULL, FILE_READ_DATA | FILE_WRITE_DATA, - 0, - NULL, + 1, + (PVOID*)&CreatedDeviceObject, &TempHandle); if (!NT_SUCCESS(Status)) diff --git a/reactos/ntoskrnl/ob/obdir.c b/reactos/ntoskrnl/ob/obdir.c index 6e394c4e436..dac008026eb 100644 --- a/reactos/ntoskrnl/ob/obdir.c +++ b/reactos/ntoskrnl/ob/obdir.c @@ -455,7 +455,7 @@ NtQueryDirectoryObject(IN HANDLE DirectoryHandle, PreviousMode, (PVOID*)&Directory, NULL); - if(NT_SUCCESS(Status)) + if (NT_SUCCESS(Status)) { /* FIXME: TODO. UNIMPLEMENTED */ Status = STATUS_INSUFFICIENT_RESOURCES; diff --git a/reactos/ntoskrnl/ob/obhandle.c b/reactos/ntoskrnl/ob/obhandle.c index eca177bfe9a..252f3d2f350 100644 --- a/reactos/ntoskrnl/ob/obhandle.c +++ b/reactos/ntoskrnl/ob/obhandle.c @@ -237,7 +237,17 @@ ObpCloseHandleTableEntry(IN PHANDLE_TABLE HandleTable, ObpDecrementHandleCount(Body, PsGetCurrentProcess(), GrantedAccess); /* Dereference the object as well */ - //ObDereferenceObject(Body); // FIXME: Needs sync changes in other code + if (!wcscmp(ObjectHeader->Type->Name.Buffer, L"Key")) + { + // + // WE DONT CLOSE REGISTRY HANDLES BECAUSE CM IS BRAINDEAD + // + DPRINT("NOT CLOSING THE KEY\n"); + } + else + { + ObDereferenceObject(Body); + } /* Return to caller */ OBTRACE(OB_HANDLE_DEBUG, @@ -310,20 +320,16 @@ ObpIncrementHandleCount(IN PVOID Object, /* Check if we're opening an existing handle */ if (OpenReason == ObOpenHandle) { - /* - * FIXME: Do validation as described in Chapter 8 - * of Windows Internals 4th. - */ -#if 0 + /* Validate the caller's access to this object */ if (!ObCheckObjectAccess(Object, AccessState, TRUE, AccessMode, &Status)) { + /* Access was denied, so fail */ return Status; } -#endif } else if (OpenReason == ObCreateHandle) { @@ -982,7 +988,14 @@ ObpSetHandleAttributes(IN PHANDLE_TABLE HandleTable, /* Check if making the handle inheritable */ if (SetHandleInfo->Information.Inherit) { - /* Set the flag. FIXME: Need to check if this is allowed */ + /* Check if inheriting is not supported for this object */ + if (ObjectHeader->Type->TypeInfo.InvalidAttributes & OBJ_INHERIT) + { + /* Fail without changing anything */ + return FALSE; + } + + /* Set the flag */ HandleTableEntry->ObAttributes |= EX_HANDLE_ENTRY_INHERITABLE; } else @@ -1757,7 +1770,7 @@ ObInsertObject(IN PVOID Object, Header->ObjectCreateInfo = NULL; /* Remove the extra keep-alive reference */ - if (Handle) ObDereferenceObject(Object); // FIXME: Needs sync changes + if (Handle) ObDereferenceObject(Object); /* Return */ OBTRACE(OB_HANDLE_DEBUG, diff --git a/reactos/ntoskrnl/ob/oblife.c b/reactos/ntoskrnl/ob/oblife.c index 0d6c2b5763c..141e5d1adb9 100644 --- a/reactos/ntoskrnl/ob/oblife.c +++ b/reactos/ntoskrnl/ob/oblife.c @@ -946,10 +946,32 @@ NtQueryObject(IN HANDLE ObjectHandle, POBJECT_BASIC_INFORMATION BasicInfo; ULONG InfoLength; PVOID Object = NULL; - NTSTATUS Status; + NTSTATUS Status = STATUS_SUCCESS; + KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); PAGED_CODE(); - /* FIXME: Needs SEH */ + /* Check if the caller is from user mode */ + if (PreviousMode != KernelMode) + { + /* Protect validation with SEH */ + _SEH_TRY + { + /* Probe the input structure */ + ProbeForWrite(ObjectInformation, Length, sizeof(UCHAR)); + + /* If we have a result length, probe it too */ + if (ResultLength) ProbeForWriteUlong(ResultLength); + } + _SEH_HANDLE + { + /* Get the exception code */ + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + /* Fail if we raised an exception */ + if (!NT_SUCCESS(Status)) return Status; + } /* * Make sure this isn't a generic type query, since the caller doesn't @@ -1082,16 +1104,33 @@ NtQueryObject(IN HANDLE ObjectHandle, /* Anything else */ default: + /* Fail it */ Status = STATUS_INVALID_INFO_CLASS; break; } - /* Derefernece the object if we had referenced it */ + /* Dereference the object if we had referenced it */ if (Object) ObDereferenceObject (Object); - /* Return the length and status */ - if (ResultLength) *ResultLength = InfoLength; + /* Check if the caller wanted the return length */ + if (ResultLength) + { + /* Protect the write to user mode */ + _SEH_TRY + { + /* Write the length */ + *ResultLength = Length; + } + _SEH_HANDLE + { + /* Otherwise, get the exception code */ + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + } + + /* Return status */ return Status; } diff --git a/reactos/ntoskrnl/ob/obref.c b/reactos/ntoskrnl/ob/obref.c index 2b03ce7c745..26d52e31c63 100644 --- a/reactos/ntoskrnl/ob/obref.c +++ b/reactos/ntoskrnl/ob/obref.c @@ -88,11 +88,30 @@ ObfDereferenceObject(IN PVOID Object) /* Extract the object header */ Header = OBJECT_TO_OBJECT_HEADER(Object); + if (Header->PointerCount <= Header->HandleCount) + { + if (!wcscmp(Header->Type->Name.Buffer, L"Event")) + { + //KEBUGCHECK(0); + } + DPRINT1("Misbehaving object: %wZ\n", &Header->Type->Name); + } + /* Check whether the object can now be deleted. */ if (!(InterlockedDecrement(&Header->PointerCount))) { /* Sanity check */ - ASSERT(!Header->HandleCount); + if (wcscmp(Header->Type->Name.Buffer, L"Key")) + { + if(Header->HandleCount) + { + DPRINT1("Unexpected misbehaving object: %wZ\n", &Header->Type->Name); + } + } + else + { + DPRINT1("Cm needs fixing!\n"); + } /* Check if we're at PASSIVE */ if (KeGetCurrentIrql() == PASSIVE_LEVEL) diff --git a/reactos/ntoskrnl/ob/obsecure.c b/reactos/ntoskrnl/ob/obsecure.c index 8bd50fac378..d1d5126b570 100644 --- a/reactos/ntoskrnl/ob/obsecure.c +++ b/reactos/ntoskrnl/ob/obsecure.c @@ -15,7 +15,107 @@ #define TAG_SEC_QUERY TAG('O', 'b', 'S', 'q') -/* FUNCTIONS ***************************************************************/ +/* PRIVATE FUNCTIONS *********************************************************/ + +/*++ +* @name ObCheckObjectAccess +* +* The ObAssignSecurity routine +* +* @param Object +* +* +* @param AccessState +* +* +* @param Unknown +* +* +* @param AccessMode +* +* +* @param ReturnedStatus +* +* +* @return TRUE if access was granted, FALSE otherwise. +* +* @remarks None. +* +*--*/ +BOOLEAN +NTAPI +ObCheckObjectAccess(IN PVOID Object, + IN OUT PACCESS_STATE AccessState, + IN BOOLEAN Unknown, + IN KPROCESSOR_MODE AccessMode, + OUT PNTSTATUS ReturnedStatus) +{ + POBJECT_HEADER ObjectHeader; + POBJECT_TYPE ObjectType; + PSECURITY_DESCRIPTOR SecurityDescriptor = NULL; + BOOLEAN SdAllocated; + NTSTATUS Status; + BOOLEAN Result; + ACCESS_MASK GrantedAccess; + PPRIVILEGE_SET Privileges = NULL; + PAGED_CODE(); + + /* Get the object header and type */ + ObjectHeader = OBJECT_TO_OBJECT_HEADER(Object); + ObjectType = ObjectHeader->Type; + + /* Get security information */ + Status = ObGetObjectSecurity(Object, &SecurityDescriptor, &SdAllocated); + if (!NT_SUCCESS(Status)) + { + /* Return failure */ + *ReturnedStatus = Status; + return FALSE; + } + else if (!SecurityDescriptor) + { + /* Otherwise, if we don't actually have an SD, return success */ + *ReturnedStatus = Status; + return TRUE; + } + + /* Lock the security context */ + SeLockSubjectContext(&AccessState->SubjectSecurityContext); + + /* Now do the entire access check */ + Result = SeAccessCheck(SecurityDescriptor, + &AccessState->SubjectSecurityContext, + TRUE, + AccessState->RemainingDesiredAccess, + AccessState->PreviouslyGrantedAccess, + &Privileges, + &ObjectType->TypeInfo.GenericMapping, + AccessMode, + &GrantedAccess, + ReturnedStatus); + if (Privileges) + { + /* We got privileges, append them to teh access state and free them */ + Status = SeAppendPrivileges(AccessState, Privileges); + SeFreePrivileges(Privileges); + } + + /* Check if access was granted */ + if (Result) + { + /* Update the access state */ + AccessState->RemainingDesiredAccess &= ~(GrantedAccess | + MAXIMUM_ALLOWED); + AccessState->PreviouslyGrantedAccess |= GrantedAccess; + } + + /* We're done, unlock the context and release security */ + SeUnlockSubjectContext(&AccessState->SubjectSecurityContext); + ObReleaseObjectSecurity(SecurityDescriptor, SdAllocated); + return Result; +} + +/* PUBLIC FUNCTIONS **********************************************************/ /*++ * @name ObAssignSecurity diff --git a/reactos/ntoskrnl/ps/process.c b/reactos/ntoskrnl/ps/process.c index b6c9e3efa0a..628f7e205be 100644 --- a/reactos/ntoskrnl/ps/process.c +++ b/reactos/ntoskrnl/ps/process.c @@ -423,8 +423,8 @@ PspCreateProcess(OUT PHANDLE ProcessHandle, Status = ObInsertObject(Process, NULL, DesiredAccess, - 0, - NULL, + 1, + (PVOID*)&Process, &hProcess); if (NT_SUCCESS(Status)) {