Re: [ros-diffs] [hbirr] 17397: Fixed a terminating NULL in IoRegisterDeviceInterface.

[Joseph Galbraith]

I'm curious why we are making this string nul terminated?

IoCreateSymbolicLink() should not care (I'm virtually
certain it doesn't under windows.)  And I'm pretty
sure that IoRegisterDeviceInterface isn't expected
to return a nul terminated string either.  And I'm also
pretty sure our implementation doesn't actually return
it NUL terminated because the length doesn't include
the NUL termination (rightly so or the symbolic link
would actually have a NUL character as part of it's
name) and the RtlMoveMemory() at the end of the function
is based of off Length().

I'm pretty sure that virtually no windows kernel mode function
taking a UNICODE_STRING structure (or a STRING structure)
expects nul termination...

Thanks,

Joseph

hbirr(a)svn.reactos.com wrote:
>   Fixed a terminating NULL in
> IoRegisterDeviceInterface.
>  
>  Modified: trunk/reactos/ntoskrnl/io/deviface.c
>  
>  ------------------------------------------------------------------------
>  *Modified: trunk/reactos/ntoskrnl/io/deviface.c*
>  
>  --- trunk/reactos/ntoskrnl/io/deviface.c	2005-08-15 16:41:43 UTC (rev 17396)
>  +++ trunk/reactos/ntoskrnl/io/deviface.c	2005-08-15 16:47:15 UTC (rev 17397)
>  @@ -815,7 +815,7 @@
>  
>         RtlAppendUnicodeToString(SymbolicLinkName, L"\\");
>         RtlAppendUnicodeStringToString(SymbolicLinkName, ReferenceString);
>      }
>  
>  -   SymbolicLinkName->Buffer[SymbolicLinkName->Length] = '\0';
>  
>  +   SymbolicLinkName->Buffer[SymbolicLinkName->Length/sizeof(WCHAR)] =
> L'\0';
>  
>   
>      /* Create symbolic link */
>      DPRINT("IoRegisterDeviceInterface(): creating symbolic link %wZ ->
> %wZ\n", SymbolicLinkName, &PdoNameInfo->Name);
>

[Hartmut Birr]

Joseph Galbraith wrote:

>   I'm curious why we are making this string nul
> terminated?
>
>  IoCreateSymbolicLink() should not care (I'm virtually
>  certain it doesn't under windows.)  And I'm pretty
>  sure that IoRegisterDeviceInterface isn't expected
>  to return a nul terminated string either.  And I'm also
>  pretty sure our implementation doesn't actually return
>  it NUL terminated because the length doesn't include
>  the NUL termination (rightly so or the symbolic link
>  would actually have a NUL character as part of it's
>  name) and the RtlMoveMemory() at the end of the function
>  is based of off Length().
>
>  I'm pretty sure that virtually no windows kernel mode function
>  taking a UNICODE_STRING structure (or a STRING structure)
>  expects nul termination...
>  
We are accessing often the buffer of a string with wcslen, wcschr and
wcsrchr. This is dangerous. The real intention for this changes was to
prevent the kernel from crashing within the red zone check.

- Hartmut