Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CMLIB] Registry transactional writes. #3932

Closed
wants to merge 5 commits into from
Closed

[CMLIB] Registry transactional writes. #3932

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e0e7f2
[CMLIB] Registry transactional writes.
mrmks04 Jun 15, 2021
2d3b888
Recover function splitted to recover header and recover data
mrmks04 Sep 6, 2021
8ac4a9d
Update drivers/filesystems/fastfat_new/create.c
mrmks04 Feb 9, 2022
1fad5d5
Update sdk/lib/cmlib/hivewrt.c
mrmks04 Feb 9, 2022
b956b00
Review fixes
mrmks04 Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion drivers/filesystems/fastfat_new/create.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ Return Value:

ULONG MatchFlags = 0;

CCB LocalCcb;
CCB LocalCcb = {0};
HBelusca marked this conversation as resolved.
Show resolved Hide resolved
mrmks04 marked this conversation as resolved.
Show resolved Hide resolved
UNICODE_STRING Lfn;
UNICODE_STRING OrigLfn = {0};

Expand Down
4 changes: 2 additions & 2 deletions ntoskrnl/config/cminit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,14 +545,14 @@ CmpOpenHiveFiles(IN PCUNICODE_STRING BaseName,

/* Now create the file */
Status = ZwCreateFile(Log,
DesiredAccess,
DesiredAccess | SYNCHRONIZE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? (same question for the IoFlags too)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this flag, it have stack corrupt. It happen if inside RegistryHive->FileWrite function ZwWriteFile return STATUS_PENDING. Or in RegistryHive->FileFlush function ZwFlushBuffersFile return STATUS_PENDING.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that claim still stand @mrmks04 with todays master head?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With latest code from master problem gone.

&ObjectAttributes,
&IoStatusBlock,
NULL,
AttributeFlags,
ShareMode,
CreateDisposition,
IoFlags,
IoFlags | FILE_SYNCHRONOUS_IO_NONALERT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Primary is also opened with this flag + the sync attributes set too. Do we want this? What does windows do?

Copy link
Contributor Author

@mrmks04 mrmks04 Feb 9, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows server 2003 open file with flags 0x40008, for "ntuser.dat.log".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this extra flag is not set by Win2k3.

NULL,
0);
if ((NT_SUCCESS(Status)) && (MarkAsSystemHive))
Expand Down
7 changes: 7 additions & 0 deletions sdk/lib/cmlib/hivedata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@
#define HV_HBLOCK_SIGNATURE 0x66676572 // "regf"
#define HV_HBIN_SIGNATURE 0x6e696268 // "hbin"

//
// Log identifiers
//
#define DIRTY_BLOCK 0xFF
#define DIRTY_ID 0x54524944 // DIRT
mrmks04 marked this conversation as resolved.
Show resolved Hide resolved
#define DIRTY_ID_SIZE 4
mrmks04 marked this conversation as resolved.
Show resolved Hide resolved

//
// Hive versions
//
Expand Down
191 changes: 183 additions & 8 deletions sdk/lib/cmlib/hiveinit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
*/
BOOLEAN CMAPI
HvpVerifyHiveHeader(
IN PHBASE_BLOCK BaseBlock)
IN PHBASE_BLOCK BaseBlock,
IN ULONG HfileType)
{
if (BaseBlock->Signature != HV_HBLOCK_SIGNATURE ||
BaseBlock->Major != HSYS_MAJOR ||
BaseBlock->Minor < HSYS_MINOR ||
BaseBlock->Type != HFILE_TYPE_PRIMARY ||
BaseBlock->Type != HfileType ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you generalized this function to be also usable for HFILE_TYPE_LOG types 👍 :
have you checked whether Windows registry LOG files' base block have all the Major and Minor fields set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base block in log file is copy of original registry base block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK so it's better to check for this.

BaseBlock->Format != HBASE_FORMAT_MEMORY ||
BaseBlock->Cluster != 1 ||
BaseBlock->Sequence1 != BaseBlock->Sequence2 ||
Expand All @@ -31,7 +32,7 @@ HvpVerifyHiveHeader(
DPRINT1(" Signature: 0x%x, expected 0x%x; Major: 0x%x, expected 0x%x\n",
BaseBlock->Signature, HV_HBLOCK_SIGNATURE, BaseBlock->Major, HSYS_MAJOR);
DPRINT1(" Minor: 0x%x expected to be >= 0x%x; Type: 0x%x, expected 0x%x\n",
BaseBlock->Minor, HSYS_MINOR, BaseBlock->Type, HFILE_TYPE_PRIMARY);
BaseBlock->Minor, HSYS_MINOR, BaseBlock->Type, HfileType);
DPRINT1(" Format: 0x%x, expected 0x%x; Cluster: 0x%x, expected 1\n",
BaseBlock->Format, HBASE_FORMAT_MEMORY, BaseBlock->Cluster);
DPRINT1(" Sequence: 0x%x, expected 0x%x; Checksum: 0x%x, expected 0x%x\n",
Expand Down Expand Up @@ -234,7 +235,7 @@ HvpInitializeMemoryHive(
DPRINT("ChunkSize: %zx\n", ChunkSize);

if (ChunkSize < sizeof(HBASE_BLOCK) ||
!HvpVerifyHiveHeader(ChunkBase))
!HvpVerifyHiveHeader(ChunkBase, HFILE_TYPE_PRIMARY))
{
DPRINT1("Registry is corrupt: ChunkSize 0x%zx < sizeof(HBASE_BLOCK) 0x%zx, "
"or HvpVerifyHiveHeader() failed\n", ChunkSize, sizeof(HBASE_BLOCK));
Expand Down Expand Up @@ -345,7 +346,7 @@ HvpInitializeFlatHive(
PHHIVE Hive,
PHBASE_BLOCK ChunkBase)
{
if (!HvpVerifyHiveHeader(ChunkBase))
if (!HvpVerifyHiveHeader(ChunkBase, HFILE_TYPE_PRIMARY))
mrmks04 marked this conversation as resolved.
Show resolved Hide resolved
return STATUS_REGISTRY_CORRUPT;

/* Setup hive data */
Expand All @@ -362,6 +363,150 @@ HvpInitializeFlatHive(
return STATUS_SUCCESS;
}

/**
* @name HvpRecoverHeaderFromLog
*
* Function to recover hive header from log.
*/
BOOLEAN CMAPI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: use instead a ULONG return value; this will allow you to specify more accurate error codes than TRUE/FALSE (see after). This would allow the caller (see HvLoadHive) to translate this hive-lib-specific error codes into their corresponding STATUS values.

HvpRecoverHeaderFromLog(
IN PHHIVE Hive,
IN OUT PHBASE_BLOCK *BaseBlock)
{
PHBASE_BLOCK LogBaseBlock;
ULONG Offset = 0;
BOOLEAN IsSuccess;

ASSERT(sizeof(HBASE_BLOCK) >= (HSECTOR_SIZE * Hive->Cluster));

LogBaseBlock = HvpAllocBaseBlockAligned(Hive, TRUE, TAG_CM);
if (!LogBaseBlock)
{
DPRINT1("Allocate base block failed\n");
return FALSE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here for example it would be NoMemory.

}

/* Read log file header */
IsSuccess = Hive->FileRead(Hive,
HFILE_TYPE_LOG,
&Offset,
LogBaseBlock,
Hive->Cluster * HSECTOR_SIZE);

if (!IsSuccess)
{
DPRINT1("Read LOG file failed\n");
return FALSE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below, all the failures would be Fail.

}

/* Validate log header */
if (!HvpVerifyHiveHeader(LogBaseBlock, HFILE_TYPE_LOG))
{
DPRINT1("LOG header corrupted\n");
// TODO: Self heal

return FALSE;
}

LogBaseBlock->Type = HFILE_TYPE_PRIMARY;
LogBaseBlock->CheckSum = HvpHiveHeaderChecksum(LogBaseBlock);

/* Write header to hive */
Offset = 0;
IsSuccess = Hive->FileWrite(Hive,
HFILE_TYPE_PRIMARY,
&Offset,
LogBaseBlock,
Hive->Cluster * HSECTOR_SIZE);

if (!IsSuccess)
{
DPRINT1("Write hive header failed\n");
return FALSE;
}

*BaseBlock = LogBaseBlock;

return TRUE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here HiveSuccess.

}

/**
* @name HvpRecoverDataLog
*
* Function to recover hive data from log.
*/
BOOLEAN CMAPI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same suggestion here as for the previous Recover function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not realy. "recover hive data" and "recover hive header".:smiley:

HvpRecoverDataFromLog(
IN PHHIVE Hive,
IN OUT PHBASE_BLOCK BaseBlock)
{
ULONG Offset = 0;
ULONG IndexInLog;
BOOLEAN IsSuccess;
UCHAR DirtyVectBuffer[HSECTOR_SIZE];
UCHAR Buffer[HBLOCK_SIZE];

/* Read dirty blocks from log file */
Offset = HV_LOG_HEADER_SIZE;
IsSuccess = Hive->FileRead(Hive,
HFILE_TYPE_LOG,
&Offset,
DirtyVectBuffer,
HSECTOR_SIZE);

if (!IsSuccess)
{
DPRINT1("Read dirty vector from LOG file failed\n");
return FALSE;
}

if (*((PULONG)DirtyVectBuffer) != DIRTY_ID)
{
DPRINT1("Wrong header in dirty block\n");
return FALSE;
}

IndexInLog = 0;
/* Write birty blocks */
for (ULONG blockIndex = 0; blockIndex < BaseBlock->Length / HBLOCK_SIZE; ++blockIndex)
mrmks04 marked this conversation as resolved.
Show resolved Hide resolved
{
if (DirtyVectBuffer[blockIndex + DIRTY_ID_SIZE] != DIRTY_BLOCK)
{
continue;
}

Offset = HSECTOR_SIZE + HSECTOR_SIZE + IndexInLog * HBLOCK_SIZE;
IsSuccess = Hive->FileRead(Hive,
HFILE_TYPE_LOG,
&Offset,
Buffer,
HBLOCK_SIZE);

if (!IsSuccess)
{
DPRINT1("Read hive data failed\n");
return FALSE;
}

Offset = HBLOCK_SIZE + blockIndex * HBLOCK_SIZE;
IsSuccess = Hive->FileWrite(Hive,
HFILE_TYPE_PRIMARY,
&Offset,
Buffer,
HBLOCK_SIZE);

if (!IsSuccess)
{
DPRINT1("Write hive data failed\n");
return FALSE;
}

IndexInLog++;
}

return TRUE;
}

typedef enum _RESULT
{
NotHive,
Expand Down Expand Up @@ -403,7 +548,11 @@ HvpGetHiveHeader(IN PHHIVE Hive,
if (!Result) return NotHive;

/* Do validation */
if (!HvpVerifyHiveHeader(BaseBlock)) return NotHive;
if (!HvpVerifyHiveHeader(BaseBlock, HFILE_TYPE_PRIMARY))
{
Hive->Free(BaseBlock, Hive->BaseBlockAlloc);
return RecoverHeader;
}

/* Return information */
*HiveBaseBlock = BaseBlock;
Expand Down Expand Up @@ -441,10 +590,36 @@ HvLoadHive(IN PHHIVE Hive,

/* Has recovery data */
case RecoverData:
case RecoverHeader:


/* Fail */
return STATUS_REGISTRY_CORRUPT;

case RecoverHeader:
{
DPRINT1("Header corrupted. Try recover.\n");

#if (NTDDI_VERSION < NTDDI_VISTA)
if (!Hive->Log)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log absence failure should be handed in both RecoverHeader and RecoverData cases.

{
DPRINT1("Haven't LOG\n");
return STATUS_REGISTRY_CORRUPT;
}
#endif

if (!HvpRecoverHeaderFromLog(Hive, &BaseBlock))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done only in RecoverHeader case.

{
DPRINT1("Recover header from LOG failed\n");
return STATUS_REGISTRY_CORRUPT;
}

if (!HvpRecoverDataFromLog(Hive, BaseBlock))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done in both RecoverHeader and RecoverData cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the data recovery be done after the whole hive be read? (see the /* Now read the whole hive */ below)

{
DPRINT1("Recover data from LOG failed\n");
return STATUS_REGISTRY_CORRUPT;
}

DPRINT1("Hive data recovered\n");
}
}

/* Set default boot type */
Expand Down
43 changes: 26 additions & 17 deletions sdk/lib/cmlib/hivewrt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@ HvpWriteLog(
ULONG LastIndex;
PVOID BlockPtr;
BOOLEAN Success;
static ULONG PrintCount = 0;

if (PrintCount++ == 0)
{
UNIMPLEMENTED;
}
return TRUE;


mrmks04 marked this conversation as resolved.
Show resolved Hide resolved
ASSERT(RegistryHive->ReadOnly == FALSE);
ASSERT(RegistryHive->BaseBlock->Length ==
RegistryHive->Storage[Stable].Length * HBLOCK_SIZE);
Expand All @@ -42,18 +36,16 @@ HvpWriteLog(
return FALSE;
}

BitmapSize = RegistryHive->DirtyVector.SizeOfBitMap;
BufferSize = HV_LOG_HEADER_SIZE + sizeof(ULONG) + BitmapSize;
BufferSize = ROUND_UP(BufferSize, HBLOCK_SIZE);

DPRINT("Bitmap size %u buffer size: %u\n", BitmapSize, BufferSize);

Buffer = RegistryHive->Allocate(BufferSize, TRUE, TAG_CM);
BitmapSize = ROUND_UP(sizeof(ULONG) + RegistryHive->DirtyVector.SizeOfBitMap, HSECTOR_SIZE);
BufferSize = HV_LOG_HEADER_SIZE + BitmapSize;
Buffer = RegistryHive->Allocate(BufferSize, TRUE, TAG_CM);
if (Buffer == NULL)
{
return FALSE;
}

RtlZeroMemory(Buffer, BufferSize);

/* Update first update counter and CheckSum */
RegistryHive->BaseBlock->Type = HFILE_TYPE_LOG;
RegistryHive->BaseBlock->Sequence1++;
Expand All @@ -63,9 +55,23 @@ HvpWriteLog(
/* Copy hive header */
RtlCopyMemory(Buffer, RegistryHive->BaseBlock, HV_LOG_HEADER_SIZE);
Ptr = Buffer + HV_LOG_HEADER_SIZE;
RtlCopyMemory(Ptr, "DIRT", 4);
Ptr += 4;
RtlCopyMemory(Ptr, RegistryHive->DirtyVector.Buffer, BitmapSize);
*((PULONG)Ptr) = DIRTY_ID;
Ptr += DIRTY_ID_SIZE;

/* Mark dirty blocks as "FF" - 1 bit per sector */
BlockIndex = 0;
while (BlockIndex < RegistryHive->Storage[Stable].Length)
{
LastIndex = BlockIndex;
BlockIndex = RtlFindSetBits(&RegistryHive->DirtyVector, 1, BlockIndex);
if (BlockIndex == ~0U || BlockIndex < LastIndex)
{
break;
}

Ptr[BlockIndex] = DIRTY_BLOCK;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be RtlSetBits or similar?
Also, why does the vector contents is set only now (as far as I understand)? Shouldn't it be set during registry operations?

Copy link
Contributor Author

@mrmks04 mrmks04 Feb 10, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ReactOS vector contains 1 bit per 1 block (4kb), but in Windows 1 bit per 1 sector (512 bytes). In Windows log file dirty vector contains only 0x00 and 0xFF values, I don't see othrers. And content write only by blocks. It is for Windows compatible. Perhaps it is worth changing the work with the vector.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ReactOS vector contains 1 bit per 1 block (4kb),

This will need to be changed at some point, for NT/Windows compatibility.

BlockIndex++;
}

/* Write hive block and block bitmap */
FileOffset = 0;
Expand Down Expand Up @@ -104,12 +110,15 @@ HvpWriteLog(
FileOffset += HBLOCK_SIZE;
}

/* TODO: Check this function. In some cases log file can be corrrupted */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to be done much sooner before (at top of function)? 🤔🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But file size calculated in while cycle before call this function. I think it neded to test with latest master.

#if 0
Success = RegistryHive->FileSetSize(RegistryHive, HFILE_TYPE_LOG, FileOffset, FileOffset);
if (!Success)
{
DPRINT("FileSetSize failed\n");
return FALSE;
}
#endif

/* Flush the log file */
Success = RegistryHive->FileFlush(RegistryHive, HFILE_TYPE_LOG, NULL, 0);
Expand Down