This incorrect behavior was leading to a call at too high IRQL for paged code.
This was triggered by MS FastFAT.
ReleaseFromLazyWrite call was already correctly called to that regard.
CORE-11819
Also, probe the service name when unloading a driver if called from
user-mode. This will avoid that userland applications can trigger an
invalid read in the kernel (and thus, a BSOD).
CORE-15468
- Require the user to have TCB privilege for using this function.
- Probe and capture the user-provided string (and avoid usermode-triggered BSODS ;-)
- Allocate the OEM-converted string in *NonPagedPool* because we are
going to transmit the buffer to BOOTVID.
This avoids performing a double-free (even though that's hidden by the
fact we use lookaside allocations for VACB), and it avoids freeing
a memory address at an uninitialized address.
We don't care about references here, the VACB was just allocated, never
linked and we're its only user.
CORE-15413
- Simplify the volume-deletion code in RawCheckForDismount().
- Fixes the OpenCount check in RawClose(): the VCB mutex must be
released when the volume has not been dismounted, either because
OpenCount != 0 or because RawCheckForDismount() returned FALSE.
- Explicitly use VCB_STATE_LOCKED instead of hardcoding its value.
- In IRP_MN_VERIFY_VOLUME handling, lock the volume before playing
with it, and again let the volume be dismounted only if OpenCount == 0
(and the IoDeleteDevice() call is done by RawCheckForDismount()).
Now, we make sure that we update ref count and BCB list membership
with the BCB lock held, in a row.
This will avoid race conditions where the BCB was removed from the
list, then referenced again, leading to inconsistencies in memory
and crashes later on.
This could notably be triggered while building ReactOS on ReactOS
(one would call this a regression).
CORE-15235
- Rework CmpSetSystemValues() and remove its 1st-stage text-mode setup hack, since a real registry hive will be used for 1st-stage either.
- Lock, then unlock the registry in NtInitializeRegistry when initializing the hives & flusher.
- Call CmpInitializeHiveList() (i.e., initialize the other hives like \Software, \User, \.Default) only when we are not in setup-mode.
svn path=/branches/setup_improvements/; revision=74747
This reverts a7c26408 (r53255) and ff75ae1b (r53694), and a hack from 6075ae9a (r46690).
svn path=/branches/setup_improvements/; revision=74745
svn path=/branches/setup_improvements/; revision=74746
- Improve the capture of OBJECT_ATTRIBUTES parameters that are passed
(by pointer) to the Cm* helper functions, and the capture of
UNICODE_STRINGs.
- Correctly differentiate user-mode vs. kernel-mode root directory handles
(in OBJECT_ATTRIBUTES): note that most of the Cm* APIs expect their
parameters to be kernel-mode (pointers, handles...).
CORE-13448
- Validate the information class parameter in NtQueryValueKey().
- Call the post-callback in NtSetValueKey() only if the callback
has been registered and the CmSetValueKey() call is executed.
We won't reuse a BCB created for mapping, we will now have
our own dedicated BCB.
This allows having a bit more cleaner implementation of CcPinMappedData()
We now handle race conditions when creating BCB to avoid
having duplicated BCB per shared maps.
Also, we already specify whether the memory will be pinned
when creating the BCB, to avoid potential duplications or
BCB misuse.
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.
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.
This quick check based on bits and operation is for 2^ based
sector sizes (most of the cases) and will perform faster than
the modulo operation which is still used in fallback in case
the sector size wouldn't be a power of 2.
IopCloseFile can be called by IopDeleteFile. In that situation, it
doesn't set any process as first parameter. Furthermore, we are in a
situation where it's not required to lock the file object (see the
assert before the call).
- SeIsTokenChild(): Correctly check whether a caller-provided token
is a child from the current process' primary token by looking at
its ParentTokenId member.
- Add a SeIsTokenSibling() helper to determine whether a caller-provided
token and the current process' primary token are siblings, by comparing
their ParentTokenId's and AuthenticationId's.
NOTE: Children tokens are created through CreateRestrictedToken();
sibling tokens are created through DuplicateToken() (amongst others).
See slide 49 of https://www.slideshare.net/Shakacon/social-engineering-the-windows-kernel-by-james-forshaw
or https://googleprojectzero.blogspot.com/2016/01/raising-dead.html
for some details.
This could be triggered when attempting to read/write to really big
files. It was causing an attempt to read 0 bytes in Cc, leading to
asserts failure in the kernel (and corrupted file).
CORE-15067
Currently, our CcMapData() behavior (same goes for CcPinRead()) is broken
and is the total opposite of what Windows kernel does. By default, the later
will let you map a view in memory without even attempting to bring its
data in memory. On first access, there will be a fault and memory will
be read from the hardware and brought to memory. If you want to force read
on mapping/pinning, you have to set the MAP_NO_READ (or PIN_NO_READ) flag
where kernel will fault on your behalf (hence the need for MAP_WAIT/PIN_WAIT).
On ReactOS, by default, on mapping (and thus pinning), we will force a view
read so that data is in memory. The way our cache memory is managed at the
moment seems not to allow to fault on invalid access and if we don't force
read, the memory content will just be zeroed.
So trying to match Windows behavior, by default, now CcMapData() will enforce
the MAP_NO_READ flag and warn once about this behavior change.
It's based on the code that was in CcPinRead() implementation. This
made no sense to have CcPinMappedData() doing nothing while implementing
everything in CcPinRead(). Indeed, drivers (starting with MS drivers)
can map data first and pin it afterwards with CcPinMappedData(). It was
leading to incorrect behavior with our previous noop implementation.
Short: The code was suffering from an off-by-one bug (inconsistency between inclusive end exclusive end address), which could lead to freeing one page above the initialization code. This led to freeing part of the kernel import section on x64. Now it is consistently using the aligned/exclusive end address.
Long:
* Initialization sections are freed both for the boot loaded images as well as for drivers that are loaded later. Obviously the second mechanism needs to be able to run at any time, so it is not initialization code itself. For some reason someone decided though, it would be a smart idea to implement the code twice, once for the boot loaded images, once for drivers and concluding that the former was initialization code itself and had to be freed.
* Since freeing the code that frees the initialization sections, while it is doing that is not possible, it uses a "smart trick", initially skipping that range, returning its start and end to the caller and have the caller free it afterwards.
* The code was using the end address in an inconsistent way, partly aligning it to the start of the following section, sometimes pointing to the last byte that should be freed. The function that freed each chunk was assuming the latter (i.e. that the end was included in the range) and thus freed the page that contained the end address. The end address for the range that was returned to the caller was aligned to the start of the next section, and the caller used it to free the range including the following page. On x64 this was the start of the import section of ntoskrnl. How that worked on x86 I don't even want to know.
The PROCESS_DEVICEMAP_INFORMATION union has 2 fields, one is a handle, the other one is a structure of 36 bytes (independent of architecture). The handle forces 64 bit alignment on 64 bit builds, making the structure 4 bytes bigger than on 32 bit builds. The site is checked in NtQueryInformationProcess (case ProcessDeviceMap). The expected size on x64 is the size of the Query structure without alignment. autocheck correctly passes the site of the Query union member, while smss passes the full size of PROCESS_DEVICEMAP_INFORMATION. Packing the structure is not an option, since it is defined in public headers without packing. Using the original headers sizeof(PROCESS_DEVICEMAP_INFORMATION) is 0x28, sizeof(PROCESS_DEVICEMAP_INFORMATION::Query) is 0x24.
- Rename ObDirectoryType to ObpDirectoryObjectType and remove it from NDK (this is not exported!)
- Rename ObSymbolicLinkType to ObpSymbolicLinkObjectType
- Remove duplicated ObpTypeObjectType from ob.h
Kernel stacks that re freed, can be placed on an SLIST for quick reuse. The old code was using a member of the PFN of the last stack page as the SLIST_ENTRY. This relies on the following (non-portable) assumptions:
- A stack always has a PTE associated with it.
- This PTE has a PFN associated with it.
- The PFN has an empty field that can be re-used as an SLIST_ENTRY.
- The PFN has another field that points back to the PTE, which then can be used to get the stack base.
Specifically: On x64 the PFN field is not 16 bytes aligned, so it cannot be used as an SLIST_ENTRY. (In a "usermode kernel" the other assumptions are also invalid).
The new code does what Windows does (and which seems absolutely obvious to do): Place the SLIST_ENTRY directly on the stack.
It's hardly understandable and doesn't really makes sense.
Furthermore, it breaks compatibility with 3rd party FSD that
don't implement such FSCTL.
Obviously, Windows doesn't do this.
For user mode, when probing output buffer, if it's null, length
will also be set to 0.
This avoids user mode applications being able to trigger various
asserts in ReactOS (and thus BSOD when no debugger is plugged ;-)).
NDK: Define PLUGPLAY_CONTROL_PROPERTY_DATA.Properties and PLUGPLAY_CONTROL_DEVICE_RELATIONS_DATA.Relations values.
NTOSKRNL: Map PLUGPLAY_CONTROL_PROPERTY_DATA.Properties values to IoGetDeviceProperty properties and add (dummy) code for unsupported cases.
UMPNPMGR: Use PLUGPLAY_CONTROL_PROPERTY_DATA.Properties values in PNP_GetDeviceRegProp.
- Overhaul SepCreateToken() and SepDuplicateToken() so that they
implement the "variable information area" of the token, where
immutable lists of user & groups and privileges reside, and the
"dynamic information area" (allocated separately in paged pool),
where mutable data such as the token's default DACL is stored.
Perform the necessary adaptations in SepDeleteToken() and in
NtSetInformationToken().
- Actually dereference the token's logon session, when needed, in the
'TokenSessionReference' case in NtSetInformationToken().
- Overhaul SepFindPrimaryGroupAndDefaultOwner() so that it returns
the indices of candidate primary group and default owner within the
token's user & groups array. This allows for fixing the 'TokenOwner'
and 'TokenPrimaryGroup' cases of NtSetInformationToken(), since the
owner or primary group being set *MUST* already exist in the token's
user & groups array (as a by-product, memory corruptions that existed
before due to the broken way of setting these properties disappear too).
- Lock tokens every time operations are performed on them (NOTE: we
still use a global token lock!).
- Touch the ModifiedId LUID member of tokens everytime a write operation
(property change, etc...) is made on them.
- Fix some group attributes in the SYSTEM process token, SepCreateSystemProcessToken().
- Make the SeCreateTokenPrivilege mandatory when calling NtCreateToken().
- Update the token pool tags.
- Explicitly use the Ex*ResourceLite() versions of the locking functions
in the token locking macros.
- Use TRUE/FALSE instead of 1/0 for booleans.
- Use NULL instead of 0 for null pointers.
- Print 0x prefix for hex values in DPRINTs.
- Use new annotations for SepCreateToken() and SepDuplicateToken().
Caught while debugging, in the case the ImpersonationLevel value was
uninitialized, due to the fact it was left untouched on purpose by
PsReferenceEffectiveToken().
kmtest:NtCreateSection calls CcInitializeCacheMap with a
NULL value for SectionObjectPointers. This will cause an exception when
trying to access it, which in Windows can be handled gracefully.
However accessing it while holding ViewLock means the lock will not be
released, leading to an APC_INDEX_MISMATCH bugcheck.
This solves the problem by allocating SharedCacheMap outside the lock,
then freeing it again under lock if another thread has updated SharedCacheMap
in the mean time. This is also What Windows Does(TM).
This avoids a really nasty race condition in our cache controler where
two concurrents could try to initialize cache on the same file.
This had two nasty effects: first shared map was purely leaked and erased
by the second one. And the private cache map, allocated on the first shared
cache map couldn't be freed and was leading to Mm BSOD (free in a middle of
a block).
This was often triggered while building ReactOS on ReactOS (with multi threads).
With that patch, I cannot crash anylonger while building ReactOS.
CORE-14634
Oneliner of the day... This typo just prevented the
whole feature to work properly. Because any allocated
work item would miserably fail to be freed.
This will obviously help real world FSD relying on
StackOverflow worker from FsRtl to work better!
CORE-14611
In the lazy writer run, first post items that are queued for this.
Only then, start executing deferred writes if any.
If there were any, reschedule immediately a lazy writer run, to keep
Cc warm and to make it unqueue write faster in case of high IOs situation.
To make second lazy writer run happen faster, we keep our state active to
use short delay (1s) instead of standard idle (3s).
Recent changes seem to show that it's not
required to be exclusive on VACB to be able
to flush it.
This commit goes with f2c44aa and fixes the
last issues going with copying huge files.
There are no longer BSODs (be it in Mm or Cc).
I could, with 750MB RAM extract a 2GB file from
a 53MB archive and copy a 2,5GB file from a VBox
share to the disk. Note that writes are often
deferred, so if copy works, it's not that fast for now.
Note that it also brings some beloved behavior from
Windows: copy times are totally unreliable now when
writes are deferred. Little remaining times when
actively copying, high remaining times when deferred
writes in action. And goes between both... Sorry! ;-)
https://xkcd.com/612/
CORE-9696
CORE-11175
