[weiden] 13718: added more irql checks and secured access to buffers in symbolic link code
added more irql checks and secured access to buffers in symbolic link code Modified: trunk/reactos/ntoskrnl/cm/cm.h Modified: trunk/reactos/ntoskrnl/cm/ntfunc.c Modified: trunk/reactos/ntoskrnl/ob/dirobj.c Modified: trunk/reactos/ntoskrnl/ob/handle.c Modified: trunk/reactos/ntoskrnl/ob/namespc.c Modified: trunk/reactos/ntoskrnl/ob/ntobj.c Modified: trunk/reactos/ntoskrnl/ob/object.c Modified: trunk/reactos/ntoskrnl/ob/security.c Modified: trunk/reactos/ntoskrnl/ob/symlink.c Modified: trunk/reactos/ntoskrnl/po/power.c _____
Modified: trunk/reactos/ntoskrnl/cm/cm.h --- trunk/reactos/ntoskrnl/cm/cm.h 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/cm/cm.h 2005-02-22 21:09:54 UTC (rev 13718) @@ -431,6 +431,7 @@
PEX_CALLBACK_FUNCTION Function; PVOID Context; LARGE_INTEGER Cookie; + BOOLEAN PendingDelete; } REGISTRY_CALLBACK, *PREGISTRY_CALLBACK;
NTSTATUS _____
Modified: trunk/reactos/ntoskrnl/cm/ntfunc.c --- trunk/reactos/ntoskrnl/cm/ntfunc.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/cm/ntfunc.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -50,6 +50,7 @@
ExInitializeRundownProtection(&Callback->RundownRef); Callback->Function = Function; Callback->Context = Context; + Callback->PendingDelete = FALSE;
/* add it to the callback list and receive a cookie for the callback */ ExAcquireFastMutex(&CmiCallbackLock); @@ -87,22 +88,32 @@ CurrentCallback = CONTAINING_RECORD(CurrentEntry, REGISTRY_CALLBACK, ListEntry); if(CurrentCallback->Cookie.QuadPart == Cookie.QuadPart) { - /* found the callback, don't unlink it from the list yet so we don't screw - the calling loop */ - ExReleaseFastMutex(&CmiCallbackLock); + if(!CurrentCallback->PendingDelete) + { + /* found the callback, don't unlink it from the list yet so we don't screw + the calling loop */ + CurrentCallback->PendingDelete = TRUE; + ExReleaseFastMutex(&CmiCallbackLock);
- /* if the callback is currently executing, wait until it finished */ - ExWaitForRundownProtectionRelease(&CurrentCallback->RundownRef); + /* if the callback is currently executing, wait until it finished */ + ExWaitForRundownProtectionRelease(&CurrentCallback->RundownRef);
- /* time to unlink it. It's now safe because every attempt to acquire a - runtime protection on this callback will fail */ - ExAcquireFastMutex(&CmiCallbackLock); - RemoveEntryList(&CurrentCallback->ListEntry); - ExReleaseFastMutex(&CmiCallbackLock); + /* time to unlink it. It's now safe because every attempt to acquire a + runtime protection on this callback will fail */ + ExAcquireFastMutex(&CmiCallbackLock); + RemoveEntryList(&CurrentCallback->ListEntry); + ExReleaseFastMutex(&CmiCallbackLock);
- /* free the callback */ - ExFreePool(CurrentCallback); - return STATUS_SUCCESS; + /* free the callback */ + ExFreePool(CurrentCallback); + return STATUS_SUCCESS; + } + else + { + /* pending delete, pretend like it already is deleted */ + ExReleaseFastMutex(&CmiCallbackLock); + return STATUS_UNSUCCESSFUL; + } } }
@@ -127,7 +138,8 @@ PREGISTRY_CALLBACK CurrentCallback;
CurrentCallback = CONTAINING_RECORD(CurrentEntry, REGISTRY_CALLBACK, ListEntry); - if(ExAcquireRundownProtectionEx(&CurrentCallback->RundownRef, 1)) + if(!CurrentCallback->PendingDelete && + ExAcquireRundownProtectionEx(&CurrentCallback->RundownRef, 1)) { NTSTATUS Status;
_____
Modified: trunk/reactos/ntoskrnl/ob/dirobj.c --- trunk/reactos/ntoskrnl/ob/dirobj.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/dirobj.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -50,6 +50,8 @@
KPROCESSOR_MODE PreviousMode; NTSTATUS Status = STATUS_SUCCESS;
+ PAGED_CODE(); + PreviousMode = ExGetPreviousMode();
if(PreviousMode != KernelMode) @@ -169,6 +171,8 @@ ULONG NextEntry = 0; NTSTATUS Status = STATUS_SUCCESS;
+ PAGED_CODE(); + PreviousMode = ExGetPreviousMode();
if(PreviousMode != KernelMode) @@ -427,6 +431,8 @@ KPROCESSOR_MODE PreviousMode; NTSTATUS Status = STATUS_SUCCESS;
+ PAGED_CODE(); + DPRINT("NtCreateDirectoryObject(DirectoryHandle %x, " "DesiredAccess %x, ObjectAttributes %x\n", DirectoryHandle, DesiredAccess, ObjectAttributes); _____
Modified: trunk/reactos/ntoskrnl/ob/handle.c --- trunk/reactos/ntoskrnl/ob/handle.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/handle.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -287,7 +287,7 @@
HANDLE TargetHandle; NTSTATUS Status;
- ASSERT_IRQL(PASSIVE_LEVEL); + PAGED_CODE();
Status = ObReferenceObjectByHandle(SourceProcessHandle, PROCESS_DUP_HANDLE, @@ -552,6 +552,8 @@ PHANDLE_TABLE HandleTable; POBJECT_HEADER Header; HANDLE_BLOCK *Block; + + PAGED_CODE();
DPRINT("ObDeleteHandle(Handle %x)\n",Handle);
@@ -630,6 +632,8 @@ HANDLE_BLOCK* new_blk = NULL; PHANDLE_TABLE HandleTable; KIRQL oldlvl; + + PAGED_CODE();
DPRINT("ObCreateHandle(Process %x, obj %x)\n",Process,ObjectBody);
@@ -723,6 +727,8 @@ PEPROCESS Process; KIRQL oldIrql; PHANDLE_ENTRY HandleEntry; + + PAGED_CODE();
DPRINT("ObQueryObjectAuditingByHandle(Handle %x)\n", Handle);
@@ -777,7 +783,7 @@ ULONG Attributes; NTSTATUS Status;
- ASSERT_IRQL(PASSIVE_LEVEL); + PAGED_CODE();
DPRINT("ObReferenceObjectByHandle(Handle %x, DesiredAccess %x, " "ObjectType %x, AccessMode %d, Object %x)\n",Handle,DesiredAccess, @@ -930,7 +936,7 @@ POBJECT_HEADER Header; NTSTATUS Status;
- ASSERT_IRQL(PASSIVE_LEVEL); + PAGED_CODE();
DPRINT("NtClose(Handle %x)\n",Handle);
@@ -966,6 +972,8 @@ { POBJECT_HEADER ObjectHeader; ACCESS_MASK Access; + + PAGED_CODE();
Access = DesiredAccess; ObjectHeader = BODY_TO_HEADER(Object); _____
Modified: trunk/reactos/ntoskrnl/ob/namespc.c --- trunk/reactos/ntoskrnl/ob/namespc.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/namespc.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -55,6 +55,8 @@
UNICODE_STRING RemainingPath; OBJECT_ATTRIBUTES ObjectAttributes; NTSTATUS Status; + + PAGED_CODE();
InitializeObjectAttributes(&ObjectAttributes, ObjectPath, @@ -126,6 +128,8 @@ UNICODE_STRING RemainingPath; PVOID Object = NULL; NTSTATUS Status; + + PAGED_CODE();
DPRINT("ObOpenObjectByName(...)\n");
_____
Modified: trunk/reactos/ntoskrnl/ob/ntobj.c --- trunk/reactos/ntoskrnl/ob/ntobj.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/ntobj.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -37,6 +37,8 @@
{ PVOID Object; NTSTATUS Status; + + PAGED_CODE();
if (ObjectInformationClass != ObjectHandleInformation) return STATUS_INVALID_INFO_CLASS; @@ -88,6 +90,8 @@ ULONG InfoLength; PVOID Object; NTSTATUS Status; + + PAGED_CODE();
Status = ObReferenceObjectByHandle (ObjectHandle, 0, @@ -260,6 +264,8 @@ { PVOID ObjectBody; NTSTATUS Status; + + PAGED_CODE();
Status = ObReferenceObjectByHandle(ObjectHandle, 0, @@ -299,6 +305,8 @@ { PVOID ObjectBody; NTSTATUS Status; + + PAGED_CODE();
Status = ObReferenceObjectByHandle(ObjectHandle, 0, _____
Modified: trunk/reactos/ntoskrnl/ob/object.c --- trunk/reactos/ntoskrnl/ob/object.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/object.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -346,6 +346,8 @@
UNICODE_STRING PathString; ULONG Attributes; PUNICODE_STRING ObjectName; + + PAGED_CODE();
DPRINT("ObFindObject(ObjectAttributes %x, ReturnedObject %x, " "RemainingPath %x)\n",ObjectAttributes,ReturnedObject,RemainingPath); @@ -483,6 +485,8 @@ POBJECT_HEADER ObjectHeader; ULONG LocalReturnLength; NTSTATUS Status; + + PAGED_CODE();
*ReturnLength = 0;
@@ -611,7 +615,7 @@ PSECURITY_DESCRIPTOR NewSecurityDescriptor = NULL; SECURITY_SUBJECT_CONTEXT SubjectContext;
- ASSERT_IRQL(APC_LEVEL); + PAGED_CODE();
if(ObjectAttributesAccessMode == UserMode && ObjectAttributes != NULL) { @@ -814,6 +818,8 @@ IN KPROCESSOR_MODE AccessMode) { POBJECT_HEADER Header; + + /* NOTE: should be possible to reference an object above APC_LEVEL! */
DPRINT("ObReferenceObjectByPointer(Object %x, ObjectType %x)\n", Object,ObjectType); @@ -876,6 +882,8 @@ { NTSTATUS Status;
+ PAGED_CODE(); + DPRINT("ObOpenObjectByPointer()\n");
Status = ObReferenceObjectByPointer(Object, @@ -1117,6 +1125,8 @@ ObGetObjectPointerCount(PVOID Object) { POBJECT_HEADER Header; + + PAGED_CODE();
ASSERT(Object); Header = BODY_TO_HEADER(Object); @@ -1142,6 +1152,8 @@ ObGetObjectHandleCount(PVOID Object) { POBJECT_HEADER Header; + + PAGED_CODE();
ASSERT(Object); Header = BODY_TO_HEADER(Object); _____
Modified: trunk/reactos/ntoskrnl/ob/security.c --- trunk/reactos/ntoskrnl/ob/security.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/security.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -1,4 +1,4 @@
-/* $Id:$ +/* $Id$ * * COPYRIGHT: See COPYING in the top level directory * PROJECT: ReactOS kernel @@ -27,6 +27,8 @@ { PSECURITY_DESCRIPTOR NewDescriptor; NTSTATUS Status; + + PAGED_CODE();
/* Build the new security descriptor */ Status = SeAssignSecurity(SecurityDescriptor, @@ -73,6 +75,8 @@ POBJECT_HEADER Header; ULONG Length; NTSTATUS Status; + + PAGED_CODE();
Header = BODY_TO_HEADER(Object); if (Header->ObjectType == NULL) @@ -129,6 +133,8 @@ ObReleaseObjectSecurity(IN PSECURITY_DESCRIPTOR SecurityDescriptor, IN BOOLEAN MemoryAllocated) { + PAGED_CODE(); + if (SecurityDescriptor == NULL) return;
@@ -156,6 +162,8 @@ POBJECT_HEADER Header; PVOID Object; NTSTATUS Status; + + PAGED_CODE();
DPRINT("NtQuerySecurityObject() called\n");
@@ -226,6 +234,8 @@ ULONG Control = 0; ULONG_PTR Current; NTSTATUS Status; + + PAGED_CODE();
DPRINT("NtSetSecurityObject() called\n");
_____
Modified: trunk/reactos/ntoskrnl/ob/symlink.c --- trunk/reactos/ntoskrnl/ob/symlink.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/ob/symlink.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -206,61 +206,104 @@
IN POBJECT_ATTRIBUTES ObjectAttributes, IN PUNICODE_STRING LinkTarget) { + HANDLE hLink; PSYMLINK_OBJECT SymbolicLink; - NTSTATUS Status; + UNICODE_STRING CapturedLinkTarget; + KPROCESSOR_MODE PreviousMode; + NTSTATUS Status = STATUS_SUCCESS;
- ASSERT_IRQL(PASSIVE_LEVEL); + PAGED_CODE(); + + PreviousMode = ExGetPreviousMode();
+ if(PreviousMode != KernelMode) + { + _SEH_TRY + { + ProbeForWrite(LinkHandle, + sizeof(HANDLE), + sizeof(ULONG)); + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + if(!NT_SUCCESS(Status)) + { + return Status; + } + } + + Status = RtlCaptureUnicodeString(&CapturedLinkTarget, + PreviousMode, + PagedPool, + FALSE, + LinkTarget); + if(!NT_SUCCESS(Status)) + { + DPRINT1("NtCreateSymbolicLinkObject: Capturing the target link failed!\n"); + return Status; + } + DPRINT("NtCreateSymbolicLinkObject(LinkHandle %p, DesiredAccess %ul, ObjectAttributes %p, LinkTarget %wZ)\n", LinkHandle, DesiredAccess, ObjectAttributes, - LinkTarget); + &CapturedLinkTarget);
Status = ObCreateObject(ExGetPreviousMode(), ObSymbolicLinkType, ObjectAttributes, - ExGetPreviousMode(), + PreviousMode, NULL, sizeof(SYMLINK_OBJECT), 0, 0, (PVOID*)&SymbolicLink); - if (!NT_SUCCESS(Status)) - { - return(Status); - } + if (NT_SUCCESS(Status)) + { + SymbolicLink->TargetName.Length = 0; + SymbolicLink->TargetName.MaximumLength = + ((wcslen(LinkTarget->Buffer) + 1) * sizeof(WCHAR)); + SymbolicLink->TargetName.Buffer = + ExAllocatePoolWithTag(NonPagedPool, + SymbolicLink->TargetName.MaximumLength, + TAG_SYMLINK_TARGET); + RtlCopyUnicodeString(&SymbolicLink->TargetName, + &CapturedLinkTarget);
- Status = ObInsertObject ((PVOID)SymbolicLink, - NULL, - DesiredAccess, - 0, - NULL, - LinkHandle); - if (!NT_SUCCESS(Status)) + DPRINT("DeviceName %S\n", SymbolicLink->TargetName.Buffer); + + ZwQuerySystemTime (&SymbolicLink->CreateTime); + + Status = ObInsertObject ((PVOID)SymbolicLink, + NULL, + DesiredAccess, + 0, + NULL, + &hLink); + if (NT_SUCCESS(Status)) { - ObDereferenceObject (SymbolicLink); - return Status; + _SEH_TRY + { + *LinkHandle = hLink; + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; } + ObDereferenceObject(SymbolicLink); + } + + RtlReleaseCapturedUnicodeString(&CapturedLinkTarget, + PreviousMode, + FALSE);
- SymbolicLink->TargetName.Length = 0; - SymbolicLink->TargetName.MaximumLength = - ((wcslen(LinkTarget->Buffer) + 1) * sizeof(WCHAR)); - SymbolicLink->TargetName.Buffer = - ExAllocatePoolWithTag(NonPagedPool, - SymbolicLink->TargetName.MaximumLength, - TAG_SYMLINK_TARGET); - RtlCopyUnicodeString(&SymbolicLink->TargetName, - LinkTarget); - - DPRINT("DeviceName %S\n", SymbolicLink->TargetName.Buffer); - - ZwQuerySystemTime (&SymbolicLink->CreateTime); - - DPRINT("%s() = STATUS_SUCCESS\n",__FUNCTION__); - ObDereferenceObject(SymbolicLink); - - return(STATUS_SUCCESS); + return Status; }
@@ -282,16 +325,58 @@ IN ACCESS_MASK DesiredAccess, IN POBJECT_ATTRIBUTES ObjectAttributes) { + HANDLE hLink; + KPROCESSOR_MODE PreviousMode; + NTSTATUS Status = STATUS_SUCCESS; + + PAGED_CODE(); + + PreviousMode = ExGetPreviousMode(); + + if(PreviousMode != KernelMode) + { + _SEH_TRY + { + ProbeForWrite(LinkHandle, + sizeof(HANDLE), + sizeof(ULONG)); + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + if(!NT_SUCCESS(Status)) + { + return Status; + } + } + DPRINT("NtOpenSymbolicLinkObject (Name %wZ)\n", ObjectAttributes->ObjectName);
- return(ObOpenObjectByName(ObjectAttributes, - ObSymbolicLinkType, - NULL, - (KPROCESSOR_MODE)KeGetPreviousMode(), - DesiredAccess, - NULL, - LinkHandle)); + Status = ObOpenObjectByName(ObjectAttributes, + ObSymbolicLinkType, + NULL, + PreviousMode, + DesiredAccess, + NULL, + &hLink); + if(NT_SUCCESS(Status)) + { + _SEH_TRY + { + *LinkHandle = hLink; + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + } + + return Status; }
@@ -313,38 +398,93 @@ OUT PUNICODE_STRING LinkTarget, OUT PULONG ResultLength OPTIONAL) { + UNICODE_STRING SafeLinkTarget; PSYMLINK_OBJECT SymlinkObject; - NTSTATUS Status; + KPROCESSOR_MODE PreviousMode; + NTSTATUS Status = STATUS_SUCCESS; + + PAGED_CODE(); + + PreviousMode = ExGetPreviousMode(); + + if(PreviousMode != KernelMode) + { + _SEH_TRY + { + /* probe the unicode string and buffers supplied */ + ProbeForWrite(LinkTarget, + sizeof(UNICODE_STRING), + sizeof(ULONG)); + SafeLinkTarget = *LinkTarget; + ProbeForWrite(SafeLinkTarget.Buffer, + SafeLinkTarget.MaximumLength, + sizeof(WCHAR));
+ if(ResultLength != NULL) + { + ProbeForWrite(ResultLength, + sizeof(ULONG), + sizeof(ULONG)); + } + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + if(!NT_SUCCESS(Status)) + { + return Status; + } + } + else + { + SafeLinkTarget = *LinkTarget; + } + Status = ObReferenceObjectByHandle(LinkHandle, SYMBOLIC_LINK_QUERY, ObSymbolicLinkType, - (KPROCESSOR_MODE)KeGetPreviousMode(), + PreviousMode, (PVOID *)&SymlinkObject, NULL); - if (!NT_SUCCESS(Status)) + if (NT_SUCCESS(Status)) + { + ULONG LengthRequired = SymlinkObject->TargetName.Length + sizeof(WCHAR); + + _SEH_TRY { - return Status; + if(SafeLinkTarget.MaximumLength >= LengthRequired) + { + /* don't pass TargetLink to RtlCopyUnicodeString here because the caller + might have modified the structure which could lead to a copy into + kernel memory! */ + RtlCopyUnicodeString(&SafeLinkTarget, + &SymlinkObject->TargetName); + SafeLinkTarget.Buffer[SafeLinkTarget.Length / sizeof(WCHAR)] = L'\0'; + /* copy back the new UNICODE_STRING structure */ + *LinkTarget = SafeLinkTarget; + } + else + { + Status = STATUS_BUFFER_TOO_SMALL; + } + + if(ResultLength != NULL) + { + *ResultLength = LengthRequired; + } } - - if (ResultLength != NULL) + _SEH_HANDLE { - *ResultLength = (ULONG)SymlinkObject->TargetName.Length + sizeof(WCHAR); + Status = _SEH_GetExceptionCode(); } + _SEH_END;
- if (LinkTarget->MaximumLength >= SymlinkObject->TargetName.Length + sizeof(WCHAR)) - { - RtlCopyUnicodeString(LinkTarget, - &SymlinkObject->TargetName); - Status = STATUS_SUCCESS; - } - else - { - Status = STATUS_BUFFER_TOO_SMALL; - } + ObDereferenceObject(SymlinkObject); + }
- ObDereferenceObject(SymlinkObject); - return Status; }
_____
Modified: trunk/reactos/ntoskrnl/po/power.c --- trunk/reactos/ntoskrnl/po/power.c 2005-02-22 20:08:06 UTC (rev 13717) +++ trunk/reactos/ntoskrnl/po/power.c 2005-02-22 21:09:54 UTC (rev 13718) @@ -91,6 +91,8 @@
IN POWER_STATE State) { POWER_STATE ps; + + ASSERT_IRQL(DISPATCH_LEVEL);
ps.SystemState = PowerSystemWorking; // Fully on ps.DeviceState = PowerDeviceD0; // Fully on @@ -228,6 +230,8 @@ ) { NTSTATUS Status; + + PAGED_CODE();
DPRINT("NtPowerInformation(PowerInformationLevel 0x%x, InputBuffer 0x%x, " "InputBufferLength 0x%x, OutputBuffer 0x%x, OutputBufferLength 0x%x)\n",
-
weiden＠svn.reactos.com