Re: [ros-dev] [ros-diffs] [jgardou] 63700: [NTOSKRNL] - Use ZwSetValueKey instead of NtSetValueKey - Properly probe and copy parameters when NtSetValueKey is called from UMode CORE-7738 #resolve #comment fixed in r63700
Calling Nt in those functions was by design, and is how Windows does it as well. I can explain Nt vs Zw (for the 100th time) as needed, but for now, I'd kindly ask for a revert please.
Second, the new code does not conform to formatting and style guidelines. Please fix the code before comitting.
Best regards, Alex Ionescu
On Tue, Jul 8, 2014 at 11:42 AM, jgardou@svn.reactos.org wrote:
Author: jgardou Date: Tue Jul 8 18:42:20 2014 New Revision: 63700
URL: http://svn.reactos.org/svn/reactos?rev=63700&view=rev Log: [NTOSKRNL]
- Use ZwSetValueKey instead of NtSetValueKey
- Properly probe and copy parameters when NtSetValueKey is called from
UMode CORE-7738 #resolve #comment fixed in r63700
Modified: trunk/reactos/ntoskrnl/config/cmconfig.c trunk/reactos/ntoskrnl/config/cmsysini.c trunk/reactos/ntoskrnl/config/i386/cmhardwr.c trunk/reactos/ntoskrnl/config/ntapi.c trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c trunk/reactos/ntoskrnl/ex/init.c trunk/reactos/ntoskrnl/io/iomgr/bootlog.c
Modified: trunk/reactos/ntoskrnl/config/cmconfig.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmconfig.c?...
============================================================================== --- trunk/reactos/ntoskrnl/config/cmconfig.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/cmconfig.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -97,7 +97,7 @@
/* Setup the component information key */ RtlInitUnicodeString(&ValueName, L"Component Information");
- Status = NtSetValueKey(KeyHandle,
- Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_BINARY,
@@ -124,7 +124,7 @@ if (NT_SUCCESS(Status)) { /* Save the identifier in the registry */
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ,@@ -178,7 +178,7 @@ CmpConfigurationData->BusNumber = BusNumber;
/* Save the actual data */
- Status = NtSetValueKey(KeyHandle,
- Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_FULL_RESOURCE_DESCRIPTOR,
Modified: trunk/reactos/ntoskrnl/config/cmsysini.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmsysini.c?...
============================================================================== --- trunk/reactos/ntoskrnl/config/cmsysini.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/cmsysini.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -416,7 +416,7 @@
/* Key opened, now write to the key */ RtlInitUnicodeString(&KeyName, L"SystemStartOptions");
- Status = NtSetValueKey(KeyHandle,
- Status = ZwSetValueKey(KeyHandle, &KeyName, 0, REG_SZ,
@@ -427,7 +427,7 @@ /* Setup value name for system boot device in ARC format */ RtlInitUnicodeString(&KeyName, L"SystemBootDevice"); RtlCreateUnicodeStringFromAsciiz(&ValueName, LoaderBlock->ArcBootDeviceName);
- Status = NtSetValueKey(KeyHandle,
- Status = ZwSetValueKey(KeyHandle, &KeyName, 0, REG_SZ,
@@ -474,7 +474,7 @@ OBJ_CASE_INSENSITIVE, NULL, NULL);
- Status = NtOpenKey(&SelectHandle, KEY_READ, &ObjectAttributes);
- Status = ZwOpenKey(&SelectHandle, KEY_READ, &ObjectAttributes); if (!NT_SUCCESS(Status)) { /* ReactOS Hack: Hard-code current to 001 for SetupLdr */
@@ -488,16 +488,17 @@ OBJ_CASE_INSENSITIVE, NULL, NULL);
Status = NtCreateKey(&KeyHandle,
Status = ZwCreateKey(&KeyHandle, KEY_ALL_ACCESS, &ObjectAttributes, 0, NULL, 0, &Disposition);
if (!NT_SUCCESS(Status)) return Status;/* Don't need the handle */
if (!NT_SUCCESS(Status))return Status;/* We don't need the handle */ ZwClose(KeyHandle); /* Use hard-coded setting */@@ -533,7 +534,7 @@ OBJ_CASE_INSENSITIVE, NULL, NULL);
- Status = NtCreateKey(&KeyHandle,
- Status = ZwCreateKey(&KeyHandle, KEY_CREATE_LINK, &ObjectAttributes, 0,
@@ -557,7 +558,7 @@ Status = RtlAnsiStringToUnicodeString(&KeyName, &TempString, FALSE);
/* Set the value */
- Status = NtSetValueKey(KeyHandle,
- Status = ZwSetValueKey(KeyHandle, &CmSymbolicLinkValueName, 0, REG_LINK,
@@ -572,7 +573,7 @@ KeyHandle, NULL); Status = NtOpenKey(&ConfigHandle, KEY_READ, &ObjectAttributes);
- NtClose(KeyHandle);
ZwClose(KeyHandle);
/* Check if we don't have one */ if (!NT_SUCCESS(Status))
@@ -689,7 +690,7 @@ ASSERT(STATUS_SUCCESS == Status);
/* Set it */
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &CmSymbolicLinkValueName, 0, REG_LINK,@@ -2180,7 +2181,7 @@ RtlInitUnicodeString(&ValueData, Buffer);
- NtSetValueKey(CurrentVersionKeyHandle,
- ZwSetValueKey(CurrentVersionKeyHandle, &ValueName, 0, REG_SZ,
Modified: trunk/reactos/ntoskrnl/config/i386/cmhardwr.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/i386/cmhard...
============================================================================== --- trunk/reactos/ntoskrnl/config/i386/cmhardwr.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/i386/cmhardwr.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -268,7 +268,7 @@
/* Set the value */ RtlInitUnicodeString(&ValueName, L"PhysicalAddressExtension");
NtSetValueKey(KeyHandle,
ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD,@@ -463,7 +463,7 @@
/* Add it to the registry */ RtlInitUnicodeString(&ValueName,L"ProcessorNameString");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ,@@ -486,7 +486,7 @@
/* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"VendorIdentifier");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ,@@ -502,7 +502,7 @@ { /* Add them to the registry */ RtlInitUnicodeString(&ValueName, L"FeatureSet");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD,@@ -515,7 +515,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"~MHz");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD,@@ -528,7 +528,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"Update Signature");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_BINARY,@@ -621,7 +621,7 @@
/* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosDate");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ,@@ -643,7 +643,7 @@ if (NT_SUCCESS(Status)) { /* Save it to the registry */
Status = NtSetValueKey(BiosHandle,
Status = ZwSetValueKey(BiosHandle, &ValueName, 0, REG_SZ,@@ -703,7 +703,7 @@
/* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosVersion");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ,@@ -746,7 +746,7 @@
/* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosDate");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ,@@ -801,7 +801,7 @@
/* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosVersion");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ,Modified: trunk/reactos/ntoskrnl/config/ntapi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/ntapi.c?rev...
============================================================================== --- trunk/reactos/ntoskrnl/config/ntapi.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/ntapi.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -33,7 +33,10 @@ CM_PARSE_CONTEXT ParseContext = {0}; HANDLE Handle; PAGED_CODE();
- DPRINT("NtCreateKey(OB name %wZ)\n", ObjectAttributes->ObjectName);
- DPRINT("NtCreateKey(Path: %wZ, Root %x, Access: %x, CreateOptions
%x)\n",
ObjectAttributes->ObjectName, ObjectAttributes->RootDirectory,DesiredAccess, CreateOptions);/* Check for user-mode caller */ if (PreviousMode != KernelMode)
@@ -60,7 +63,8 @@ sizeof(OBJECT_ATTRIBUTES), sizeof(ULONG));
if (Disposition) ProbeForWriteUlong(Disposition);
if (Disposition)ProbeForWriteUlong(Disposition); } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) {@@ -101,6 +105,8 @@ } _SEH2_END;
- DPRINT("Returning handle %x, Status %x.\n", Handle, Status);
- /* Return status */ return Status;
} @@ -116,7 +122,8 @@ NTSTATUS Status; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); PAGED_CODE();
- DPRINT("NtOpenKey(OB 0x%wZ)\n", ObjectAttributes->ObjectName);
- DPRINT("NtOpenKey(Path: %wZ, Root %x, Access: %x)\n",
ObjectAttributes->ObjectName,ObjectAttributes->RootDirectory, DesiredAccess);
/* Check for user-mode caller */ if (PreviousMode != KernelMode)@@ -166,6 +173,8 @@ _SEH2_END; }
- DPRINT("Returning handle %x, Status %x.\n", Handle, Status);
- /* Return status */ return Status;
} @@ -301,6 +310,7 @@
/* Dereference and return status */ ObDereferenceObject(KeyObject);
- DPRINT("Returning status %x.\n", Status); return Status;
}
@@ -604,14 +614,52 @@ IN PVOID Data, IN ULONG DataSize) {
- NTSTATUS Status;
- PCM_KEY_BODY KeyObject;
- NTSTATUS Status = STATUS_SUCCESS;
- PCM_KEY_BODY KeyObject = NULL; REG_SET_VALUE_KEY_INFORMATION SetValueKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo;
- UNICODE_STRING ValueNameCopy = *ValueName;
- UNICODE_STRING ValueNameCopy;
- KPROCESSOR_MODE PreviousMode;
- PAGED_CODE();
- PreviousMode = ExGetPreviousMode();
- if (Data && !DataSize)
return STATUS_INVALID_PARAMETER;- /* Probe and copy the data */
- if ((PreviousMode != KernelMode) && Data)
- {
PVOID DataCopy = ExAllocatePoolWithTag(PagedPool, DataSize,TAG_CM);
if (!DataCopy)return STATUS_NO_MEMORY;_SEH2_TRY{ProbeForRead(Data, DataSize, 1);RtlCopyMemory(DataCopy, Data, DataSize);}_SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER){Status = _SEH2_GetExceptionCode();}_SEH2_END;if (!NT_SUCCESS(Status)){ExFreePoolWithTag(DataCopy, TAG_CM);return Status;}Data = DataCopy;- }
- /* Capture the string */
- Status = ProbeAndCaptureUnicodeString(&ValueNameCopy, PreviousMode,
ValueName);
- if (!NT_SUCCESS(Status))
goto end;- DPRINT("NtSetValueKey() KH 0x%p, VN '%wZ', TI %x, T %lu, DS %lu\n",
KeyHandle, ValueName, TitleIndex, Type, DataSize);
KeyHandle, &ValueNameCopy, TitleIndex, Type, DataSize);/* Verify that the handle is valid and is a registry key */ Status = ObReferenceObjectByHandle(KeyHandle,
@@ -620,7 +668,8 @@ ExGetPreviousMode(), (PVOID*)&KeyObject, NULL);
- if (!NT_SUCCESS(Status)) return Status;
if (!NT_SUCCESS(Status))
goto end;/* Make sure the name is aligned, not too long, and the data under
4GB */ if ( (ValueNameCopy.Length > 32767) || @@ -628,8 +677,8 @@ (DataSize > 0x80000000)) { /* Fail */
ObDereferenceObject(KeyObject);return STATUS_INVALID_PARAMETER;
Status = STATUS_INVALID_PARAMETER;goto end;}
/* Ignore any null characters at the end */
@@ -644,14 +693,14 @@ if (KeyObject->KeyControlBlock->ExtFlags & CM_KCB_READ_ONLY_KEY) { /* Fail */
ObDereferenceObject(KeyObject);return STATUS_ACCESS_DENIED;
Status = STATUS_ACCESS_DENIED;goto end;}
/* Setup callback */ PostOperationInfo.Object = (PVOID)KeyObject; SetValueKeyInfo.Object = (PVOID)KeyObject;
- SetValueKeyInfo.ValueName = ValueName;
- SetValueKeyInfo.ValueName = &ValueNameCopy; SetValueKeyInfo.TitleIndex = TitleIndex; SetValueKeyInfo.Type = Type; SetValueKeyInfo.Data = Data;
@@ -673,8 +722,13 @@ PostOperationInfo.Status = Status; CmiCallRegisteredCallbacks(RegNtPostSetValueKey, &PostOperationInfo);
+end: /* Dereference and return status */
- ObDereferenceObject(KeyObject);
- if (KeyObject)
ObDereferenceObject(KeyObject);- ReleaseCapturedUnicodeString(&ValueNameCopy, PreviousMode);
- if ((PreviousMode != KernelMode) && Data)
ExFreePoolWithTag(Data, TAG_CM);}
Modified: trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/powerpc/cmh...
============================================================================== --- trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -267,7 +267,7 @@
/* Set the value */ RtlInitUnicodeString(&ValueName, L"PhysicalAddressExtension");
NtSetValueKey(KeyHandle,
ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD,@@ -465,7 +465,7 @@
/* Add it to the registry */ RtlInitUnicodeString(&ValueName,L"ProcessorNameString");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ,@@ -488,7 +488,7 @@
/* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"VendorIdentifier");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ,@@ -504,7 +504,7 @@ { /* Add them to the registry */ RtlInitUnicodeString(&ValueName, L"FeatureSet");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD,@@ -517,7 +517,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"~MHz");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD,@@ -530,7 +530,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"Update Signature");
Status = NtSetValueKey(KeyHandle,
Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_BINARY,@@ -623,7 +623,7 @@
/* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosDate");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ,@@ -645,7 +645,7 @@ if (NT_SUCCESS(Status)) { /* Save it to the registry */
Status = NtSetValueKey(BiosHandle,
Status = ZwSetValueKey(BiosHandle, &ValueName, 0, REG_SZ,@@ -705,7 +705,7 @@
/* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosVersion");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ,@@ -749,7 +749,7 @@
/* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosDate");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ,@@ -804,7 +804,7 @@
/* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosVersion");
Status = NtSetValueKey(SystemHandle,
Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ,Modified: trunk/reactos/ntoskrnl/ex/init.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/init.c?rev=6370...
============================================================================== --- trunk/reactos/ntoskrnl/ex/init.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/ex/init.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -1853,7 +1853,7 @@ { /* Write the safe boot type */ RtlInitUnicodeString(&KeyName, L"OptionValue");
NtSetValueKey(OptionHandle,
ZwSetValueKey(OptionHandle, &KeyName, 0, REG_DWORD,@@ -1866,7 +1866,7 @@ /* Remember this for later */ Disposition = TRUE; RtlInitUnicodeString(&KeyName, L"UseAlternateShell");
NtSetValueKey(OptionHandle,
ZwSetValueKey(OptionHandle, &KeyName, 0, REG_DWORD,Modified: trunk/reactos/ntoskrnl/io/iomgr/bootlog.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/bootlog.c...
============================================================================== --- trunk/reactos/ntoskrnl/io/iomgr/bootlog.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/io/iomgr/bootlog.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -135,7 +135,7 @@
if (!NT_SUCCESS(Status)) {
DPRINT1("NtSetValueKey() failed (Status %lx)\n", Status);
DPRINT1("ZwSetValueKey() failed (Status %lx)\n", Status);
http://www.osronline.com/article.cfm?id=257 That article was the first I read about Nt vs Zw difference back then. It's quite helpful, and explains it nicely IMO. And saves precious Alex's time :-)
Regards, Aleksey
On 08.07.2014 23:58, Alex Ionescu wrote:
Calling Nt in those functions was by design, and is how Windows does it as well. I can explain Nt vs Zw (for the 100th time) as needed, but for now, I'd kindly ask for a revert please.
Second, the new code does not conform to formatting and style guidelines. Please fix the code before comitting.
Best regards, Alex Ionescu
On Tue, Jul 8, 2014 at 11:42 AM, <jgardou@svn.reactos.org mailto:jgardou@svn.reactos.org> wrote:
Author: jgardou Date: Tue Jul 8 18:42:20 2014 New Revision: 63700 URL: http://svn.reactos.org/svn/reactos?rev=63700&view=rev Log: [NTOSKRNL] - Use ZwSetValueKey instead of NtSetValueKey - Properly probe and copy parameters when NtSetValueKey is called from UMode CORE-7738 #resolve #comment fixed in r63700
Explaining the difference between Zw and Nt for the 101st time will not explain why it's used here, instead it would be an argument FOR converting to Zw, since the functions are called with kernel mode buffers, so under "normal" circumstances Zw would be correct. The reason why it can be done here, is because these are (as far as I checked) all init functions, and we are already in PreviousMode == KernelMode when we call them, so there is no difference. The reason why it is done might be performance, since we are saving the system service overhead here.
Regarding the style guidelines: If you are referring to
- if (Disposition) ProbeForWriteUlong(Disposition); + if (Disposition) + ProbeForWriteUlong(Disposition);
then I have to disappoint you.
Line breaking
1.
Each statement should get its own line.
*Right:*
x++; y++;
if (Condition) DoSomething();
*Wrong:*
x++; y++;
if (Condition) DoSomething();
see: https://reactos.org/wiki/Coding_Style
Regards, TImo
Am 08.07.2014 21:58, schrieb Alex Ionescu:
Calling Nt in those functions was by design, and is how Windows does it as well. I can explain Nt vs Zw (for the 100th time) as needed, but for now, I'd kindly ask for a revert please.
Second, the new code does not conform to formatting and style guidelines. Please fix the code before comitting.
Best regards, Alex Ionescu
Please don't waste your time, I know the difference between Zw* and Nt* calls.
What I don't know is then why the corrected version of NtSetValueKey fails on this precise call when probing the data from kernel mode: http://git.reactos.org/?p=reactos.git;a=blob;f=reactos/ntoskrnl/config/cmsys... Unless you disagree on the fact that the new code is correct. I certainly should have investigated. Sorry about that.
Regarding code style, I'm not sure about what you are referring to.
As for reverting, I hope you will be patient enough to wait for tomorrow ;-)
Best regards Jérôme
Le 08.07.2014 21:58, Alex Ionescu a écrit :
Calling Nt in those functions was by design, and is how Windows does it as well. I can explain Nt vs Zw (for the 100th time) as needed, but for now, I'd kindly ask for a revert please.
Second, the new code does not conform to formatting and style guidelines. Please fix the code before comitting.
Best regards, Alex Ionescu
On Tue, Jul 8, 2014 at 11:42 AM, <jgardou@svn.reactos.org mailto:jgardou@svn.reactos.org> wrote:
Author: jgardou Date: Tue Jul 8 18:42:20 2014 New Revision: 63700 URL: http://svn.reactos.org/svn/reactos?rev=63700&view=rev Log: [NTOSKRNL] - Use ZwSetValueKey instead of NtSetValueKey - Properly probe and copy parameters when NtSetValueKey is called from UMode CORE-7738 #resolve #comment fixed in r63700 Modified: trunk/reactos/ntoskrnl/config/cmconfig.c trunk/reactos/ntoskrnl/config/cmsysini.c trunk/reactos/ntoskrnl/config/i386/cmhardwr.c trunk/reactos/ntoskrnl/config/ntapi.c trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c trunk/reactos/ntoskrnl/ex/init.c trunk/reactos/ntoskrnl/io/iomgr/bootlog.c Modified: trunk/reactos/ntoskrnl/config/cmconfig.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmconfig.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/config/cmconfig.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/cmconfig.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -97,7 +97,7 @@ /* Setup the component information key */ RtlInitUnicodeString(&ValueName, L"Component Information"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_BINARY, @@ -124,7 +124,7 @@ if (NT_SUCCESS(Status)) { /* Save the identifier in the registry */ - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ, @@ -178,7 +178,7 @@ CmpConfigurationData->BusNumber = BusNumber; /* Save the actual data */ - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_FULL_RESOURCE_DESCRIPTOR, Modified: trunk/reactos/ntoskrnl/config/cmsysini.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmsysini.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/config/cmsysini.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/cmsysini.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -416,7 +416,7 @@ /* Key opened, now write to the key */ RtlInitUnicodeString(&KeyName, L"SystemStartOptions"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &KeyName, 0, REG_SZ, @@ -427,7 +427,7 @@ /* Setup value name for system boot device in ARC format */ RtlInitUnicodeString(&KeyName, L"SystemBootDevice"); RtlCreateUnicodeStringFromAsciiz(&ValueName, LoaderBlock->ArcBootDeviceName); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &KeyName, 0, REG_SZ, @@ -474,7 +474,7 @@ OBJ_CASE_INSENSITIVE, NULL, NULL); - Status = NtOpenKey(&SelectHandle, KEY_READ, &ObjectAttributes); + Status = ZwOpenKey(&SelectHandle, KEY_READ, &ObjectAttributes); if (!NT_SUCCESS(Status)) { /* ReactOS Hack: Hard-code current to 001 for SetupLdr */ @@ -488,16 +488,17 @@ OBJ_CASE_INSENSITIVE, NULL, NULL); - Status = NtCreateKey(&KeyHandle, + Status = ZwCreateKey(&KeyHandle, KEY_ALL_ACCESS, &ObjectAttributes, 0, NULL, 0, &Disposition); - if (!NT_SUCCESS(Status)) return Status; - - /* Don't need the handle */ + if (!NT_SUCCESS(Status)) + return Status; + + /* We don't need the handle */ ZwClose(KeyHandle); /* Use hard-coded setting */ @@ -533,7 +534,7 @@ OBJ_CASE_INSENSITIVE, NULL, NULL); - Status = NtCreateKey(&KeyHandle, + Status = ZwCreateKey(&KeyHandle, KEY_CREATE_LINK, &ObjectAttributes, 0, @@ -557,7 +558,7 @@ Status = RtlAnsiStringToUnicodeString(&KeyName, &TempString, FALSE); /* Set the value */ - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &CmSymbolicLinkValueName, 0, REG_LINK, @@ -572,7 +573,7 @@ KeyHandle, NULL); Status = NtOpenKey(&ConfigHandle, KEY_READ, &ObjectAttributes); - NtClose(KeyHandle); + ZwClose(KeyHandle); /* Check if we don't have one */ if (!NT_SUCCESS(Status)) @@ -689,7 +690,7 @@ ASSERT(STATUS_SUCCESS == Status); /* Set it */ - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &CmSymbolicLinkValueName, 0, REG_LINK, @@ -2180,7 +2181,7 @@ RtlInitUnicodeString(&ValueData, Buffer); - NtSetValueKey(CurrentVersionKeyHandle, + ZwSetValueKey(CurrentVersionKeyHandle, &ValueName, 0, REG_SZ, Modified: trunk/reactos/ntoskrnl/config/i386/cmhardwr.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/i386/cmhardwr.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/config/i386/cmhardwr.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/i386/cmhardwr.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -268,7 +268,7 @@ /* Set the value */ RtlInitUnicodeString(&ValueName, L"PhysicalAddressExtension"); - NtSetValueKey(KeyHandle, + ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD, @@ -463,7 +463,7 @@ /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"ProcessorNameString"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ, @@ -486,7 +486,7 @@ /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"VendorIdentifier"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ, @@ -502,7 +502,7 @@ { /* Add them to the registry */ RtlInitUnicodeString(&ValueName, L"FeatureSet"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD, @@ -515,7 +515,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"~MHz"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD, @@ -528,7 +528,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"Update Signature"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_BINARY, @@ -621,7 +621,7 @@ /* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosDate"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ, @@ -643,7 +643,7 @@ if (NT_SUCCESS(Status)) { /* Save it to the registry */ - Status = NtSetValueKey(BiosHandle, + Status = ZwSetValueKey(BiosHandle, &ValueName, 0, REG_SZ, @@ -703,7 +703,7 @@ /* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosVersion"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ, @@ -746,7 +746,7 @@ /* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosDate"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ, @@ -801,7 +801,7 @@ /* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosVersion"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ, Modified: trunk/reactos/ntoskrnl/config/ntapi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/ntapi.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/config/ntapi.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/ntapi.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -33,7 +33,10 @@ CM_PARSE_CONTEXT ParseContext = {0}; HANDLE Handle; PAGED_CODE(); - DPRINT("NtCreateKey(OB name %wZ)\n", ObjectAttributes->ObjectName); + + DPRINT("NtCreateKey(Path: %wZ, Root %x, Access: %x, CreateOptions %x)\n", + ObjectAttributes->ObjectName, ObjectAttributes->RootDirectory, + DesiredAccess, CreateOptions); /* Check for user-mode caller */ if (PreviousMode != KernelMode) @@ -60,7 +63,8 @@ sizeof(OBJECT_ATTRIBUTES), sizeof(ULONG)); - if (Disposition) ProbeForWriteUlong(Disposition); + if (Disposition) + ProbeForWriteUlong(Disposition); } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) { @@ -101,6 +105,8 @@ } _SEH2_END; + DPRINT("Returning handle %x, Status %x.\n", Handle, Status); + /* Return status */ return Status; } @@ -116,7 +122,8 @@ NTSTATUS Status; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); PAGED_CODE(); - DPRINT("NtOpenKey(OB 0x%wZ)\n", ObjectAttributes->ObjectName); + DPRINT("NtOpenKey(Path: %wZ, Root %x, Access: %x)\n", + ObjectAttributes->ObjectName, ObjectAttributes->RootDirectory, DesiredAccess); /* Check for user-mode caller */ if (PreviousMode != KernelMode) @@ -166,6 +173,8 @@ _SEH2_END; } + DPRINT("Returning handle %x, Status %x.\n", Handle, Status); + /* Return status */ return Status; } @@ -301,6 +310,7 @@ /* Dereference and return status */ ObDereferenceObject(KeyObject); + DPRINT("Returning status %x.\n", Status); return Status; } @@ -604,14 +614,52 @@ IN PVOID Data, IN ULONG DataSize) { - NTSTATUS Status; - PCM_KEY_BODY KeyObject; + NTSTATUS Status = STATUS_SUCCESS; + PCM_KEY_BODY KeyObject = NULL; REG_SET_VALUE_KEY_INFORMATION SetValueKeyInfo; REG_POST_OPERATION_INFORMATION PostOperationInfo; - UNICODE_STRING ValueNameCopy = *ValueName; + UNICODE_STRING ValueNameCopy; + KPROCESSOR_MODE PreviousMode; + PAGED_CODE(); + + PreviousMode = ExGetPreviousMode(); + + if (Data && !DataSize) + return STATUS_INVALID_PARAMETER; + + /* Probe and copy the data */ + if ((PreviousMode != KernelMode) && Data) + { + PVOID DataCopy = ExAllocatePoolWithTag(PagedPool, DataSize, TAG_CM); + if (!DataCopy) + return STATUS_NO_MEMORY; + _SEH2_TRY + { + ProbeForRead(Data, DataSize, 1); + RtlCopyMemory(DataCopy, Data, DataSize); + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + Status = _SEH2_GetExceptionCode(); + } + _SEH2_END; + + if (!NT_SUCCESS(Status)) + { + ExFreePoolWithTag(DataCopy, TAG_CM); + return Status; + } + Data = DataCopy; + } + + /* Capture the string */ + Status = ProbeAndCaptureUnicodeString(&ValueNameCopy, PreviousMode, ValueName); + if (!NT_SUCCESS(Status)) + goto end; + DPRINT("NtSetValueKey() KH 0x%p, VN '%wZ', TI %x, T %lu, DS %lu\n", - KeyHandle, ValueName, TitleIndex, Type, DataSize); + KeyHandle, &ValueNameCopy, TitleIndex, Type, DataSize); /* Verify that the handle is valid and is a registry key */ Status = ObReferenceObjectByHandle(KeyHandle, @@ -620,7 +668,8 @@ ExGetPreviousMode(), (PVOID*)&KeyObject, NULL); - if (!NT_SUCCESS(Status)) return Status; + if (!NT_SUCCESS(Status)) + goto end; /* Make sure the name is aligned, not too long, and the data under 4GB */ if ( (ValueNameCopy.Length > 32767) || @@ -628,8 +677,8 @@ (DataSize > 0x80000000)) { /* Fail */ - ObDereferenceObject(KeyObject); - return STATUS_INVALID_PARAMETER; + Status = STATUS_INVALID_PARAMETER; + goto end; } /* Ignore any null characters at the end */ @@ -644,14 +693,14 @@ if (KeyObject->KeyControlBlock->ExtFlags & CM_KCB_READ_ONLY_KEY) { /* Fail */ - ObDereferenceObject(KeyObject); - return STATUS_ACCESS_DENIED; + Status = STATUS_ACCESS_DENIED; + goto end; } /* Setup callback */ PostOperationInfo.Object = (PVOID)KeyObject; SetValueKeyInfo.Object = (PVOID)KeyObject; - SetValueKeyInfo.ValueName = ValueName; + SetValueKeyInfo.ValueName = &ValueNameCopy; SetValueKeyInfo.TitleIndex = TitleIndex; SetValueKeyInfo.Type = Type; SetValueKeyInfo.Data = Data; @@ -673,8 +722,13 @@ PostOperationInfo.Status = Status; CmiCallRegisteredCallbacks(RegNtPostSetValueKey, &PostOperationInfo); +end: /* Dereference and return status */ - ObDereferenceObject(KeyObject); + if (KeyObject) + ObDereferenceObject(KeyObject); + ReleaseCapturedUnicodeString(&ValueNameCopy, PreviousMode); + if ((PreviousMode != KernelMode) && Data) + ExFreePoolWithTag(Data, TAG_CM); return Status; } Modified: trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/config/powerpc/cmhardwr.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -267,7 +267,7 @@ /* Set the value */ RtlInitUnicodeString(&ValueName, L"PhysicalAddressExtension"); - NtSetValueKey(KeyHandle, + ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD, @@ -465,7 +465,7 @@ /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"ProcessorNameString"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ, @@ -488,7 +488,7 @@ /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"VendorIdentifier"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_SZ, @@ -504,7 +504,7 @@ { /* Add them to the registry */ RtlInitUnicodeString(&ValueName, L"FeatureSet"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD, @@ -517,7 +517,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"~MHz"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_DWORD, @@ -530,7 +530,7 @@ { /* Add it to the registry */ RtlInitUnicodeString(&ValueName, L"Update Signature"); - Status = NtSetValueKey(KeyHandle, + Status = ZwSetValueKey(KeyHandle, &ValueName, 0, REG_BINARY, @@ -623,7 +623,7 @@ /* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosDate"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ, @@ -645,7 +645,7 @@ if (NT_SUCCESS(Status)) { /* Save it to the registry */ - Status = NtSetValueKey(BiosHandle, + Status = ZwSetValueKey(BiosHandle, &ValueName, 0, REG_SZ, @@ -705,7 +705,7 @@ /* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"SystemBiosVersion"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ, @@ -749,7 +749,7 @@ /* Write the date into the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosDate"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_SZ, @@ -804,7 +804,7 @@ /* Write the BIOS Version to the registry */ RtlInitUnicodeString(&ValueName, L"VideoBiosVersion"); - Status = NtSetValueKey(SystemHandle, + Status = ZwSetValueKey(SystemHandle, &ValueName, 0, REG_MULTI_SZ, Modified: trunk/reactos/ntoskrnl/ex/init.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/init.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/ex/init.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/ex/init.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -1853,7 +1853,7 @@ { /* Write the safe boot type */ RtlInitUnicodeString(&KeyName, L"OptionValue"); - NtSetValueKey(OptionHandle, + ZwSetValueKey(OptionHandle, &KeyName, 0, REG_DWORD, @@ -1866,7 +1866,7 @@ /* Remember this for later */ Disposition = TRUE; RtlInitUnicodeString(&KeyName, L"UseAlternateShell"); - NtSetValueKey(OptionHandle, + ZwSetValueKey(OptionHandle, &KeyName, 0, REG_DWORD, Modified: trunk/reactos/ntoskrnl/io/iomgr/bootlog.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/bootlog.c?rev=63700&r1=63699&r2=63700&view=diff ============================================================================== --- trunk/reactos/ntoskrnl/io/iomgr/bootlog.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/io/iomgr/bootlog.c [iso-8859-1] Tue Jul 8 18:42:20 2014 @@ -135,7 +135,7 @@ if (!NT_SUCCESS(Status)) { - DPRINT1("NtSetValueKey() failed (Status %lx)\n", Status); + DPRINT1("ZwSetValueKey() failed (Status %lx)\n", Status); } else {
Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
Maybe he is referring to this:
- if (Data && !DataSize)
Some kernel code use
if (!(ActiveTimers) || !(Timers))
or
if ((!CmpNoWrite) && (!CmpHoldLazyFlush))
Dmitry
Aaaahhhh.... Extra parentheses!!!!
-----Message d'origine----- De : Ros-dev [mailto:ros-dev-bounces@reactos.org] De la part de Dmitry Gorbachev Envoyé : mercredi 9 juillet 2014 00:52 À : ReactOS Development List Objet : Re: [ros-dev] [ros-diffs] [jgardou] 63700: [NTOSKRNL] - Use ZwSetValueKey instead of NtSetValueKey - Properly probe and copy parameters when NtSetValueKey is called from UMode CORE-7738 #resolve #comment fixed in r63700
Maybe he is referring to this:
- if (Data && !DataSize)
Some kernel code use
if (!(ActiveTimers) || !(Timers))
or
if ((!CmpNoWrite) && (!CmpHoldLazyFlush))
Dmitry
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
Sorry I was not on e-mail yesterday.
Indeed Timo did a good job explaining -- when you are
1) Inside ntoskrnl 2) In guranteed KernelMode context
It is better to use Nt for performance, as Zw provides no advantages. Especially in the boot path, eliminating system calls is a good thing. That's why we should prefer to use Nt whenever possible in system threads or boot code. In fact not even Windows always respects this "rule"/benefit, and we could do better as well.
As for coding style, indeed, double parantheses are something I'd like to see. I also personally hate one-line if statements without braces, however I understand that the coding style does not enforce that... HOWEVER, if you see a file written 99% by one dev with 99% one style, it seems a bit offputting to add a few extra lines of code that use a different style. The point of coding guidelines is consistency.
Best regards, Alex Ionescu
On Tue, Jul 8, 2014 at 5:43 PM, Hermès BÉLUSCA - MAÏTO < hermes.belusca@sfr.fr> wrote:
Aaaahhhh.... Extra parentheses!!!!
-----Message d'origine----- De : Ros-dev [mailto:ros-dev-bounces@reactos.org] De la part de Dmitry Gorbachev Envoyé : mercredi 9 juillet 2014 00:52 À : ReactOS Development List Objet : Re: [ros-dev] [ros-diffs] [jgardou] 63700: [NTOSKRNL] - Use ZwSetValueKey instead of NtSetValueKey - Properly probe and copy parameters when NtSetValueKey is called from UMode CORE-7738 #resolve #comment fixed in r63700
Maybe he is referring to this:
- if (Data && !DataSize)
Some kernel code use
if (!(ActiveTimers) || !(Timers))
or
if ((!CmpNoWrite) && (!CmpHoldLazyFlush))
Dmitry
Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
-
Aleksey Bragin -
Alex Ionescu -
Dmitry Gorbachev -
Hermès BÉLUSCA - MAÏTO -
Jérôme Gardou -
Timo Kreuzer