Re: [ros-dev] [ros-diffs] [cgutman] 53232: [NTOSKRNL] - Fix NULL termination of strings in IoGetDeviceProperty - Fixes garbage displayed in the Enumerator field of the device manager property page

[Alex Ionescu]

And, you have, of course, confirmed that this piece of kernel code is wrong,
and that DevMgr/Cfgapi/SetupApi are doing the right thing, right?

So if I was to reverse engineer ntoskrnl I wouldn't discover that it was
actually doing the same thing as the un-"fixed" code, right?

Best regards,
Alex Ionescu

On Sun, Aug 14, 2011 at 10:44 AM, [e-mail] wrote:
> Author: cgutman
> Date: Sun Aug 14 14:44:34 2011
> New Revision: 53232
>
> URL: http://svn.reactos.org/svn/reactos?rev=53232&view=rev
> Log:
> [NTOSKRNL]
> Fix NULL termination of strings in IoGetDeviceProperty
> Fixes garbage displayed in the Enumerator field of the device manager
>
> property page
>
> Modified:
>    trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c
>
> Modified: trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c
> URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c...
>
> ==============================================================================
> --- trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c [iso-8859-1] Sun Aug 14
> 14:44:34 2011
> @@ -3467,6 +3467,8 @@
>     NTSTATUS Status = STATUS_BUFFER_TOO_SMALL;
>     GUID BusTypeGuid;
>     POBJECT_NAME_INFORMATION ObjectNameInfo = NULL;
>
> BOOLEAN NullTerminate = FALSE;
>
> DPRINT("IoGetDeviceProperty(0x%p %d)\n", DeviceObject, DeviceProperty);
>
> /* Assume failure */
>
> @@ -3517,7 +3519,10 @@
>             /* Get the name from the path */
>             EnumeratorNameEnd = wcschr(DeviceInstanceName,
> OBJ_NAME_PATH_SEPARATOR);
>             ASSERT(EnumeratorNameEnd);
>        /* This string needs to be NULL-terminated */
>        NullTerminate = TRUE;
>       /* This is the format of the returned data */
>       PIP_RETURN_DATA((EnumeratorNameEnd - DeviceInstanceName) *
>
> sizeof(WCHAR),
>                             DeviceInstanceName);
> @@ -3567,7 +3572,10 @@
>                 /* It's up to the caller to try again */
>                 Status = STATUS_BUFFER_TOO_SMALL;
>             }
>        /* This string needs to be NULL-terminated */
>        NullTerminate = TRUE;
>       /* Return if successful */
>       if (NT_SUCCESS(Status))
>
> PIP_RETURN_DATA(ObjectNameInfo->Name.Length,
>
> ObjectNameInfo->Name.Buffer);
> @@ -3633,15 +3641,14 @@
>     else if (NT_SUCCESS(Status))
>     {
>         /* We know up-front how much data to expect, check the caller's
> buffer */
>    *ResultLength = ReturnLength;
>    if (ReturnLength <= BufferLength)
>    *ResultLength = ReturnLength + (NullTerminate ?
>
> sizeof(UNICODE_NULL) : 0);
>    if (*ResultLength <= BufferLength)
>   {
>       /* Buffer is all good, copy the data */
>       RtlCopyMemory(PropertyBuffer, Data, ReturnLength);
>
>        /* Check for properties that require a null-terminated string
>
> */
>        if ((DeviceProperty == DevicePropertyEnumeratorName) ||
>            (DeviceProperty ==
>
> DevicePropertyPhysicalDeviceObjectName))
>        /* Check if we need to NULL-terminate the string */
>        if (NullTerminate)
>       {
>           /* Terminate the string */
>           ((PWCHAR)PropertyBuffer)[ReturnLength / sizeof(WCHAR)] =
>
> UNICODE_NULL;