Ros-diffs October 2018

ros-diffs@reactos.org

21 participants
354 discussions

02/05: [NTOSKRNL] When mapping data, try to find if there's already a BCB

by Pierre Schweitzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=9fc75c11322e45fa02a8b… commit 9fc75c11322e45fa02a8b65780f4f53b0ffbb9fb Author: Pierre Schweitzer <pierre(a)reactos.org> AuthorDate: Fri Oct 5 21:01:02 2018 +0200 Commit: Pierre Schweitzer <pierre(a)reactos.org> CommitDate: Fri Oct 5 21:26:16 2018 +0200 [NTOSKRNL] When mapping data, try to find if there's already a BCB If so, return such BCB instead of creating a new one. This will allow (at some point) to be more consistent in case of concurrent mapping. This fixes a few CcMapData tests. --- ntoskrnl/cc/pin.c | 91 +++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 16 deletions(-) diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c index 53d51ed824..0d23211da9 100644 --- a/ntoskrnl/cc/pin.c +++ b/ntoskrnl/cc/pin.c @@ -35,7 +35,7 @@ static BOOLEAN NTAPI CcpMapData( - IN PFILE_OBJECT FileObject, + IN PROS_SHARED_CACHE_MAP SharedCacheMap, IN PLARGE_INTEGER FileOffset, IN ULONG Length, IN ULONG Flags, @@ -44,7 +44,6 @@ CcpMapData( { LONGLONG ReadOffset; BOOLEAN Valid; - PROS_SHARED_CACHE_MAP SharedCacheMap; PROS_VACB Vacb; NTSTATUS Status; PINTERNAL_BCB iBcb; @@ -53,13 +52,6 @@ CcpMapData( ReadOffset = FileOffset->QuadPart; - ASSERT(FileObject); - ASSERT(FileObject->SectionObjectPointer); - ASSERT(FileObject->SectionObjectPointer->SharedCacheMap); - - SharedCacheMap = FileObject->SectionObjectPointer->SharedCacheMap; - ASSERT(SharedCacheMap); - DPRINT("SectionSize %I64x, FileSize %I64x\n", SharedCacheMap->SectionSize.QuadPart, SharedCacheMap->FileSize.QuadPart); @@ -67,7 +59,7 @@ CcpMapData( if (ReadOffset % VACB_MAPPING_GRANULARITY + Length > VACB_MAPPING_GRANULARITY) { CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx -> FALSE\n", - FileObject, FileOffset, Length, Flags); + SharedCacheMap->FileObject, FileOffset, Length, Flags); return FALSE; } @@ -92,7 +84,7 @@ CcpMapData( if (!NT_SUCCESS(Status)) { CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx -> FALSE\n", - FileObject, FileOffset, Length, Flags); + SharedCacheMap->FileObject, FileOffset, Length, Flags); ExRaiseStatus(Status); return FALSE; } @@ -103,7 +95,7 @@ CcpMapData( { CcRosReleaseVacb(SharedCacheMap, Vacb, FALSE, FALSE, FALSE); CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx -> FALSE\n", - FileObject, FileOffset, Length, Flags); + SharedCacheMap->FileObject, FileOffset, Length, Flags); return FALSE; } @@ -112,7 +104,7 @@ CcpMapData( { CcRosReleaseVacb(SharedCacheMap, Vacb, FALSE, FALSE, FALSE); CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx -> FALSE\n", - FileObject, FileOffset, Length, Flags); + SharedCacheMap->FileObject, FileOffset, Length, Flags); ExRaiseStatus(Status); return FALSE; } @@ -124,7 +116,7 @@ CcpMapData( { CcRosReleaseVacb(SharedCacheMap, Vacb, TRUE, FALSE, FALSE); CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx -> FALSE\n", - FileObject, FileOffset, Length, Flags); + SharedCacheMap->FileObject, FileOffset, Length, Flags); ExRaiseStatus(STATUS_INSUFFICIENT_RESOURCES); return FALSE; } @@ -148,6 +140,40 @@ CcpMapData( return TRUE; } +static +PINTERNAL_BCB +NTAPI +CcpFindBcb( + IN PROS_SHARED_CACHE_MAP SharedCacheMap, + IN PLARGE_INTEGER FileOffset, + IN ULONG Length, + IN BOOLEAN Pinned) +{ + PINTERNAL_BCB Bcb; + BOOLEAN Found = FALSE; + PLIST_ENTRY NextEntry; + + for (NextEntry = SharedCacheMap->BcbList.Flink; + NextEntry != &SharedCacheMap->BcbList; + NextEntry = NextEntry->Flink) + { + Bcb = CONTAINING_RECORD(NextEntry, INTERNAL_BCB, BcbEntry); + + if (Bcb->PFCB.MappedFileOffset.QuadPart <= FileOffset->QuadPart && + (Bcb->PFCB.MappedFileOffset.QuadPart + Bcb->PFCB.MappedLength) >= + (FileOffset->QuadPart + Length)) + { + if ((Pinned && Bcb->PinCount > 0) || (!Pinned && Bcb->PinCount == 0)) + { + Found = TRUE; + break; + } + } + } + + return (Found ? Bcb : NULL); +} + /* * @implemented */ @@ -162,11 +188,21 @@ CcMapData ( OUT PVOID *pBuffer) { BOOLEAN Ret; + KIRQL OldIrql; + PINTERNAL_BCB iBcb; + PROS_SHARED_CACHE_MAP SharedCacheMap; DPRINT("CcMapData(FileObject 0x%p, FileOffset %I64x, Length %lu, Flags 0x%lx," " pBcb 0x%p, pBuffer 0x%p)\n", FileObject, FileOffset->QuadPart, Length, Flags, pBcb, pBuffer); + ASSERT(FileObject); + ASSERT(FileObject->SectionObjectPointer); + ASSERT(FileObject->SectionObjectPointer->SharedCacheMap); + + SharedCacheMap = FileObject->SectionObjectPointer->SharedCacheMap; + ASSERT(SharedCacheMap); + if (Flags & MAP_WAIT) { ++CcMapDataWait; @@ -176,7 +212,21 @@ CcMapData ( ++CcMapDataNoWait; } - Ret = CcpMapData(FileObject, FileOffset, Length, Flags, pBcb, pBuffer); + KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql); + iBcb = CcpFindBcb(SharedCacheMap, FileOffset, Length, FALSE); + KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql); + + if (iBcb == NULL) + { + Ret = CcpMapData(SharedCacheMap, FileOffset, Length, Flags, pBcb, pBuffer); + } + else + { + ++iBcb->RefCount; + *pBcb = iBcb; + *pBuffer = (PUCHAR)iBcb->Vacb->BaseAddress + FileOffset->QuadPart % VACB_MAPPING_GRANULARITY; + Ret = TRUE; + } CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx -> %d Bcb=%p\n", FileObject, FileOffset, Length, Flags, Ret, *pBcb); @@ -249,9 +299,18 @@ CcPinRead ( OUT PVOID * Bcb, OUT PVOID * Buffer) { + PROS_SHARED_CACHE_MAP SharedCacheMap; + CCTRACE(CC_API_DEBUG, "FileOffset=%p FileOffset=%p Length=%lu Flags=0x%lx\n", FileObject, FileOffset, Length, Flags); + ASSERT(FileObject); + ASSERT(FileObject->SectionObjectPointer); + ASSERT(FileObject->SectionObjectPointer->SharedCacheMap); + + SharedCacheMap = FileObject->SectionObjectPointer->SharedCacheMap; + ASSERT(SharedCacheMap); + if (Flags & PIN_WAIT) { ++CcPinReadWait; @@ -262,7 +321,7 @@ CcPinRead ( } /* Map first */ - if (!CcpMapData(FileObject, FileOffset, Length, Flags, Bcb, Buffer)) + if (!CcpMapData(SharedCacheMap, FileOffset, Length, Flags, Bcb, Buffer)) { return FALSE; }

