In case of failure, ObInsertObject itself dereferences the object, so calling ObDereferenceObject one more time in the failure handler is redundant.
- Remove ObDereferenceObject call from CmpCreateRegistryRoot in case ObInsertObject fails.
- Add missing RootKey dereference if CmpCreateRegistryRoot failed to get the key cell (KeyCell).
CORE-17904
+ Improve related comments.
Registry hives are opened in shared read access when NT is loaded in PE
mode (MININT) or from network (the hives residing on a network share).
This is true in particular for the main system hives (SYSTEM, SOFTWARE,
DEFAULT, ...).
However, in PE mode, we can allow other hives, e.g. those loaded by the
user (with NtLoadKey) to be loaded with full read/write access, since we
boot from a local computer.
In particular remove some extra-parentheses around single code tokens,
and replace few "DPRINT1 + while (TRUE);" by UNIMPLEMENTED_DBGBREAK.
+ Improve some comments.
Sometimes repairing a broken hive with a hive log does not always guarantee the hive
in question has fully recovered. In worst cases it could happen the LOG itself is even
corrupt too and that would certainly lead to a total unbootable system. This is most likely
if the victim hive is the SYSTEM hive.
This can be anyhow solved by the help of a mirror hive, or also called an "alternate hive".
Alternate hives serve the purpose as backup hives for primary hives of which there is still
a risk that is not worth taking. For now only the SYSTEM hive is granted the right to have
a backup alternate hive.
=== NOTE ===
Currently the SYSTEM hive can only base upon the alternate SYSTEM.ALT hive, which means the
corresponding LOG file never gets updated. When time comes the existing code must be adapted
to allow the possibility to use .ALT and .LOG hives simultaneously.
If FreeLdr performed recovery against the SYSTEM hive with a log, all of its data is only present in volatile memory thus dirty. So the kernel is responsible to flush all the data that's been recovered within the SYSTEM hive into the backing storage.
In addition to that, in some functions like CmFlushKey, CmSaveKey and CmSaveMergedKeys we must validate the underlying hives as a matter of precaution that everything is alright and we don't fuck all the shit up.
When shutting down the registry of the system we don't want that the registry in question gets poked again, such as flushing the hives or syncing the hives and respective logs for example. The reasoning behind this is very simple, during a complete shutdown the system does final check-ups and stuff until the computer
shuts down.
Any writing operations done to the registry can lead to erratic behaviors. CmShutdownSystem call already invokes a final flushing of all the hives on the backing storage which is more than enough to ensure consistency of the last session configuration. So after that final flushing, mark HvShutdownComplete as TRUE indicating
that any eventual flushing or syncying (in the case where HvSyncHive gets called) request is outright ignored.
This prevents a deadlock in DelistKeyBodyFromKCB when we delete a key
object because of an access check failure during a open procedure of a
registry key, as we are already holding a lock against the target KCB of
the key body.
Whenever a security request is invoked into a key object, such as when requesting
information from its security descriptor, the Object Manager will execute
the CmpSecurityMethod method to do the job.
The problem is that CmpSecurityMethod is not aware if the key control block
of the key body already has a lock acquired which means the function will attempt
to acquire a lock again, leading to a deadlock. This happens if the same
calling thread locks the KCB but it also wants to acquire security information
with ObCheckObjectAccess in CmpDoOpen.
Windows has a hack in CmpSecurityMethod where the passed KCB pointer is ORed
with a bitfield mask to avoid locking in all cases. This is ugly because it negates
every thread to acquire a lock if at least one has it.
I intend to port back the combined work of Thomas Faber and Serge Gautherie in context of CORE 14271.
Both developers fixed wrong retval evaluations for SeSinglePrivilegeCheck() and RtlCreateUnicodeString().
Both functions do return a BOOLEAN, and therefore using NTSTATUS() on them is wrong.
Those bugs have been fixed at multiple places. That is long gone.
But Serge fixed his locations a bit more elegantly, without the need for additional variables.
Therefore this addendum adapts a few of Thomas locations to the improved Serge-ified style.
Yes: I intentionally used a space instead of a minus after the mentioned CORE 14271,
as I don't want that pure stylistic addendum to be linked with the initial ticket anymore.
That would be overkill.
Whenever ReactOS finishes its operations onto the registry and unlocks it, a lazy flush is invoked to do an eventual flushing of the registry to the backing storage of the system. Except that... lazy flushing never comes into place.
This is because whenever CmpLazyFlush is called that sets up a timer which needs to expire in order to trigger the lazy flusher engine worker. However, registry locking/unlocking is a frequent occurrence, mainly when on desktop. Therefore as a matter of fact, CmpLazyFlush keeps removing and inserting the timer and the lazy flusher will never kick in that way.
Ironically the lazy flusher actually does the flushing when on USETUP installation phase because during text-mode setup installation in ReactOS the frequency of registry operations is actually less so the timer has the opportunity to expire and fire up the flusher.
In addition to that, we must queue a lazy flush when marking cells as dirty because such dirty data has to be flushed down to the media storage of the system. Of course, the real place where lazy flushing operation is done should be in a subset helper like HvMarkDirty that marks parts of a hive as dirty but since we do not have that, we'll be lazy flushing the registry during cells dirty marking instead for now.
CORE-18303
- 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 **
- Change INIT_FUNCTION and INIT_SECTION to CODE_SEG("INIT") and DATA_SEG("INIT") respectively
- Remove INIT_FUNCTION from function prototypes
- Remove alloc_text pragma calls as they are not needed anymore
* Add an NDK header to define INIT_FUNCTION/INIT_SECTION globally
* Use _declspec(allocate(x)) and _declspec(code_seg(x)) on MSVC versions that support it
* Use INIT_FUNCTION on functions only and INIT_SECTION on data only (required by MSVC)
* Place INIT_FUNCTION before the return type (required by MSVC)
* Make sure declarations and implementations share the same modifiers (required by MSVC)
* Add a global linker option to suppress warnings about defined but unused INIT section
* Merge INIT section into .text in freeldr
- 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
Based on an original patch by Timo Kreuzer, with modifications by me to adapt it to latest HEAD and use a single exit path through the Cleanup label. This reliably frees all allocated handles.
The original code returns STATUS_SUCCESS for many cases. This has been preserved.
In the future, it should be checked though whether returning success is appropriate for all these cases.
CORE-6844
- When we are in LiveCD mode (more generally, when hives are shared), load
the system hives as volatile.
- Ignore hive write operations when everything operates in read-only mode
and just return success instead.
- Just return success on hive file I/O if no file is associated with a
given hive. This happens when e.g. a CM hive has a primary but no log.