New issue
Advanced search Search tips

Issue 595052 link

Starred by 1 user
Status: WontFix
Owner:
joedow@chromium.org
Closed: Mar 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Uninitialized read in IPC::ChannelWin::IsNamedServerInitialized

Project Member Reported by reillyg@chromium.org, Mar 15 2016 
Dr. Memory reports that IPC::ChannelWin::IsNamedServerInitialized results in a call to NtFsControlFile with an uninitialized parameter. This is triggered by a number of remoting tests:

https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%283%29/builds/9861

UNINITIALIZED READ: reading 0x0300c7fd-0x0300c7fe 1 byte(s) within 0x0300c7f0-0x0300c874
# 0 system call NtFsControlFile parameter #6
# 1 KERNELBASE.dll!WaitNamedPipeW                                             +0x2c6    (0x759083cc <KERNELBASE.dll+0x83cc>)
# 2 ipc.dll!IPC::ChannelWin::IsNamedServerInitialized                          [ipc\ipc_channel_win.cc:194]
# 3 remoting::GnubbyAuthHandlerWinTest::CreateGnubbyConnection                 [remoting\host\security_key\gnubby_auth_handler_win_unittest.cc:141]
# 4 remoting::GnubbyAuthHandlerWinTest_HandleGnubbyErrorResponse_Test::TestBody [remoting\host\security_key\gnubby_auth_handler_win_unittest.cc:454]
# 5 testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
Note: @0:01:33.353 in thread 4536

UNINITIALIZED READ: reading 0x02dd288d-0x02dd288e 1 byte(s) within 0x02dd2880-0x02dd2908
# 0 system call NtFsControlFile parameter #6
# 1 KERNELBASE.dll!WaitNamedPipeW                                             +0x2c6    (0x759083cc <KERNELBASE.dll+0x83cc>)
# 2 ipc.dll!IPC::ChannelWin::IsNamedServerInitialized                          [ipc\ipc_channel_win.cc:194]
# 3 remoting::GnubbyAuthHandlerWinTest::CreateGnubbyConnection                 [remoting\host\security_key\gnubby_auth_handler_win_unittest.cc:141]
# 4 remoting::GnubbyAuthHandlerWinTest_HandleConcurrentGnubbyRequests_Test::TestBody [remoting\host\security_key\gnubby_auth_handler_win_unittest.cc:275]
# 5 testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
Note: @0:01:32.890 in thread 4536

UNINITIALIZED READ: reading 0x07f2b5d5-0x07f2b5d6 1 byte(s) within 0x07f2b5c8-0x07f2b660
# 0 system call NtFsControlFile parameter #6
# 1 KERNELBASE.dll!WaitNamedPipeW                                             +0x2c6    (0x759083cc <KERNELBASE.dll+0x83cc>)
# 2 ipc.dll!IPC::ChannelWin::IsNamedServerInitialized                          [ipc\ipc_channel_win.cc:194]
# 3 remoting::RemoteSecurityKeyIpcClient::WaitForSecurityKeyIpcServerChannel   [remoting\host\security_key\remote_security_key_ipc_client.cc:44]
# 4 remoting::RemoteSecurityKeyIpcClientTest::EstablishConnection              [remoting\host\security_key\remote_security_key_ipc_client_unittest.cc:171]
# 5 remoting::RemoteSecurityKeyIpcClientTest_GnubbyIpcServerRunningInWrongSession_Test::TestBody [remoting\host\security_key\remote_security_key_ipc_client_unittest.cc:329]
# 6 testing::Test::Run                                                         [testing\gtest\src\gtest.cc:2474]
Note: @0:01:40.182 in thread 4536

Comment 1 by reillyg@chromium.org, Mar 15 2016

Components: Internals>Core
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42a0278c91dea9291822d55a79fbe7d111d3db2b

commit 42a0278c91dea9291822d55a79fbe7d111d3db2b
Author: reillyg <reillyg@chromium.org>
Date: Tue Mar 15 18:19:59 2016

Suppress Dr. Memory error in IPC::ChannelWin::IsNamedServerInitialized.

Dr. Memory complains that a parameter to the NtFsControlFile system call
is not initialized.

BUG= 595052 
TBR=benwells@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/1803063003

Cr-Commit-Position: refs/heads/master@{#381257}

[modify] https://crrev.com/42a0278c91dea9291822d55a79fbe7d111d3db2b/tools/valgrind/drmemory/suppressions_full.txt

Comment 3 by jamiewa...@chromium.org, Mar 15 2016

Labels: M-52
Owner: joedow@chromium.org
Status: Assigned (was: Untriaged)
Joe, can you see if this is an easy fix on our end?

Comment 4 by reillyg@chromium.org, Mar 15 2016 

Parameter #6 of NtFsControlFile is FsControlCode. Since this call is reached through WaitNamedPipeW I think that our code may not be in control of this parameter and so this is an OS issue.

NTSTATUS ZwFsControlFile(
  _In_      HANDLE           FileHandle,
  _In_opt_  HANDLE           Event,
  _In_opt_  PIO_APC_ROUTINE  ApcRoutine,
  _In_opt_  PVOID            ApcContext,
  _Out_     PIO_STATUS_BLOCK IoStatusBlock,
  _In_      ULONG            FsControlCode,
  _In_opt_  PVOID            InputBuffer,
  _In_      ULONG            InputBufferLength,
  _Out_opt_ PVOID            OutputBuffer,
  _In_      ULONG            OutputBufferLength
);

Comment 5 by joedow@chromium.org, Mar 15 2016

Status: WontFix (was: Assigned)
Agree with Reilly's conclusion.  The params we pass to WaitNamedPipe seem legit and under the covers the OS API is not passing valid data to another OS API.

Comment 6 by bruening@chromium.org, Mar 19 2016 

This is a known bug, now fixed: https://github.com/DynamoRIO/drmemory/issues/1827
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 20 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62ed9214eb560ec9ecbf726703429a8f0b6735be

commit 62ed9214eb560ec9ecbf726703429a8f0b6735be
Author: bruening <bruening@chromium.org>
Date: Sun Mar 20 01:02:23 2016

Remove a Dr. Memory suppression no longer needed

There was a false positive that is now fixed in the latest Dr. Memory
update.

BUG= 595052 
TBR=reillyg@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/1815943002

Cr-Commit-Position: refs/heads/master@{#382208}

[modify] https://crrev.com/62ed9214eb560ec9ecbf726703429a8f0b6735be/tools/valgrind/drmemory/suppressions_full.txt
►

Sign in to add a comment