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.
