The data has to be written into ObjectTypeInfo based on the return length,
not only what is provided by the input buffer length. Fix suggested by
Hermès.
On a x86 system aligning the return length pointer to a 4-byte boundary
works best since pointers in general are 4-byte aligned for x86 systems.
However, what happens on a AMD64 system is that we still align this pointer
to 4-byte, ObjectTypeInfo is a 8-byte pointer and we might write into
the return length past the 4-byte boundary.
If one were to allocate a pool of memory with that length and query all
the object types info and free the said pool of memory thereafter, the
system will crash with BAD_POOL_HEADER because ObQueryTypeInfo overwrote
the return length past the 4-byte boundary length therefore leading up with
corrupted memory blocks in the pool header.
This symptom of BAD_POOL_HEADER happens exactly the same in Windows Server
2003 x64 Edition. Newer versions of Windows like 10 aren't affected.
But, Windows has another bug where they are using MaximumLength for the
calculation of the needed length to be returned to caller. MaximumLength
does not guarantee you that it includes the NULL-terminator in the length
and that potentially leads to a buffer overrun.
Also annotate the ObQueryTypeInfo function with SAL2.
https://processhacker.sourceforge.io/doc/object_8c_source.html (read the
comment in KphObjectTypeInformation).
Second parameter is optional, so mark it as such and check whether it was passed. Fixes a sporadic 0x24 bugcheck caused by access violation when running ReactOS on NTFS volume with WinXP ntfs.sys.
Finally handlers are - unlike except blocks - not part of the function they are in, but separate functions, which are called during unwind. PSEH implements them on GCC using nested functions. While "return" from a finally handler is allowed with native SEH, it's handled by the compiler through an extra unwinding operation using _local_unwind, WHICH IS NOT SUPPORTED BY PSEH! With PSEH, returning from a finally handler does not return from the function, instead it will only return from the finally handler and the function will continue below the finally handler as if there was no return at all. To fix this, the return is removed and an additional success check is added.
Also use _SEH_VOLATILE to make sure the variable assignment is not optimized away by the compiler and add zero out the result parameters on error.
... that would otherwise cause a debugger re-entry.
Also use KdbPuts/Printf instead of KdpDprintf that won't be available
once KDBG is moved out of it.
... since the original ones are internal to the kernel and won't be
available once KDBG is moved out of it.
Use these functions in the pager/prompt support.
The built string can be:
°°Kernel Debugger: Serial port found: COM1 (Port 0x000003F8) BaudRate 115200°°°°
(with ° representing the \r and \n in the message)
and you can verify that this is more than 80 characters in total.
CORE-17627
When closing a file, fastfat zeroes it out from ValidDataLength up to the end of the file.
The ValidDataLength field is updated when the file content is actually written to disk.
There is currently a race between the file-close path and the page out path, leading to potential file corruptions when the zeroing happens after the memory has been flushed to disk.
Fix this by actually flushing the file to disk when unmapping files, with file lock acquired. This way, the FS driver cannot zero out the tail of the file while we're actually flushing it to disk.
This one was more subtle because the prompt (KdIoReadLine) functionality
makes a call-back to KDBG own command history getter function KdbGetHistoryEntry.
It is planned for this to become a registered optional callback pointer.
This is done in preparation for moving all this functionality in a
separate KDTERM "KD Terminal Driver" DLL.
Additionally:
- Flush the terminal input before sending ANSI escape sequences.
- In KDBG pager, always use the correct reading-key function (the
same used also for reading keys for a line of user input), and not
the simplistic two-call KdbpGetCharSerial + KdbpTryGetCharSerial
that would split the \r \n across calls.
- Call KdbpGetCommandLineSettings() in KdbInitialize() at BootPhase 0,
which is indirectly called by KdDebuggerInitialize0(). And fix its
command-line parsing too.
Rename KdbpReadCommand as KdIoReadLine. Extract the last-command
repetition functionality out of KdIoReadLine and put it where it
belongs: only in the KDBG command main loop KdbpCliMainLoop.
Use this function instead of KdpDprintf(), otherwise, we send them to
**ALL** the display providers, including for example dmesg. Replaying
the listing with dmesg would then cause the terminal to misbehave later.
For example, it would send the answer of a "Query Device Attributes"
command, as the response to a query for terminal size...
Addendum to commit 84e32e4e.
Explain more accurately what's going on regarding the returned string
and the inaccurate claims made in the official DbgPrompt documentation
in MSDN. (Has been verified by looking through the traffic in WinDbg
debugging of Windows and ReactOS.)
Of course, now that we **correctly** set the LoadSymbools setting,
we attempt loading symbols at BootPhase 0 and everything goes awry!
So introduce that hack to fallback to our old behaviour.
A proper fix (and removal of the hack) will be done in future commits.
Addendum to commit de892d5b.
The boot options get stripped of their optional command switch '/'
(and replaced by whitspace separation) by the NT loader. Also, forbid
the presence of space between the optional '=' character following
(NO)LOADSYMBOLS.
In addition, fix the default initialization of LoadSymbols in KdbSymInit():
we cannot rely on MmNumberOfPhysicalPages in BootPhase 0 since at this point,
the Memory Manager hasn't been initialized and this variable is not yet set.
(We are called by KdInitSystem(0) -> KdDebuggerInitialize0 at kernel init.)
It gets initialized later on between BootPhase 0 and 1.
Also display a nice KDBG signon showing the status of symbols loading.
LoadSymbols was reset to its default value whenever KdbSymInit() was
called, thus we would e.g. load symbols even if /NOLOADSYMBOLS or
/LOADSYMBOLS=NO were specified at the command line.
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.
CORE-10749
The dmesg command is now available even if screen output is disabled.
Co-authored-by: Hermès Bélusca-Maïto <hermes.belusca-maito@reactos.org>
- KdbSymInit() in kdb_symbols.c only initializes symbols implementation
support.
- The rest of KdbInitialize gets moved into kdb_cli.c and initializes
the KDBG debugger itself.
- Move KdbDebugPrint to kdb_cli.c as well.
Access check is an expensive operation, that is, whenever an access to an
object is performed an access check has to be done to ensure the access
can be allowed to the calling thread who attempts to access such object.
Currently SepAnalyzeAcesFromDacl allocates a block of pool memory for
access check rights, nagging the Memory Manager like a desperate naughty
creep. So instead initialize the access rights as a simple variable in
SepAccessCheck and pass it out as an address to SepAnalyzeAcesFromDacl so
that the function will fill it up with access rights. This helps with
performance, avoiding wasting a few bits of memory just to hold these
access rights.
In addition to that, add a few asserts and fix the copyright header on
both se.h and accesschk.c, to reflect the Coding Style rules.
This mutes a lot of debug spam that fills up the debugger when an access
check fails because a requestor doesn't have enough privileges to access
an object.
MmLoadSystemImage has a PUNICODE_STRING NamePrefix parameter which is
currently unused in ReactOS. When the kernel loads the crash dump
storage stack drivers, the drivers will be loaded with MmLoadSystemImage
with a "dump_" or "hiber_" (for hibernation, which uses crash dump
stack too) prefix. This change adds in the prefix support, and is
supposed to push crash dump support forward.
CORE-376
