TokenGroupsAndPrivileges is the younger sister of two TokenGroups and TokenPrivileges classes. In its purpose there's no huge substantial differences apart that this class comes with its own structure, TOKEN_GROUPS_AND_PRIVILEGES, and that this structure comes with extra information.
In NtQueryInformationToken function, remove the useless and redundant NULL check for two primary reasons. First, DefaultQueryInfoBufferCheck already does the necessary probing validation checks and second, ReturnLength must NEVER be NULL!
If the caller does not respect the calling rules of NtQueryInformationToken, the caller is expected to be miserably punished.
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
The current state of Security manager's code is kind of a mess. Mainly, there's code scattered around places where they shouldn't belong and token implementation (token.c) is already of a bloat in itself as it is. The file has over 6k lines and it's subject to grow exponentially with improvements, features, whatever that is.
With that being said, the token implementation code in the kernel will be split accordingly and rest of the code moved to appropriate places. The new layout will look as follows (excluding the already existing files):
- client.c (Client security implementation code)
- objtype.c (Object type list implementation code -- more code related to object types will be put here when I'm going to implement object type access checks in the future)
- subject.c (Subject security context support)
The token implementation in the kernel will be split in 4 distinct files as shown:
- token.c (Base token support routines)
- tokenlif.c (Life management of a token object -- that is Duplication, Creation and Filtering)
- tokencls.c (Token Query/Set Information Classes support)
- tokenadj.c (Token privileges/groups adjusting support)
In addition to that, tidy up the internal header and reorganize it as well.
Fix MiInsertSharedUserPageVad to not charge the system process pool quota.
Even though PsChargeProcessNonPagedPoolQuota itself checks if the process specified is the system process, this doesn't work here as we're too early into boot for the kernel to know what the system process is.
This commit fully implements the inner logic of KeSaveFloatingPointState and KeRestoreFloatingPointState routines. On ReactOS we're currently simply doing a FNSAVE operation whereas on Windows it is a lot more than that.
On Windows Server 2003 the logic more or less goes like this. In order to save the FPU state the NPX state of the current thread has to be checked first, that is, if NPX is loaded and currently charged for the current thread then the system will acquire the NPX registers actively present. From that point it performs either a FNSAVE or FXSAVE
if FX is actually supported. Otherwise the control word and MXCsr registers are obtained.
FXSAVE/FNSAVE operation is done solely if the FX save area is held up in a pool allocation. Pool allocation occurs if it's been found out that the NPX IRQL of the thread is not the same as the current thread which from where it determines if the interrupt level is APC then allocate some pool memory and hold the save area there, otherwise
the save area in question is grabbed from the current processor control region. If NPX is not loaded for the current thread then the FPU state is obtained from the NPX frame.
In our case we'll be doing something way simpler. Only do a FXSAVE/FNSAVE directly of the FPU state registers, in this way we are simplifying the code and the actual logic of Save/Restore mechanism.
This is needed to store FPU state information when saving or restoring the floating point state of a system due to a call to KeSaveFloatingPointState or KeRestoreFloatingPointState.
- Add a check for correct PDO before doing anything
- Process the request only for started devices
- Send the request synchronously during the start sequence
This makes Windows' i8042prt.sys work on ReactOS.
Addendum to cf0bc1c132
The problem is that EndMem is changed to point to the DynamicPart of
the token, but the code after that expects it to still point into the
VariablePart instead.
Problem fixed by moving the insertion of RestrictedSids much sooner
(where the original ones are also being copied).
Shared locking must be used on the source token as it is accessed for
reading only. This fixes in particular the kmtest SeTokenFiltering that
would hang otherwise on a (wrong) exclusive locking.
- SepPerformTokenFiltering(): Always shared-lock the source token.
Its callers (NtFilterToken and SeFilterToken) just need to sanitize and
put the parameters in correct form before calling this helper function.
- Sync comments in NtFilterToken() with SeFilterToken().
This function is either called inter-kernel (in which case, all
parameters must be valid, and if not, we have to bugcheck), or, it
is called with **captured** parameters (from NtFilterToken) and those
latter ones are now expected to be valid and reside in kernel-mode.
Finally, data copied between token structures reside in kernel-mode
only and again are expected to be valid (if not, we bugcheck).
- The ACL is however not validated when the function is called within
kernel mode and no capture is actually being done.
- Simplify aspects of the function (returning early when possible).
If inserting the allocated VAD fails, MiMapViewOfDataSection will make no attempt to free the allocated VAD. Nor will it call MiDereferenceControlArea(ControlArea); like other failure return paths. This commit fixes this behavior.
Co-authored-by: Hermès BÉLUSCA - MAÏTO <hermes.belusca-maito@reactos.org>
We are allocating blocks of pool memory for a security descriptor with its own specific tag, TAG_SEC_QUERY, so just use it when freeing when releasing the descriptor as well (aka freeing the said pool).
There are two fundamental problems when it comes to access checks in ReactOS. First, the internal function SepAccessCheck which is the heart and brain of the whole access checks logic of the kernel warrants access to the calling thread of a process to an object even though access could not be given.
This can potentially leave security issues as we literally leave objects to be touched indiscriminately by anyone regardless of their ACEs in the DACL of a security descriptor. Second, the current access check code doesn't take into account the fact that an access token can have restricted SIDs. In such scenario we must perform additional access checks by iterating over the restricted SIDs of the primary token by comparing the SID equality and see if the group can be granted certain rights based on the ACE policy that represents the same SID.
Part of SepAccessCheck's code logic will be split for a separate private kernel routine, SepAnalyzeAcesFromDacl. The reasons for this are primarily two -- such code is subject to grow eventually as we'll support different type ACEs and handle them accordingly -- and we avoid further code duplicates. On Windows Server 2003 there are 5 different type of ACEs that are supported for access checks:
- ACCESS_DENIED_ACE_TYPE (supported by ReactOS)
- ACCESS_ALLOWED_ACE_TYPE (supported by ReactOS)
- ACCESS_DENIED_OBJECT_ACE_TYPE
- ACCESS_ALLOWED_OBJECT_ACE_TYPE
- ACCESS_ALLOWED_COMPOUND_ACE_TYPE
This gives the opportunity for us to have a semi serious kernel where security of objects are are taken into account, rather than giving access to everyone.
CORE-9174
CORE-9175
CORE-9184
CORE-14520
SepSidInTokenEx function already provides the necessary mechanism to handle scenario where a token has restricted SIDs or a principal SID is given to the call. There's no reason to have these redundant ASSERTs anymore.
In addition to that make sure if the SID is not a restricted and if that SID is the first element on the array and it's enabled, this is the primary user.
This function will be used to retrieve a security identifier from a valid access control entry in the kernel. Mostly and exclusively used within access checks related code and such.
Implement the correct start-stop sequence for resource rebalancing
without the actual rebalancing. Also move IoInvalidateDeviceState
processing into the enumeration thread as it should be.
CORE-17519
Implement initial token debug code. For now debug information that is being tracked are: process image file name, process and thread client IDs and token creation method. More specific debug code can be added later only if needed.
As for the token creation method, this follows the same principle as on Windows where the creation method is defined by a value denoting the first letter of the said method of creation. That is, 0xC is for token creation, 0xD is for token duplication and 0xF is for token filtering. The debug field names are taken from Windows PDB symbols for WinDBG debug extension support purposes. The names must not be changed!
- Add a new cmboot.h header to isolate the boot-support definitions
shared with the NT/ReactOS bootloader.
- Move CmpFreeDriverList() to cmboot.c so that we can use it for
cleanup paths in the NT/ReactOS bootloader.
- CmpFindControlSet(): Directly build the control set name in UNICODE,
instead of doing an ANSI->UNICODE conversion.
- Directly assign the CurrentControlSet\Services constant string,
instead of going the route of init-empty-string + append-string.
This is possible since that string is not modified later.
- Remove ASSERT(FALSE), replacing them with correct failure handling.
- Add cleanup paths in CmpAddDriverToList().
- Simplify and fix CmpFreeDriverList(): it's the full DriverNode
that needs to be freed; not the LIST_ENTRY pointer.
- Add other validity checks:
* Registry value types and data sizes;
* For multi-strings, verify that they are NULL-terminated.
* For (multi-)strings, check whether they are NULL-terminated before
optionally removing their trailing NULL character from the count.
Check also whether they are of zero-length and take appropriate
action where necessary.
- Add CmpIsDriverInList() for future usage in CMBOOT compiled in
bootloader mode.
- Add SAL annotations and Doxygen documentation.
- Add debug traces.
- Formatting / code style fixes.
** TODO: Fix SafeBoot support **
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
- inbv.c now only contains the Inbv-specific API and nothing else.
- It will make easier for people to write their own boot themes & animations,
by just copying/adapting the bootanim.c file (and the resources).
- Add SAL annotations.
- All INBV progress bar functions (except for InbvIndicateProgress())
should not be INIT-only functions, since they can be (not yet in ROS)
used at later times -- namely, for feedback during hibernation.
In particular, the progress percentage specified to InbvUpdateProgressBar(),
or the progress feedback made with InbvIndicateProgress() calls, is
**relative** to the progress sub-range specified with a previous call to
InbvSetProgressBarSubset() (by default, the range is 0...100%).
This functionality is used e.g. when the number of progress steps is
unknown prior, for example when loading drivers: in this case progress
is made within a given percentage range.
This bug has always been with us since 2010.
This reverts 8479509 commit which pretty much does nothing at all (the captured pointer is NULL within the stack of the function has no effect outside of the function). My mistake, sorry.
Whenever a captured security property such as privilege or SID is released, we must not have such captured property point at random address in memory but rather we must assign it as NULL after it's been freed from pool memory. This avoids potential double-after-free situations where we might release a buffer twice.
This is exactly the case with token filtering.
Before purging the data cache of a certain section of a file from system cache, we have to unintialize the private cache maps of that section if a filesystem or any other component prompts the kernel to do so.
Currently, these features are vital for the kernel:
- CPUID instruction
- CMPXCHG8B instruction
- TSC aka Time Stamp Counter
All of that have to be present on i586
As it currently stands the Object Manager doesn't charge any quotas when objects are created, nor it returns quotas when objects are de-allocated and freed from the objects namespace database. This alone can bring inconsistencies in the kernel as we simply don't know what is the amount charged in an object and thus we aren't keeping track of quotas flow.
Now with both PsReturnSharedPoolQuota and PsChargeSharedPoolQuota implemented, the Object Manager can now track the said flow of quotas every time an object is created or de-allocated, thus enforcing consistency with the use of quota resources.
Ensure that when we're cleaning up the EPROCESS object, that we are dereferencing the quota block the process in question was using. The routine will automatically request a quota block cleanup if the process that dereferenced the quota block was the last.
-- Rewrite PspChargeProcessQuotaSpecifiedPool and PspReturnProcessQuotaSpecifiedPool private kernel routines, with the goal to implement the algorithms necessary to manage the fields of quota blocks (Usage, Return, Limit and Peak).
-- Invoke the Mm if quota limit raising or returning has to be done
-- When destroying a quota block, make sure that we're giving back all the rest of non-returned quotas to Memory Mm
-- Crash the system with QUOTA_UNDERFLOW if someone is returning way too much quota than it was previously charged
-- When a process exits, ensure that it doesn't hold up any charged quotas in QuotaUsage field of the process object, that way we're enforcing proper kernel consistency
-- Implement PsChargeSharedPoolQuota and PsChargeProcessPageFileQuota functions, used exclusively by the Object Manager. These routines are used to charge or return amount of quotas of a newly created object.
-- On PspInheritQuota, when assigning to process the default quota block if no parent process is given, we must increment the reference counts as we're using it
-- Handle the ProcessCount reference field, as it wasn't used
-- Annotate the functions with SAL
-- Document the code
=== REMARKS ===
Windows LogOn (Winlogon) is responsible for setting up a different quota block for all the processes within an interactive session, which is what we don't do. What we're currently doing instead is we're using the default block, PspDefaultQuotaBlock, for all the processes
across the system. The default block contains the default limits of -1 (which would imply no limits). By definition, the kernel won't ever return STATUS_QUOTA_EXCEEDED as we literally don't set up a definite limit for regular processes. This situation has to be tackled
in the future.
=== TODO FOR FUTURE ===
Most of the code in PspChargeProcessQuotaSpecifiedPool and PspReturnProcessQuotaSpecifiedPool private routines must be refactored in order to reduce the usage of the quota spin lock, possibly wrapping such code in a loop and whatnot.
CORE-17784
This implements both MmRaisePoolQuota and MmReturnPoolQuota functions, which serve exclusively for quota pool management. The process manager communicates with the memory manager in a call of need to charge or return pool quota limits.
OBP_SYSTEM_PROCESS_QUOTA is a macro that'll be used as a way to assign a dummy quota block to system processes, as we mustn't do anything to those in case the Object Manager is charging or returning pool quotas.
Declare PsReturnSharedPoolQuota and PsChargeSharedPoolQuota prototypes and annotate the functions. Furthermore, add two definitions related to quota pool limits threshold -- PSP_NON_PAGED_POOL_QUOTA_THRESHOLD and PSP_PAGED_POOL_QUOTA_THRESHOLD. For further details, see the commit description of "[NTOS:MM] Add the pool quota prototypes and some definitions".
Declare the MmRaisePoolQuota and MmReturnPoolQuota prototypes in the header and add some definitions related to pool quotas, namely MmTotalNonPagedPoolQuota and MmTotalPagedPoolQuota. These variables are used internally by the kernel as sort of "containers" (for the lack of a better term)
which uphold the amount of quotas that the Process Manager is requesting the Memory Manager to raise or return the pool quota limit. In addition to that, add some definitions needed for both of these functions.
The definitions, MI_CHARGE_PAGED_POOL_QUOTA and MI_CHARGE_NON_PAGED_POOL_QUOTA respectively, bear some interesting aspect. Seemingly the 0x80000 and 0x10000 values (that would denote to 524288 and 65536 specifically) are used as quota "limits" or in other words, thresholds that the kernel
uses. So for example if one would want to raise the quota limit charge, MmRaisePoolQuota will raise it so based on this formula -- NewMaxQuota = CurrentQuota + LIMIT_VALUE. LIMIT_VALUE can be either MI_CHARGE_PAGED_POOL_QUOTA or MI_CHARGE_NON_PAGED_POOL_QUOTA, depending a per quota pool basis.
What's more interesting is that these values are pervasive in Process Manager even. This is when quotas are to be returned back and trim the limit of the quota block if needed, the kernel would either take the amount provided by the caller of quotas to return or the threshold (paged or not paged)
if the amount to return exceeds the said threshold in question.
This fixes an issue where ReactOS would assert on QuotaUsage == 0 as the process was still taking up quotas during a quota block de-reference with root cause of ThisBufferLength member field being 0 which made process quota charging/returning flow unbalanced.
In addition to that, on FsRtlCancelNotify routine API all we must ensure that if PsChargePoolQuota or ExAllocatePoolWithTag function fails we have to handle the raised exceptions accordingly and return the charged quota back (if we actually charged quotas that is). With said, wrap that part of code with SEH.
=== DOCUMENTATION REMARKS ===
The cause of the assert is due to the fact ThisBufferLength was being handled wrongly ever since, until this commit. When FsRtl of the Executive has to filter reported changes (with logic algorithm implemented in FsRtlNotifyFilterReportChange function), the said function will charge the quota of a given process
with an amount that is represented as the buffer length whose size is expressed in bytes. This length buffer is preserved in a local variable called NumberOfBytes, which is initialized from BufferLength member field of notification structure or from the length from stack parameters pointed from an IRP.
As it currently stands, the code is implemented in such a way that FsRtlNotifyFilterReportChange will charge quotas to a process but it doesn't assign the buffer length to ThisBufferLength. On the first glimpse ThisBufferLength and BufferLength are very similar members that serve exact same purpose but in reality there's a subtle distinction between the two.
BufferLength is a member whose length size is given by FSDs (filesystem drivers) during a notification dispatching. Whenever FsRtl receives the notification structure packed with data from the filesystem, the length pointed by BufferLength gets passed to ThisBufferLength and from now on the kernel has to use this member for the whole time of its task to accomplish
whatever request it's been given by the filesystem. In other words, BufferLength is strictly used only to pass length size data to the kernel by initializing ThisBufferLength based on that length and unequivocally the kernel uses this member field. What we're doing is that ThisBufferLength never receives the length from BufferLength therefore whenever FsRtl component
has to return quotas back it'll return an amount of 0 (which means no amount to return) and that's a bug in the kernel.
Also generate processor identifier properly based on this value
on the Configuration Manager machine-dependent initialization.
Update processor driver INF file accordingly.
CORE-17970 CORE-14922
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.
SIDs are variadic by nature which means their lengths can vary in a given amount of time and certain factors that allow for this happen. This also especially can lead to issues when capturing SIDs and attributes because SeCaptureSidAndAttributesArray might end up overwriting the buffer during the time it's been called.
Therefore when we're copying the SIDs, validate their lengths. In addition to that, update the documentation header accordingly and add some debug prints in code.
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.
This implements the support of token filtering within the kernel, where the kernel can create restricted tokens of regular ones on demand by the caller. The implementation can be accessed thorough a NT syscall, NtFilterToken, and a kernel mode routine, SeFilterToken.
The continue statements do not server any useful purpose in these loops so they're basically pointless. These have been introduced by mistake so my bad.
A scenario where it happens that an access token belongs to an administrators group but it's disabled (that is, SeAliasAdminsSid has no attributes or it doesn't have SE_GROUP_ENABLED turn ON), the function removes this group from the token but still has TOKEN_HAS_ADMIN_GROUP flag which can lead to erratic behavior across the kernel and security modules -- implying that the token still belongs to administrators group.
This is an oversight from my part.
Only resources of HAL were checked against conflicts, not those of PnP Manager
Let IoReportResourceForDetection() make a silent conflict check.
Otherwise IopCheckResourceDescriptor() will always return 'no conflict'.
CORE-17789
Previous code did not detect equal resource ranges as conflicting.
Thanks Hervé Poussineau for pointing this out!
Meanwhile, simplify the code to make it more readable.
In SeCaptureLuidAndAttributesArray we must ensure that we don't go onto a potential integer overflow scenario by checking against the maximum limit threshold the kernel states. In addition, write an explicit name macro for the value.
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.)
- NtQuerySymbolicLinkObject(): Use an intermediate variable for the object header.
- Simplify code in ObpLookupEntryDirectory() by calling ObpReleaseLookupContextObject() instead.
- Use TAG_OBJECT_TYPE instead of hardcoded tag values.
- Disentangle the usage of ObpAcquireDirectoryLockExclusive() when it's
used only for accessing a directory structure, or as part of a lookup
operation.
The Obp*DirectoryLock*() -- both shared and exclusive -- functions
are only for locking an OB directory, for reading or writing its
structure members.
When performing lookup operations (insertions/deletions of entries
within a directory), use a ObpAcquireLookupContextLock() function that
exclusively locks the directory and saves extra lock state, that can
be used by ObpReleaseLookupContextObject() for cleanup.
- Add documentation for these functions.
The function might assign the flag yet it could possibly fail on creating a DACL and insert an "access allowed" right to the access entry within the DACL. In this case, make sure we actually succeeded on all the tasks and THEN assign the flag that the DACL is truly present.
Also, make sure that the Current buffer size variable gets its new size so that we avoid overidding the memory of the DACL if the security descriptor wants both a DACL and SACL and so that happens that the DACL memory gets overwritten by the SACL.
Implement the portion chunk of code that is responsible for setting the system access control list (SACL) to the World security descriptor, based from SeWorldSid (World security identifier).
Addendum to 608032bd and 835c3023.
The IRQL is actually raised by KeFreezeExecution() and lowered by
KeThawExecution(), always to HIGH_IRQL on MP systems, or if necessary
on UP. These functions are called respectively by KdEnterDebugger()
and KdExitDebugger().
When performing access security check, use the security descriptor that we've captured it to determine based on that descriptor if the client can be granted access or not.
As we now have the SEF_* flags declared within the SDK we can simply check for such flags directly wihout having to check for the hard-coded flag values.
This implements the EffectiveOnly option of SepDuplicateToken routine (used by NtDuplicateToken syscall and other functions alike) which makes the access token effective by removing the disabled parts like privileges and groups.
Fix a wrong returned datatype of the function, as SepSinglePrivilegeCheck calls the internal private SepPrivilegeCheck function which returns a BOOLEAN value.
When processing:
Make sure that the process is not terminating.
Make sure that the process WorkingSet is still valid
Protect accessing & writing to PTEs by acquiring the working set lock
CORE-17595 CORE-17642
When creating or duplicating an access token object, make sure that the logon session is getting referenced by the token must be inserted onto the logon reference member (a.k.a LogonSession) for proper logon session referencing tracking.
Also when a token object is about to be destroyed or that we are taking away a reference session from it, we must ensure that the referenced logon session data gets removed from the token in question.
CORE-17700
When duplicating an access token, the authentication ID is already copied from the existing token to the new one anyway so there's no point on having the commented call still left in the code.
Note to SELF and EVERYONE: the commit implements the initial logon session termination notification implementation, the SeMarkLogonSessionForTerminationNotification function, but as it currently stands there are several other tasks to be addressed in the future in order for the logon termination notification to be fully completed. The tasks as of which are.
1. Our SepRmDereferenceLogonSession is not fully implemented, as it doesn't inform the LSA and filesystems of logon deletion notification
2. Implement two worker routines that are actually in charge of such tasks of informing LSA and FSDs
3. Perform logon deletion
4. Do further investigations and check whatever that is left to address, if any
* Quality of service kernel stuff bears nothing with security descriptors in anyway, so just have a file specifically for it
* Annotate the function arguments parameters with SAL
* Document the functions
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
We allocate memory pool for a new security descriptor with specific info filled by the caller but we don't set the control flag bits for the newly allocated descriptor, which is wrong. Originally spotted by Vadim Galyant.
CORE-17650
KD64: Raise to HIGH_LEVEL when entering trap
KDBG: lower to DISPATCH_LEVEL when applying IRQL hack & use a worker thread to load symbols
KD&KDBG: Actually unload symbols when required
Raise IRQL before entering debugger, so that KeAcquireSpinLockAtDpcLevel works as expected.
- HIGH_LEVEL since we don't know where we are coming from.
Do not try to read debug symbol from files in KDBG.
- There is no reason that this works if Mm didn't map it in the first place.
GCC has some functions, variables & type attributes which can be used as aliases
for some of the SAL annotations. Although it's not as rich & precise, it's still useful
since we actually enable -Werror on GCC builds whereas we don't use such an option
on MSVC builds.
For now, _Must_inspect_result_ is aliased to warn_result_unused attribute.
ProcessUserModeIOPL, ProcessWow64Information and ThreadZeroTlsCell classes fail on AMD64 build because of wrong IQS values assigned to them. Also explicitly tell the compiler that ProcessUserModeIOPL is strictly for x86 as user mode I/O privilege level is 32-bit stuff.
In addition to that, implement IQS_NO_TYPE_LENGTH macro which it'll be used for classes such as ProcessUserModeIOPL where type length is not required and that it should be 0. With that said, we indirectly fix a size length mismatch issue with ProcessUserModeIOPL on 32-bit of ReactOS as well.
- Remove a redundant call of ObReferenceObjectByHandle. Not only it didn't make much sense (we reference the object from thread handle and the new thread object referencing the same handle!), specifying a request access of THREAD_ALL_ACCESS for the thread object is kind of suspicious and all of these access rights are unwanted.
- Add some failure checks involving the CopyOnOpen code paths
- Add some DPRINT1 debug prints (concerning the CopyOnOpen code paths as usual)
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).
When freeing pages, free page entries with pages num == 3 were
incorrectly treated as entries with pages num >= 4 and thus
their re-insertion was not triggered. That lead to non paged pool
fragmentation (can be triggered by kmtest:ExPools, for example)
Also, altered the index acquisition code for MmNonPagedPoolFreeList
entries so it looks more clear
- 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.
KiSetTrapContext is an asm wrapper around RtlSetUnwindContext, which first stores an exception frame to assure that all non-volatile registers were put on the stack, then calls RtlSetUnwindContext to update their first saving positions on the stack and finally restore the exception frame to potentially load any updated registers, that haven't been saved elsewhere on the stack.
A few of these classes have fixed size lengths, the rest are arbitrary. Also the TokenAuditPolicy class hasn't a size length type specified in the table, which is wrong (and move the corresponding TOKEN_AUDIT_POLICY_INFORMATION structure into the private header).
