Well, writing beyond the buffer length we were supplied to null terminate a string seems bad in my book ;)
Cameron On Aug 14, 2011, at 11:00 AM, Alex Ionescu <[email protected]> wrote: > 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, <[email protected]> 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?rev=53232&r1=53231&r2=53232&view=diff > ============================================================================== > --- 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; > > > > _______________________________________________ > Ros-dev mailing list > [email protected] > http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
