Skip to content

Commit

Permalink
AdbWinUsbApi.dll: fix race condition crash in WinUsb.dll
Browse files Browse the repository at this point in the history
There is a race condition in AdbWinUsbApi.dll where AdbCloseHandle() of an
interface would clobber the WinUsb handles, causing a concurrent
Adb{Read,Write}EndpointSync() to crash.

The fix is to make AdbCloseHandle(endpoint) set a flag to prevent future IOs
from starting up, abort any pending IOs, and wait for the pending IOs to abort.
Adb{Read,Write}EndpointSync() participates in this scheme.

There is still a race condition if the caller calls AdbCloseHandle(interface)
before calling AdbCloseHandle(endpoint). No AOSP code does this and assuming
that this never happens simplifies the fix.

This fix also ignores Adb{Read,Write}EndpointAsync() (the async APIs) since
those APIs are unused by AOSP.

This fix should not affect devices whose vendor supplies Windows USB drivers
that use a 'legacy kernel driver'. This causes AdbWinApi.dll to call a 'legacy
kernel driver' instead of AdbWinUsbApi.dll (which uses WinUsb.dll which uses a
kernel driver included with Windows). The source code for the 'legacy kernel
driver' was deleted from AOSP over 4 years ago, so it is probably deprecated
(I don't know the official status of it). Even so, I wouldn't be surprised if
some modern 3rd party devices were still using the legacy driver or a similar
driver derived from the original source code.

Also in this change:

 - Added a test case to adb_winapi_test that reproduces the race condition and
   verifies the fix.

 - adb_winapi_test misc: more strictly check return values and error codes,
   symbolize some dumped data to make things more readable, disable old test
   code that looked for specific hardware, test AdbGetInterfaceName() the same
   way adb uses it, fix dumping of initial "handshake".

 - Increased AdbWinUsbApi.dll file version info from 2.0.0.0 to 2.0.0.1.

 - Update AdbWinUsbApi.dll in prebuilt tree.

 - Include AdbWinUsbApi.pdb (debugging symbols) so the DLL can be debugged in
   the future (or at least so crash addresses can be manually symbolized).

 - Update AdbWinApi.dll, AdbWinUsbApi.dll, adb_winapi_test.exe build
   environments to WDK 7.1.0, which seems to be the publicly available closest
   version to what built the last version of these files.

   This entailed modifying SOURCES files to use USE_NATIVE_EH=1 instead of
   USER_C_FLAGS=/EHsc, removing /Wp64, manually setting DLL base addresses to
   the old address, using DEBUG_CRTS=1 to pickup the debug ATL for checked
   builds.

 - Update BUILDME.TXT files with up-to-date instructions.

 - For source files that are already majority CRLF, make the whole file CRLF.

 - Update SOURCES to explicitly set Windows Vista as the target. This means
   that future rebuilders don't need to worry as much about their build
   environment.

Bug: https://code.google.com/p/android/issues/detail?id=161890

Change-Id: I30a4e2ff3919929001c2319c4bb80354f7bcfda0
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
  • Loading branch information
CompareAndSwap authored and enh-google committed Aug 12, 2015
1 parent f6b91cf commit 487b1de
Show file tree
Hide file tree
Showing 18 changed files with 758 additions and 190 deletions.
5 changes: 4 additions & 1 deletion host/windows/.gitignore
Expand Up @@ -6,7 +6,10 @@ usb/Debug
usb/Release
usb/api/obj*
usb/api/*.log
usb/api/*.wrn
usb/adb_winapi_test/obj*
usb/adb_winapi_test/*.log
usb/adb_winapi_test/*.wrn
usb/winusb/obj*
usb/winusb/*.log
usb/winusb/*.log
usb/winusb/*.wrn
15 changes: 0 additions & 15 deletions host/windows/prebuilt/usb/AdbWinApi.def

This file was deleted.

Binary file modified host/windows/prebuilt/usb/AdbWinApi.dll
Binary file not shown.
Binary file added host/windows/prebuilt/usb/AdbWinApi.pdb
Binary file not shown.
Binary file modified host/windows/prebuilt/usb/AdbWinUsbApi.dll
Binary file not shown.
Binary file added host/windows/prebuilt/usb/AdbWinUsbApi.pdb
Binary file not shown.
33 changes: 28 additions & 5 deletions host/windows/usb/adb_winapi_test/BUILDME.TXT
Expand Up @@ -12,8 +12,31 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

In order to build adb_winapi_test.dll you will need to install Windows Driver
Kit, which can be obtained from Microsoft. Assuming that WDK is installed, you
need to set one of the WDK's build environments, "cd" back into this directory,
and execute "build -cbeEIFZ" to clean and rebuild this project, or you can
execute "build -befEIF" to do a minimal build.
In order to build a directory with a SOURCES file you will need to install
the Windows Driver Kit, which can be obtained from Microsoft:

Windows Driver Kit Version 7.1.0
https://www.microsoft.com/en-us/download/details.aspx?id=11800
md5: 8fe981a1706d43ad34bda496e6558f94
sha1: de6abdb8eb4e08942add4aa270c763ed4e3d8242

This old version is used because it can build for Windows Vista (WDK 8.1
cannot), it includes compilers (so it doesn't require Visual Studio), and it is
probably not too far from the WDK that this code was originally built with, so
it should be less risky.

When installing the WDK, uncheck `Device Simulation Framework' because it is
unnecessary and it installs a kernel-mode driver that we don't need.

Assuming that WDK is installed, you need to set one of the WDK's build
environments (Start Menu -> Windows Driver Kits -> x86 Free Build Environment;
choose the one for the oldest version of Windows you want to support),
"cd" back into this directory, and execute "build -cbeEIFZ" to clean and rebuild
this project, or you can execute "build -befEIF" to do a minimal build.

Note that you need to build AdbWinApi.dll (..\api) before you build
this directory, as this depends on the AdbWinApi.lib import library.

When you're done with the WDK build environment, don't forget to right-click the
OACR icon (in the lower-right notification area of the taskbar) and choose
`Close'.
23 changes: 17 additions & 6 deletions host/windows/usb/adb_winapi_test/SOURCES
Expand Up @@ -18,12 +18,17 @@ TARGETNAME = adb_winapi_test
TARGETPATH = obj
TARGETTYPE = PROGRAM

_NT_TARGET_VERSION = $(_NT_TARGET_VERSION_VISTA)

UMTYPE = console
UMENTRY = main

# Use statically linked atl libraries:
USE_STATIC_ATL = 1

# Use STL, default version
USE_STL = 1

# Use multithreaded libraries
USE_LIBCMT = 1

Expand All @@ -34,13 +39,19 @@ TARGETLIBS=$(SDK_LIB_PATH)\ole32.lib \
INCLUDES=$(DDK_INC_PATH)\;$(SDK_INC_PATH)\;$(CRT_INC_PATH)\;$(ATL_INC_PATH)\api

# Common C defines
USER_C_FLAGS = $(USER_C_FLAGS) /FD /wd4100 /nologo
USER_C_FLAGS = $(USER_C_FLAGS) /FD /wd4100 /nologo

# Turn on all warnings, and treat warnings as errors
MSC_WARNING_LEVEL = /W4 /Wp64 /WX
# The STL uses C++ exception handling.
USE_NATIVE_EH=1

PRECOMPILED_CXX = 1
PRECOMPILED_INCLUDE = stdafx.h
PRECOMPILED_SOURCEFILE = stdafx.cpp
# Turn on all warnings, and treat warnings as errors
MSC_WARNING_LEVEL = /W4 /WX

# Disable precompiled header to work-around compiler issue with interaction with
# ASLR on Windows 7 and newer.
# http://blogs.msdn.com/b/vcblog/archive/2009/11/12/visual-c-precompiled-header-errors-on-windows-7.aspx
#PRECOMPILED_CXX = 1
#PRECOMPILED_INCLUDE = stdafx.h
#PRECOMPILED_SOURCEFILE = stdafx.cpp

SOURCES = adb_winapi_test.cpp

0 comments on commit 487b1de

Please sign in to comment.