Problematic behaviour was added in commit a97f262ed (r26067), and
commit c39812d1b (r46193) converted to RtlUpcaseUnicodeString() call.
This was modifying the caller's given string. This is not really
a good practice to do so just to make display fancier.
For example, IopInitializeBuiltinDriver(), that calls the display
function, also uses the passed ServiceName later after.
Because IopDisplayLoadingMessage() executes only in SOS mode,
uppercasing the ServiceName in one case but not the other would
implicitly modify the observable OS behaviour.
IopSuffixUnicodeString() is adapted to be similar to RtlPrefixUnicodeString().
- Make the boolean SosEnabled from ex/init.c visible globally so that
it can be checked against by IopDisplayLoadingMessage().
- Also use RtlString* function to construct the string.
- Doxygen comments;
- SAL annotations;
- These two functions are local to driver.c file only -> static'ify them.
- 2 -> sizeof(WCHAR);
- Rename Length to NumChars;
- static const'ify the L".SYS" string.
Implement IoConnectInterruptEx() for CONNECT_FULLY_SPECIFIED.
This gives ability to load various modern drivers that use IoConnectInterruptEx.
Various drivers work after this change, such as serial.sys MS sample driver when compiled with the reactos tree and many more KMDF drivers from later Windows versions.
Co-authored-by: Victor Perevertkin <victor@perevertkin.ru>
- Add missing ExAllocatePool NULL checks.
- Fix order of KeBugCheckEx parameters for PNP_DETECTED_FATAL_ERROR.
- The Controller and Peripheral numbers are zero-based, so if the caller
wants to inspect controller (or peripheral) zero, let it be so!
The original code was treating controller number zero for enumerating
controllers of a given class within the different buses, which is
wrong. See the diff'ed trace below.
Tested with Windows' videoprt.sys VideoPortGetDeviceData().
```diff
IoQueryDeviceDescription()
BusType: 0xB093C224 (0)
BusNumber: 0xB093C228 (0)
ControllerType: 0xF9D01030 (19)
ControllerNumber: 0xF9D01038 (0)
PeripheralType: 0x00000000 (4294967295)
PeripheralNumber: 0x00000000 (4294967295)
CalloutRoutine: 0xF9CF74E4
Context: 0xF9D5A340
--> Query: 0xF9D5A22C
IopQueryBusDescription(Query: 0xF9D5A22C)
RootKey: '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM'
RootKeyHandle: 0x00000598
KeyIsRoot: TRUE
Bus: 0xF9D5A290 (4294967295)
Seen: 'CentralProcessor'
Seen: 'FloatingPointProcessor'
Seen: 'MultifunctionAdapter'
SubRootRegName: '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter'
IopQueryBusDescription(Query: 0xF9D5A22C)
RootKey: '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter'
RootKeyHandle: 0x00000590
KeyIsRoot: FALSE
Bus: 0xF9D5A290 (4294967295)
Seen: '0'
SubRootRegName: '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter\0'
Getting bus value: 'Identifier'
Getting bus value: 'Configuration Data'
Getting bus value: 'Component Information'
--> Getting device on Bus #0 : '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter\0'
IopQueryDeviceDescription(Query: 0xF9D5A22C)
RootKey: '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter\0'
RootKeyHandle: 0x00000590
Bus: 0
- Enumerating controllers in '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter\0\DisplayController'...
+ Getting controller #0
+ Retrieving controller '\REGISTRY\MACHINE\HARDWARE\DESCRIPTION\SYSTEM\MultifunctionAdapter\0\DisplayController\0'
```
- Fix whitespace; add SAL annotations, doxygen documentation...
- Deduplicate the array of description strings corresponding to
IO_QUERY_DEVICE_DATA_FORMAT.
- Unhardcode the "[3]" into 'IoQueryDeviceMaxData': the maximum number
of device data queried.
CORE-17470
+ KdpDebugLogInit: Add resources cleanup in failure code paths.
Fix, in an NT-compatible manner, how (and when) the KD/KDBG BootPhase >=2
initialization steps are performed.
These are necessary for any functionality KDBG needs, that would depend
on the NT I/O Manager and the storage and filesystem stacks to be running.
This includes, creating the debug log file, and for KDBG, loading its
KDBinit initialization file.
As a result, file debug logging is fixed.
The old ReactOS-specific (NT-incompatible) callback we did in the middle
of IoInitSystem() is removed, in favor of a runtime mechanism that should
work on Windows as well.
The idea for this new mechanism is loosely inspired by the TDL4 rootkit,
see http://blog.w4kfu.com/public/tdl4_article/draft_tdl4article.html
but contrary to it, a specific hook is used instead, as well as the
technique of driver reinitialization:
https://web.archive.org/web/20211021050515/https://driverentry.com.br/en/blog/?p=261
Its rationale is as follows:
We want to be able to perform I/O-related initialization (starting a
logger thread for file log debugging, loading KDBinit file for KDBG,
etc.). A good place for this would be as early as possible, once the
I/O Manager has started the storage and the boot filesystem drivers.
Here is an overview of the initialization steps of the NT Kernel and
Executive:
----
KiSystemStartup(KeLoaderBlock)
if (Cpu == 0) KdInitSystem(0, KeLoaderBlock);
KiSwitchToBootStack() -> KiSystemStartupBootStack()
-> KiInitializeKernel() -> ExpInitializeExecutive(Cpu, KeLoaderBlock)
(NOTE: Any unexpected debugger break will call KdInitSystem(0, NULL); )
KdInitSystem(0, LoaderBlock) -> KdDebuggerInitialize0(LoaderBlock);
ExpInitializeExecutive(Cpu == 0): ExpInitializationPhase = 0;
HalInitSystem(0, KeLoaderBlock); <-- Sets HalInitPnpDriver callback.
...
PsInitSystem(LoaderBlock)
PsCreateSystemThread(Phase1Initialization)
Phase1Initialization(Discard): ExpInitializationPhase = 1;
HalInitSystem(1, KeLoaderBlock);
...
Early initialization of Ob, Ex, Ke.
KdInitSystem(1, KeLoaderBlock);
...
KdDebuggerInitialize1(LoaderBlock);
...
IoInitSystem(LoaderBlock);
...
----
As we can see, KdDebuggerInitialize1() is the last KD initialization
routine the kernel calls, and is called *before* the I/O Manager starts.
Thus, direct Nt/ZwCreateFile ... calls done there would fail. Also,
we want to do the I/O initialization as soon as possible. There does
not seem to be any exported way to be notified about the I/O manager
initialization steps... that is, unless we somehow become a driver and
insert ourselves in the flow!
Since we are not a regular driver, we need to invoke IoCreateDriver()
to create one. However, remember that we are currently running *before*
IoInitSystem(), the I/O subsystem is not initialized yet. Due to this,
calling IoCreateDriver(), much like any other IO functions, would lead
to a crash, because it calls
ObCreateObject(..., IoDriverObjectType, ...), and IoDriverObjectType
is non-initialized yet (it's NULL).
The chosen solution is to hook a "known" exported callback: namely, the
HalInitPnpDriver() callback (it initializes the "HAL Root Bus Driver").
It is set very early on by the HAL via the HalInitSystem(0, ...) call,
and is called early on by IoInitSystem() before any driver is loaded,
but after the I/O Manager has been minimally set up so that new drivers
can be created.
When the hook: KdpInitDriver() is called, we create our driver with
IoCreateDriver(), specifying its entrypoint KdpDriverEntry(), then
restore and call the original HalInitPnpDriver() callback.
Another possible unexplored alternative, could be to insert ourselves
in the KeLoaderBlock->LoadOrderListHead boot modules list, or in the
KeLoaderBlock->BootDriverListHead boot-driver list. (Note that while
we may be able to do this, because boot-drivers are resident in memory,
much like we are, we cannot insert ourselves in the system-driver list
however, since those drivers are expected to come from PE image files.)
Once the KdpDriverEntry() driver entrypoint is called, we register
KdpDriverReinit() for re-initialization with the I/O Manager, in order
to provide more initialization points. KdpDriverReinit() calls the KD
providers at BootPhase >= 2, and schedules further reinitializations
(at most 3 more) if any of the providers request so.
- Remove KdbInit() macro and directly use KdbpCliInit() (since the place
where it was used was already within an #ifdef KDBG block).
- Declare KdpKdbgInit() only when KDBG is defined, move its definition
into kdio.c and remove the legacy wrappers/kdbg.c file.
And in KdbInitialize(), set KdpInitRoutine directly to the former,
instead of using the KdpKdbgInit indirection.
- Don't reset KdComPortInUse in KdpDebugLogInit().
- Minor refactorings: KdpSerialDebugPrint -> KdpSerialPrint and make it
static; argument name "Message" -> "String", "StringLength" -> "Length".
NtQueryInformationToken is by far the only system call in NT where ReturnLength simply cannot be optional. On Windows this parameter is always probed and an argument to NULL directly leads to an access violation exception.
This is due to the fact of how tokens work, as its information contents (token user, owner, primary group, et al) are dynamic and can vary throughout over time in memory.
What happens on current ReactOS master however is that ReturnLength is only probed if the parameter is not NULL. On a NULL case scenario the probing checks succeed and NtQueryInformationToken fails later. For this, just get rid of CompleteProbing
parameter and opt in for a bit mask flag based approach, with ICIF_FORCE_RETURN_LENGTH_PROBE being set on DefaultQueryInfoBufferCheck which NtQueryInformationToken calls it to do sanity checks.
In addition to that...
- Document the ICIF probe helpers
- Annotate the ICIF prope helpers with SAL
- With the riddance of CompleteProbing and adoption of flags based approach, add ICIF_PROBE_READ_WRITE and ICIF_PROBE_READ flags alongside with ICIF_FORCE_RETURN_LENGTH_PROBE
Always create only non-volatile (sub)keys when registering a new device interface, so then they are saved after reboot.
On Windows, nearly all device interface keys are non-volatile, except the "Control" subkey, which is managed by IoSetDeviceInterfaceState instead.
In particular, it fixes MS sysaudio loading failure with MS audio drivers replacement (ks, portcls, swenum, sysaudio, wdmaud). My IoGetDeviceInterfaceAlias implementation is also required to be applied. MS sysaudio implementation(s) except that those keys are non-volatile (but we're creating them volatile instead), and trying to create a subkey(s) there (via other IoDeviceInterface* routines), to read/write some needed data. But then they fail to do that with STATUS_CHILD_MUST_BE_VOLATILE (0xc0000181), obviously because our keys are volatile.
The volatile keys can never have non-volatile subkeys.
CORE-17361
We have a special file, tag.h, which serves as a place to store whatever kernel pool allocation tag yet we still have some tags sparse over the kernel code... So just re-group them in one unique place.
NormalContext and NormalRoutine are just for good measure, but
SystemArgument2 is actually used by the function.
And yes, this appears to be a bug in Win 2003.
Certainly due to copy-pasta error from the original code.
A consequence of this oversight, was that the IoGetDeviceObjectPointer()
calls on these device names, in fltmgr!DriverEntry() couldn't work.
(See drivers/filters/fltmgr/Interface.c, line 1880 and below.)
Use REG_OPTION_NON_VOLATILE instead of REG_OPTION_VOLATILE in all ZwCreateKey calls of OpenRegistryHandlesFromSymbolicLink, since the keys created/opened by this function, should be non-volatile (in other words, be saved after reboot).
Also Device Parameters subkey that is created in IoOpenDeviceInterfaceRegistryKey (which uses that routine as well), is non-volatile too, so the parent keys whose contain it, cannot be volatile.
It will fix an error with status 0xc0000181 (STATUS_CHILD_MUST_BE_VOLATILE) occuring during loading kernel mode audio drivers from Windows XP/2003, especially checked (debug) versions, with my IoGetDeviceInterfaceAlias implementation. Also it may fix other error cases.
CORE-17361
driverName.Buffer leaked when the "(!NT_SUCCESS(status) || ServiceName != NULL)"
case is taken because ServiceName != NULL, and some of the functions fail.
- Fix CID 1477246: Uninitialized pointer read (UNINIT) (happens in
the last ExFreePoolWithTag(basicInfo, TAG_IO) call when the
"(!NT_SUCCESS(status) || ServiceName != NULL)" case is not taken).
- Centralize all the ExFreePoolWithTag(basicInfo, TAG_IO) cleanups
at the end of the function.
- Both cases "(driverName.Buffer == NULL)" and "(ServiceName != NULL)"
can only be taken when basicInfo != NULL, so assert on this fact.
- Fix/add comments;
- Reduce indentation level;
- Direct copy for registry integer values;
- Use for-loops for linked lists;
- Use ULONG when the API uses it (sizes for Ob, or REG_DWORD data in registry).
- Manage the lifetime of the temporary 'PartitionBuffer' buffer where
it is locally used only, and free it as soon as possible, just after
calculating the sector checksum. No need to then free it outside of
the main for-loop.
- When the 'DriveLayout' buffer is freed, ensure the pointer is NULL-ed
(and assert this at the top of the main for-loop), since it can also
be freed at cleanup outside this for-loop, and in this case a NULL
check is performed.
This will avoid the scenario of possibly double-freeing a pointer,
in the case the 'DriveLayout' was previously freed (when e.g. reading
the sector for checksum calculation failed), then the for-loop goes to
the next disk and stops early.
The purpose of 'SingleDisk' is the same as in the IopCreateArcNames()
function. It is an optimization for that when looking up the
firmware-recognized ARC disks list, in order to match one of these with
the current NT disk being analysed (see e.g. also in IopCreateArcNamesDisk()),
we avoid a possible IopVerifyDiskSignature() call and directly build a
corresponding ARC name NT symbolic link for it.
'SingleDisk' will actually be TRUE, whether the DiskSignatureListHead
list is empty or contains only one element: Indeed in only both these
cases, 'DiskSignatureListHead.Flink->Flink' will refer to the list head.
(If the list is empty but 'SingleDisk' is TRUE, this does not matter,
because the DiskSignatureListHead looking-up loop never starts.)
In addition to that, here are some stuff done in this commit whilst testing:
- ICIF_QUERY_SIZE_VARIABLE and friends were badly misused, they should be used only when an information class whose information length size is dyanmic and not fixed. By removing such flags from erroneous classes, this fixes the STATUS_INFO_LENGTH_MISMATCH testcases.
- Use CHAR instead of UCHAR for classes that do not need alignment probing, as every other class in the table do, for the sake of consistency.
- ProcessEnableAlignmentFaultFixup uses BOOLEAN as type size, not CHAR. This fixes a testcase failure on ROS.
- Check for information length size before proceeding further on querying the process' cookie information.
- ProcessHandleTracing wants an alignment of a ULONG, not CHAR.
- Move PROCESS_LDT_INFORMATION and PROCESS_LDT_SIZE outside of NTOS_MODE_USER macro case. This fixes a compilation issue when enabling the alignment probing. My mistake of having them inside NTOS_MODE_USER case, sorry.
- On functions like NtQueryInformationThread and the Process equivalent, complete probing is not done at the beginning of the function, complete probing including if the buffer is writable alongside with datatype misalignment check that is. Instead such check is done on each information class case basis. With that said, we have to explicitly tell DefaultQueryInfoBufferCheck if we want a complete probing or not initially.