5 years, 11 months

01/05: [NTOSKRNL] Move the PinCount out of the VACB to the BCB

by Pierre Schweitzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f2849476220161e202e25… commit f2849476220161e202e25b000d8c68a9d4123e00 Author: Pierre Schweitzer <pierre(a)reactos.org> AuthorDate: Fri Oct 5 19:43:10 2018 +0200 Commit: Pierre Schweitzer <pierre(a)reactos.org> CommitDate: Fri Oct 5 21:26:16 2018 +0200 [NTOSKRNL] Move the PinCount out of the VACB to the BCB Given current ReactOS implementation, a VACB can be pinned several times, with different BCB. In next commits, a single BCB will be able to be pinned several times. That would lead to severe inconsistencies in counting and thus corruption. --- ntoskrnl/cc/pin.c | 23 ++++++++++------------- ntoskrnl/cc/view.c | 8 +++----- ntoskrnl/include/internal/cc.h | 4 +--- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c index 3c8747e76b..53d51ed824 100644 --- a/ntoskrnl/cc/pin.c +++ b/ntoskrnl/cc/pin.c @@ -136,7 +136,7 @@ CcpMapData( iBcb->PFCB.MappedFileOffset = *FileOffset; iBcb->Vacb = Vacb; iBcb->Dirty = FALSE; - iBcb->Pinned = FALSE; + iBcb->PinCount = 0; iBcb->RefCount = 1; ExInitializeResourceLite(&iBcb->Lock); *pBcb = (PVOID)iBcb; @@ -215,10 +215,9 @@ CcPinMappedData ( } iBcb = *Bcb; - ASSERT(iBcb->Pinned == FALSE); + ASSERT(iBcb->PinCount == 0); - iBcb->Pinned = TRUE; - iBcb->Vacb->PinCount++; + iBcb->PinCount++; if (BooleanFlagOn(Flags, PIN_EXCLUSIVE)) { @@ -231,8 +230,7 @@ CcPinMappedData ( if (!Result) { - iBcb->Pinned = FALSE; - iBcb->Vacb->PinCount--; + iBcb->PinCount--; } return Result; @@ -353,11 +351,10 @@ CcUnpinDataForThread ( CCTRACE(CC_API_DEBUG, "Bcb=%p ResourceThreadId=%lu\n", Bcb, ResourceThreadId); - if (iBcb->Pinned) + if (iBcb->PinCount != 0) { ExReleaseResourceForThreadLite(&iBcb->Lock, ResourceThreadId); - iBcb->Pinned = FALSE; - iBcb->Vacb->PinCount--; + iBcb->PinCount--; } if (--iBcb->RefCount == 0) @@ -365,6 +362,7 @@ CcUnpinDataForThread ( KIRQL OldIrql; PROS_SHARED_CACHE_MAP SharedCacheMap; + ASSERT(iBcb->PinCount == 0); SharedCacheMap = iBcb->Vacb->SharedCacheMap; CcRosReleaseVacb(SharedCacheMap, iBcb->Vacb, @@ -432,12 +430,11 @@ CcUnpinRepinnedBcb ( IoStatus->Status = STATUS_SUCCESS; } - if (iBcb->Pinned) + if (iBcb->PinCount != 0) { ExReleaseResourceLite(&iBcb->Lock); - iBcb->Pinned = FALSE; - iBcb->Vacb->PinCount--; - ASSERT(iBcb->Vacb->PinCount == 0); + iBcb->PinCount--; + ASSERT(iBcb->PinCount == 0); } SharedCacheMap = iBcb->Vacb->SharedCacheMap; diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c index e3c8285254..b9c4930986 100644 --- a/ntoskrnl/cc/view.c +++ b/ntoskrnl/cc/view.c @@ -808,7 +808,6 @@ CcRosCreateVacb ( #endif current->MappedCount = 0; current->ReferenceCount = 0; - current->PinCount = 0; InitializeListHead(&current->CacheMapVacbListEntry); InitializeListHead(&current->DirtyVacbListEntry); InitializeListHead(&current->VacbLruListEntry); @@ -1056,16 +1055,15 @@ CcRosInternalFreeVacb ( NULL); MmUnlockAddressSpace(MmGetKernelAddressSpace()); - if (Vacb->PinCount != 0 || Vacb->ReferenceCount != 0) + if (Vacb->ReferenceCount != 0) { - DPRINT1("Invalid free: %ld, %ld\n", Vacb->ReferenceCount, Vacb->PinCount); + DPRINT1("Invalid free: %ld\n", Vacb->ReferenceCount); if (Vacb->SharedCacheMap->FileObject && Vacb->SharedCacheMap->FileObject->FileName.Length) { DPRINT1("For file: %wZ\n", &Vacb->SharedCacheMap->FileObject->FileName); } } - ASSERT(Vacb->PinCount == 0); ASSERT(Vacb->ReferenceCount == 0); ASSERT(IsListEmpty(&Vacb->CacheMapVacbListEntry)); ASSERT(IsListEmpty(&Vacb->DirtyVacbListEntry)); @@ -1229,7 +1227,7 @@ CcRosDeleteFileCache ( { DPRINT1("Leaking VACB %p attached to %p (%I64d)\n", current, FileObject, current->FileOffset.QuadPart); DPRINT1("There are: %d references left\n", Refs); - DPRINT1("Pin: %d, Map: %d\n", current->PinCount, current->MappedCount); + DPRINT1("Map: %d\n", current->MappedCount); DPRINT1("Dirty: %d\n", current->Dirty); if (FileObject->FileName.Length != 0) { diff --git a/ntoskrnl/include/internal/cc.h b/ntoskrnl/include/internal/cc.h index 0c12004326..16dd1f67f0 100644 --- a/ntoskrnl/include/internal/cc.h +++ b/ntoskrnl/include/internal/cc.h @@ -221,8 +221,6 @@ typedef struct _ROS_VACB LARGE_INTEGER FileOffset; /* Number of references. */ volatile ULONG ReferenceCount; - /* How many times was it pinned? */ - LONG PinCount; /* Pointer to the shared cache map for the file which this view maps data for. */ PROS_SHARED_CACHE_MAP SharedCacheMap; /* Pointer to the next VACB in a chain. */ @@ -235,7 +233,7 @@ typedef struct _INTERNAL_BCB PUBLIC_BCB PFCB; PROS_VACB Vacb; BOOLEAN Dirty; - BOOLEAN Pinned; + ULONG PinCount; CSHORT RefCount; /* (At offset 0x34 on WinNT4) */ LIST_ENTRY BcbEntry; } INTERNAL_BCB, *PINTERNAL_BCB;

5 years, 11 months

01/01: [DEVMGR] Hackfix CORE-5643

by Eric Kohl

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f9739601b380b9bd8ab43… commit f9739601b380b9bd8ab43d8fa87c940c65517d94 Author: Eric Kohl <eric.kohl(a)reactos.org> AuthorDate: Fri Oct 5 15:09:27 2018 +0200 Commit: Eric Kohl <eric.kohl(a)reactos.org> CommitDate: Fri Oct 5 15:09:27 2018 +0200 [DEVMGR] Hackfix CORE-5643 --- dll/win32/devmgr/devmgmt/DeviceView.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dll/win32/devmgr/devmgmt/DeviceView.cpp b/dll/win32/devmgr/devmgmt/DeviceView.cpp index 4be0c2e75f..690609c73c 100644 --- a/dll/win32/devmgr/devmgmt/DeviceView.cpp +++ b/dll/win32/devmgr/devmgmt/DeviceView.cpp @@ -1206,6 +1206,11 @@ CDeviceView::RefreshDeviceList() // create a new device node and add it to the list DeviceNode = new CDeviceNode(DeviceInfoData.DevInst, &m_ImageListData); + /* FIXME: Start of Hack for CORE-5643 */ + if (!DeviceNode->IsInstalled()) + continue; + /* FIXME: End of Hack for CORE-5643 */ + if (DeviceNode->SetupNode()) { m_DeviceNodeList.AddTail(DeviceNode);

5 years, 11 months

01/01: [SERVICES] Ignore the case when we compare account names

by Eric Kohl

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=9159190039eb941a7dfd3… commit 9159190039eb941a7dfd3bc5917d4c9776029a3a Author: Eric Kohl <eric.kohl(a)reactos.org> AuthorDate: Fri Oct 5 11:40:55 2018 +0200 Commit: Eric Kohl <eric.kohl(a)reactos.org> CommitDate: Fri Oct 5 12:04:21 2018 +0200 [SERVICES] Ignore the case when we compare account names --- base/system/services/database.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/base/system/services/database.c b/base/system/services/database.c index c4c372c320..ea91a5cb45 100644 --- a/base/system/services/database.c +++ b/base/system/services/database.c @@ -151,16 +151,23 @@ ScmIsSameServiceAccount( _In_ PCWSTR pszAccountName1, _In_ PCWSTR pszAccountName2) { - if (pszAccountName1 == NULL && pszAccountName2 == NULL) + if (pszAccountName1 == NULL && + pszAccountName2 == NULL) return TRUE; - if (pszAccountName1 == NULL && pszAccountName2 && wcscmp(pszAccountName2, L"LocalSystem") == 0) + if (pszAccountName1 == NULL && + pszAccountName2 != NULL && + _wcsicmp(pszAccountName2, L"LocalSystem") == 0) return TRUE; - if (pszAccountName2 == NULL && pszAccountName1 && wcscmp(pszAccountName1, L"LocalSystem") == 0) + if (pszAccountName1 != NULL && + pszAccountName2 == NULL && + _wcsicmp(pszAccountName1, L"LocalSystem") == 0) return TRUE; - if (pszAccountName1 && pszAccountName2 && wcscmp(pszAccountName1, pszAccountName2) == 0) + if (pszAccountName1 != NULL && + pszAccountName2 != NULL && + _wcsicmp(pszAccountName1, pszAccountName2) == 0) return TRUE; return FALSE; @@ -173,7 +180,7 @@ ScmIsLocalSystemAccount( _In_ PCWSTR pszAccountName) { if (pszAccountName == NULL || - wcscmp(pszAccountName, L"LocalSystem") == 0) + _wcsicmp(pszAccountName, L"LocalSystem") == 0) return TRUE; return FALSE;

5 years, 11 months

04/04: [NTOSKRNL] On open, verify and validate the hint device object if any

by Pierre Schweitzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=0b713d4fa0781ecce0367… commit 0b713d4fa0781ecce03673ce9b3eb64d7a683382 Author: Pierre Schweitzer <pierre(a)reactos.org> AuthorDate: Fri Oct 5 10:49:11 2018 +0200 Commit: Pierre Schweitzer <pierre(a)reactos.org> CommitDate: Fri Oct 5 10:49:59 2018 +0200 [NTOSKRNL] On open, verify and validate the hint device object if any --- ntoskrnl/io/iomgr/file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ntoskrnl/io/iomgr/file.c b/ntoskrnl/io/iomgr/file.c index c14e61806b..d8bb852606 100644 --- a/ntoskrnl/io/iomgr/file.c +++ b/ntoskrnl/io/iomgr/file.c @@ -673,10 +673,16 @@ IopParseDevice(IN PVOID ParseObject, } } + /* If we have a top level device hint, verify it */ if (OpenPacket->InternalFlags & IOP_USE_TOP_LEVEL_DEVICE_HINT) { - // FIXME: Verify our device object is good to use - ASSERT(DirectOpen == FALSE); + Status = IopCheckTopDeviceHint(&DeviceObject, OpenPacket, DirectOpen); + if (!NT_SUCCESS(Status)) + { + IopDereferenceDeviceObject(OriginalDeviceObject, FALSE); + if (Vpb) IopDereferenceVpbAndFree(Vpb); + return Status; + } } /* If we traversed a mount point, reset the information */

5 years, 11 months

03/04: [NTOSKRNL] Implement IopCheckTopDeviceHint()

by Pierre Schweitzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=779d87b48366a6941c394… commit 779d87b48366a6941c394ef9dc1b19d4e2535d21 Author: Pierre Schweitzer <pierre(a)reactos.org> AuthorDate: Fri Oct 5 10:45:21 2018 +0200 Commit: Pierre Schweitzer <pierre(a)reactos.org> CommitDate: Fri Oct 5 10:49:59 2018 +0200 [NTOSKRNL] Implement IopCheckTopDeviceHint() --- ntoskrnl/include/internal/io.h | 7 ++++++ ntoskrnl/io/iomgr/file.c | 48 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/ntoskrnl/include/internal/io.h b/ntoskrnl/include/internal/io.h index 8f0398b4ad..18a9b2de50 100644 --- a/ntoskrnl/include/internal/io.h +++ b/ntoskrnl/include/internal/io.h @@ -1299,6 +1299,13 @@ IopGetFileInformation( OUT PULONG ReturnedLength ); +BOOLEAN +NTAPI +IopVerifyDeviceObjectOnStack( + IN PDEVICE_OBJECT BaseDeviceObject, + IN PDEVICE_OBJECT TopDeviceObjectHint +); + // // I/O Timer Routines // diff --git a/ntoskrnl/io/iomgr/file.c b/ntoskrnl/io/iomgr/file.c index a8da317317..c14e61806b 100644 --- a/ntoskrnl/io/iomgr/file.c +++ b/ntoskrnl/io/iomgr/file.c @@ -266,6 +266,54 @@ IopDoNameTransmogrify(IN PIRP Irp, ExFreePool(DataBuffer); } +NTSTATUS +IopCheckTopDeviceHint(IN OUT PDEVICE_OBJECT * DeviceObject, + IN POPEN_PACKET OpenPacket, + BOOLEAN DirectOpen) +{ + PDEVICE_OBJECT LocalDevice; + DEVICE_TYPE DeviceType; + + LocalDevice = *DeviceObject; + + /* Direct open is not allowed */ + if (DirectOpen) + { + return STATUS_INVALID_PARAMETER; + } + + /* Validate we have a file system device */ + DeviceType = LocalDevice->DeviceType; + if (DeviceType != FILE_DEVICE_DISK_FILE_SYSTEM && + DeviceType != FILE_DEVICE_CD_ROM_FILE_SYSTEM && + DeviceType != FILE_DEVICE_TAPE_FILE_SYSTEM && + DeviceType != FILE_DEVICE_NETWORK_FILE_SYSTEM && + DeviceType != FILE_DEVICE_DFS_FILE_SYSTEM) + { + return STATUS_INVALID_PARAMETER; + } + + /* Verify the hint and if it's OK, return it */ + if (IopVerifyDeviceObjectOnStack(LocalDevice, OpenPacket->TopDeviceObjectHint)) + { + *DeviceObject = OpenPacket->TopDeviceObjectHint; + return STATUS_SUCCESS; + } + + /* Failure case here */ + /* If we thought was had come through a mount point, + * actually update we didn't and return the error + */ + if (OpenPacket->TraversedMountPoint) + { + OpenPacket->TraversedMountPoint = FALSE; + return STATUS_MOUNT_POINT_NOT_RESOLVED; + } + + /* Otherwise, just return the fact the hint is invalid */ + return STATUS_INVALID_DEVICE_OBJECT_PARAMETER; +} + NTSTATUS NTAPI IopParseDevice(IN PVOID ParseObject,

5 years, 11 months

02/04: [NTOSKRNL] In IoGetRelatedDeviceObject(), validate hint is on the stack before returning it

by Pierre Schweitzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=670a7ac7188f4912a48a5… commit 670a7ac7188f4912a48a50bae4d275f2f21e1810 Author: Pierre Schweitzer <pierre(a)reactos.org> AuthorDate: Fri Oct 5 10:39:50 2018 +0200 Commit: Pierre Schweitzer <pierre(a)reactos.org> CommitDate: Fri Oct 5 10:49:59 2018 +0200 [NTOSKRNL] In IoGetRelatedDeviceObject(), validate hint is on the stack before returning it --- ntoskrnl/io/iomgr/device.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ntoskrnl/io/iomgr/device.c b/ntoskrnl/io/iomgr/device.c index 189e47b71b..416a16e6aa 100644 --- a/ntoskrnl/io/iomgr/device.c +++ b/ntoskrnl/io/iomgr/device.c @@ -6,6 +6,7 @@ * PROGRAMMERS: Alex Ionescu (alex.ionescu(a)reactos.org) * Filip Navara (navaraf(a)reactos.org) * Hervé Poussineau (hpoussin(a)reactos.org) + * Pierre Schweitzer */ /* INCLUDES *******************************************************************/ @@ -1387,8 +1388,10 @@ IoGetRelatedDeviceObject(IN PFILE_OBJECT FileObject) /* Cast the buffer to something we understand */ FileObjectExtension = FileObject->FileObjectExtension; - /* Check if have a replacement top level device */ - if (FileObjectExtension->TopDeviceObjectHint) + /* Check if have a valid replacement top level device */ + if (FileObjectExtension->TopDeviceObjectHint && + IopVerifyDeviceObjectOnStack(DeviceObject, + FileObjectExtension->TopDeviceObjectHint)) { /* Use this instead of returning the top level device */ return FileObjectExtension->TopDeviceObjectHint;

5 years, 11 months

01/04: [NTOSKRNL] Implement IopVerifyDeviceObjectOnStack()

by Pierre Schweitzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=161b6728ef4522a069e2a… commit 161b6728ef4522a069e2a6375fc2d1a39b683aa5 Author: Pierre Schweitzer <pierre(a)reactos.org> AuthorDate: Fri Oct 5 10:36:52 2018 +0200 Commit: Pierre Schweitzer <pierre(a)reactos.org> CommitDate: Fri Oct 5 10:49:59 2018 +0200 [NTOSKRNL] Implement IopVerifyDeviceObjectOnStack() --- ntoskrnl/io/iomgr/device.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/ntoskrnl/io/iomgr/device.c b/ntoskrnl/io/iomgr/device.c index 04f7d5d2db..189e47b71b 100644 --- a/ntoskrnl/io/iomgr/device.c +++ b/ntoskrnl/io/iomgr/device.c @@ -687,6 +687,40 @@ IopGetRelatedTargetDevice(IN PFILE_OBJECT FileObject, return Status; } +BOOLEAN +NTAPI +IopVerifyDeviceObjectOnStack(IN PDEVICE_OBJECT BaseDeviceObject, + IN PDEVICE_OBJECT TopDeviceObjectHint) +{ + KIRQL OldIrql; + BOOLEAN Result; + PDEVICE_OBJECT LoopObject; + + ASSERT(BaseDeviceObject != NULL); + + Result = FALSE; + /* Simply loop on the device stack and try to find our hint */ + OldIrql = KeAcquireQueuedSpinLock(LockQueueIoDatabaseLock); + for (LoopObject = BaseDeviceObject; ; LoopObject = LoopObject->AttachedDevice) + { + /* It was found, it's a success */ + if (LoopObject == TopDeviceObjectHint) + { + Result = TRUE; + break; + } + + /* End of the stack, that's a failure - default */ + if (LoopObject == NULL) + { + break; + } + } + KeReleaseQueuedSpinLock(LockQueueIoDatabaseLock, OldIrql); + + return Result; +} + /* PUBLIC FUNCTIONS ***********************************************************/ /*

5 years, 11 months

01/01: [SERVICES] Revert f6d81f22 because it breaks the advapi32 LockServiceDatabase test.

by Eric Kohl

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1820aa8fe621372258c07… commit 1820aa8fe621372258c07b283f2ff8aee13f3565 Author: Eric Kohl <eric.kohl(a)reactos.org> AuthorDate: Fri Oct 5 09:41:50 2018 +0200 Commit: Eric Kohl <eric.kohl(a)reactos.org> CommitDate: Fri Oct 5 09:41:50 2018 +0200 [SERVICES] Revert f6d81f22 because it breaks the advapi32 LockServiceDatabase test. --- base/system/services/rpcserver.c | 59 ++++++---------------------------------- 1 file changed, 9 insertions(+), 50 deletions(-) diff --git a/base/system/services/rpcserver.c b/base/system/services/rpcserver.c index 8e383a5bdc..7f251c8760 100644 --- a/base/system/services/rpcserver.c +++ b/base/system/services/rpcserver.c @@ -1671,43 +1671,6 @@ ScmIsValidServiceState(DWORD dwCurrentState) } -static -DWORD -WINAPI -ScmStopThread( - _In_ PVOID pParam) -{ - PSERVICE pService; - - DPRINT("ScmStopThread(%p)\n", pParam); - - pService = (PSERVICE)pParam; - - if (pService->lpImage->dwImageRunCount != 0) - return 0; - - Sleep(2000); - - /* Lock the service database exclusively */ - ScmLockDatabaseExclusive(); - - /* Stop the dispatcher thread */ - ScmControlService(pService->lpImage->hControlPipe, - L"", - (SERVICE_STATUS_HANDLE)pService, - SERVICE_CONTROL_STOP); - - /* Remove the service image */ - ScmRemoveServiceImage(pService->lpImage); - - /* Unlock the service database */ - ScmUnlockDatabase(); - - DPRINT("ScmStopThread done!\n"); - return 0; -} - - /* Function 7 */ DWORD WINAPI @@ -1720,8 +1683,6 @@ RSetServiceStatus( DWORD dwPreviousType; LPCWSTR lpLogStrings[2]; WCHAR szLogBuffer[80]; - HANDLE hStopThread = NULL; - DWORD dwStopThreadId; UINT uID; DPRINT("RSetServiceStatus() called\n"); @@ -1801,17 +1762,15 @@ RSetServiceStatus( /* If we just stopped the last running service... */ if (lpService->lpImage->dwImageRunCount == 0) { - /* Run the stop thread to stop the service dispatcher */ - hStopThread = CreateThread(NULL, - 0, - (LPTHREAD_START_ROUTINE)ScmStopThread, - (LPVOID)lpService, - 0, - &dwStopThreadId); - if (hStopThread != NULL) - { - CloseHandle(hStopThread); - } + /* Stop the dispatcher thread */ + ScmControlService(lpService->lpImage->hControlPipe, + L"", + (SERVICE_STATUS_HANDLE)lpService, + SERVICE_CONTROL_STOP); + + /* Remove the service image */ + ScmRemoveServiceImage(lpService->lpImage); + lpService->lpImage = NULL; } }

5 years, 11 months

01/01: [SERVICES] Fix timeout when a service is stopped

by Eric Kohl

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f6d81f225747e4b1b3716… commit f6d81f225747e4b1b3716e9ca8629f77dcc52b1a Author: Eric Kohl <eric.kohl(a)reactos.org> AuthorDate: Fri Oct 5 08:41:23 2018 +0200 Commit: Eric Kohl <eric.kohl(a)reactos.org> CommitDate: Fri Oct 5 08:41:23 2018 +0200 [SERVICES] Fix timeout when a service is stopped RSetServiceStatus: Send the stop command for the dispatcher thread from a separate thread. Fixes CORE-15064 --- base/system/services/rpcserver.c | 59 ++++++++++++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/base/system/services/rpcserver.c b/base/system/services/rpcserver.c index 7f251c8760..8e383a5bdc 100644 --- a/base/system/services/rpcserver.c +++ b/base/system/services/rpcserver.c @@ -1671,6 +1671,43 @@ ScmIsValidServiceState(DWORD dwCurrentState) } +static +DWORD +WINAPI +ScmStopThread( + _In_ PVOID pParam) +{ + PSERVICE pService; + + DPRINT("ScmStopThread(%p)\n", pParam); + + pService = (PSERVICE)pParam; + + if (pService->lpImage->dwImageRunCount != 0) + return 0; + + Sleep(2000); + + /* Lock the service database exclusively */ + ScmLockDatabaseExclusive(); + + /* Stop the dispatcher thread */ + ScmControlService(pService->lpImage->hControlPipe, + L"", + (SERVICE_STATUS_HANDLE)pService, + SERVICE_CONTROL_STOP); + + /* Remove the service image */ + ScmRemoveServiceImage(pService->lpImage); + + /* Unlock the service database */ + ScmUnlockDatabase(); + + DPRINT("ScmStopThread done!\n"); + return 0; +} + + /* Function 7 */ DWORD WINAPI @@ -1683,6 +1720,8 @@ RSetServiceStatus( DWORD dwPreviousType; LPCWSTR lpLogStrings[2]; WCHAR szLogBuffer[80]; + HANDLE hStopThread = NULL; + DWORD dwStopThreadId; UINT uID; DPRINT("RSetServiceStatus() called\n"); @@ -1762,15 +1801,17 @@ RSetServiceStatus( /* If we just stopped the last running service... */ if (lpService->lpImage->dwImageRunCount == 0) { - /* Stop the dispatcher thread */ - ScmControlService(lpService->lpImage->hControlPipe, - L"", - (SERVICE_STATUS_HANDLE)lpService, - SERVICE_CONTROL_STOP); - - /* Remove the service image */ - ScmRemoveServiceImage(lpService->lpImage); - lpService->lpImage = NULL; + /* Run the stop thread to stop the service dispatcher */ + hStopThread = CreateThread(NULL, + 0, + (LPTHREAD_START_ROUTINE)ScmStopThread, + (LPVOID)lpService, + 0, + &dwStopThreadId); + if (hStopThread != NULL) + { + CloseHandle(hStopThread); + } } }

5 years, 11 months

← Newer
1
...
27
28
29
30
31
32
33
...
36
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs October 2018