Skip to content

Commit 922a021

Browse files
committed
Kernel/Io,Ps: Fix resource leaks, IRQL corruption, and race conditions
IopDeleteDevice: lower IRQL when DeletePending is false, preventing the caller from being stuck at elevated IRQL indefinitely. IopParseDevice: free FileObject and decrement DeviceObject refcount when GetObjectNativeHandle fails, fixing per-failed-open leak. IopQueryDeviceInformation: add missing ObfDereferenceObject call, fixing a FileObject reference leak on every file query operation. IoQueryVolumeInformation: return X_STATUS_NOT_IMPLEMENTED instead of S_OK to prevent callers from using uninitialized output buffers. IoCompletionObjectType: add IopDeleteIoCompletion delete procedure that drains queued CXBX_IO_COMPLETION_PACKET entries on destruction. PsTerminateSystemThread: wrap ThreadListEntry removal, StackCount decrement, and reaper list insertion in KeRaiseIrqlToDpcLevel/ KfLowerIrql to match KeInitializeThread's insertion locking.
1 parent e6701e0 commit 922a021

2 files changed

Lines changed: 35 additions & 8 deletions

File tree

src/core/kernel/exports/EmuKrnlIo.cpp

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ static xbox::void_xt IopDeleteDevice
6868
ObfDereferenceObject(DeviceObject);
6969
}
7070
}
71+
else {
72+
KfLowerIrql(OldIrql);
73+
}
7174
}
7275

7376
// ******************************************************************
@@ -258,6 +261,21 @@ XBSYSAPI EXPORTNUM(63) xbox::ntstatus_xt NTAPI xbox::IoCheckShareAccess
258261
RETURN(X_STATUS_SUCCESS);
259262
}
260263

264+
// Drain any remaining completion packets from the KQUEUE when the IoCompletion object is destroyed
265+
static xbox::void_xt NTAPI IopDeleteIoCompletion(IN xbox::PVOID ObjectBody)
266+
{
267+
xbox::PKQUEUE Queue = reinterpret_cast<xbox::PKQUEUE>(ObjectBody);
268+
269+
// Remove and free all queued completion packets
270+
while (Queue->EntryListHead.Flink != &Queue->EntryListHead) {
271+
xbox::LIST_ENTRY *Entry = Queue->EntryListHead.Flink;
272+
Entry->Blink->Flink = Entry->Flink;
273+
Entry->Flink->Blink = Entry->Blink;
274+
xbox::PCXBX_IO_COMPLETION_PACKET Packet = CONTAINING_RECORD(Entry, xbox::CXBX_IO_COMPLETION_PACKET, ListEntry);
275+
xbox::ExFreePool(Packet);
276+
}
277+
}
278+
261279
// ******************************************************************
262280
// * 0x0040 - IoCompletionObjectType
263281
// ******************************************************************
@@ -266,7 +284,7 @@ XBSYSAPI EXPORTNUM(64) xbox::OBJECT_TYPE xbox::IoCompletionObjectType =
266284
xbox::ExAllocatePoolWithTag,
267285
xbox::ExFreePool,
268286
NULL,
269-
NULL, // TODO : xbox::IopDeleteIoCompletion,
287+
IopDeleteIoCompletion,
270288
NULL,
271289
&xbox::ObpDefaultObject,
272290
'pmoC' // = first four characters of "Completion" in reverse
@@ -973,7 +991,10 @@ xbox::ntstatus_xt NTAPI xbox::IopParseDevice(
973991
}
974992
if (!RootDirectory) {
975993
// Then it must be from xbox's end which we don't have any support.
976-
// TODO: How to free object resource?
994+
if (!UseDummyFile) {
995+
ObfDereferenceObject(FileObject);
996+
}
997+
reinterpret_cast<PDEVICE_OBJECT>(ParseObject)->ReferenceCount--;
977998
EmuLog(LOG_LEVEL::ERROR2, "IopParseDevice attempt call GetObjectNativeHandle could not find any.");
978999
return X_STATUS_OBJECT_NAME_NOT_FOUND;
9791000
}
@@ -1276,6 +1297,8 @@ xbox::ntstatus_xt IopQueryDeviceInformation
12761297
result = X_STATUS_INVALID_PARAMETER;
12771298
}
12781299

1300+
ObfDereferenceObject(FileObject);
1301+
12791302
return result;
12801303

12811304
}
@@ -1328,7 +1351,7 @@ XBSYSAPI EXPORTNUM(76) xbox::ntstatus_xt NTAPI xbox::IoQueryVolumeInformation
13281351
// DxbxPC2XB_FS_INFORMATION
13291352
LOG_UNIMPLEMENTED();
13301353

1331-
RETURN(S_OK);
1354+
RETURN(X_STATUS_NOT_IMPLEMENTED);
13321355
}
13331356

13341357
// ******************************************************************

src/core/kernel/exports/EmuKrnlPs.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -525,14 +525,18 @@ XBSYSAPI EXPORTNUM(258) xbox::void_xt NTAPI xbox::PsTerminateSystemThread
525525
eThread->UniqueThread = xbox::zeroptr;
526526
}
527527

528-
// Remove thread from the process
529-
RemoveEntryList(&eThread->Tcb.ThreadListEntry);
530-
eThread->Tcb.State = Terminated;
531-
KiUniqueProcess.StackCount--;
528+
// Remove thread from the process (synchronized with KeInitializeThread's insertion)
529+
{
530+
KIRQL OldIrql = KeRaiseIrqlToDpcLevel();
531+
RemoveEntryList(&eThread->Tcb.ThreadListEntry);
532+
eThread->Tcb.State = Terminated;
533+
KiUniqueProcess.StackCount--;
534+
InsertTailList(&PspReaperListHead, &((PETHREAD)eThread)->ReaperLink);
535+
KfLowerIrql(OldIrql);
536+
}
532537

533538
// PspReaperRoutine technically free'd the memory allocation from MmCreateKernelStack function.
534539
// Therefore is run from another thread.
535-
InsertTailList(&PspReaperListHead, &((PETHREAD)eThread)->ReaperLink);
536540
KeInsertQueueDpc(&PsReaperDpc, NULL, NULL);
537541

538542
EmuKeFreePcr();

0 commit comments

Comments
 (0)