From 5990191c4e4802567881db2f19db4adbddc64e1e Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 23 Aug 2016 15:28:43 -0700 Subject: [PATCH 1/2] adb: check our socketpair ends in our win32 emulation. In our Win32 socketpair emulation, check that the ends are properly connected before returning. Change-Id: I33d356fd9ebcac89fc6a89a5200e926032220383 Test: no additional failing tests in adb_test.exe --- adb/sysdeps.h | 4 ++++ adb/sysdeps_win32.cpp | 29 +++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 333b861bc..322f99256 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -285,6 +285,10 @@ extern int adb_socket_accept(int serverfd, struct sockaddr* addr, socklen_t #undef accept #define accept ___xxx_accept +int adb_getsockname(int fd, struct sockaddr* sockaddr, socklen_t* optlen); +#undef getsockname +#define getsockname(...) ___xxx_getsockname(__VA__ARGS__) + // Returns the local port number of a bound socket, or -1 on failure. int adb_socket_get_local_port(int fd); diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 109a5486c..5fda27b8a 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -1061,7 +1061,7 @@ int adb_getsockname(int fd, struct sockaddr* sockaddr, socklen_t* optlen) { return -1; } - int result = getsockname(fh->fh_socket, sockaddr, optlen); + int result = (getsockname)(fh->fh_socket, sockaddr, optlen); if (result == SOCKET_ERROR) { const DWORD err = WSAGetLastError(); D("adb_getsockname: setsockopt on fd %d failed: %s\n", fd, @@ -1119,6 +1119,11 @@ int adb_socketpair(int sv[2]) { int local_port = -1; std::string error; + struct sockaddr_storage peer_addr = {}; + struct sockaddr_storage client_addr = {}; + socklen_t peer_socklen = sizeof(peer_addr); + socklen_t client_socklen = sizeof(client_addr); + server = network_loopback_server(0, SOCK_STREAM, &error); if (server < 0) { D("adb_socketpair: failed to create server: %s", error.c_str()); @@ -1138,12 +1143,32 @@ int adb_socketpair(int sv[2]) { goto fail; } - accepted = adb_socket_accept(server, nullptr, nullptr); + // Make sure that the peer that connected to us and the client are the same. + accepted = adb_socket_accept(server, reinterpret_cast(&peer_addr), &peer_socklen); if (accepted < 0) { D("adb_socketpair: failed to accept: %s", strerror(errno)); goto fail; } + + if (adb_getsockname(client, reinterpret_cast(&client_addr), &client_socklen) != 0) { + D("adb_socketpair: failed to getpeername: %s", strerror(errno)); + goto fail; + } + + if (peer_socklen != client_socklen) { + D("adb_socketpair: client and peer sockaddrs have different lengths"); + errno = EIO; + goto fail; + } + + if (memcmp(&peer_addr, &client_addr, peer_socklen) != 0) { + D("adb_socketpair: client and peer sockaddrs don't match"); + errno = EIO; + goto fail; + } + adb_close(server); + sv[0] = client; sv[1] = accepted; return 0; From 78e1eb1949fd48bcb0578276ffdc2ff3c7ddd89b Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 23 Aug 2016 15:41:56 -0700 Subject: [PATCH 2/2] adb: remove unnecessary addr arguments to accept. Follow up to https://android-review.googlesource.com/#/c/261412/ Change-Id: I0ee130db302940f3224cc823a26b02fc45da0fca Test: mma --- adb/adb_auth_client.cpp | 4 +--- adb/adb_listeners.cpp | 13 ++----------- adb/jdwp_service.cpp | 5 +---- adb/transport_local.cpp | 6 +----- 4 files changed, 5 insertions(+), 23 deletions(-) diff --git a/adb/adb_auth_client.cpp b/adb/adb_auth_client.cpp index 9d8dfc04a..84ad6efae 100644 --- a/adb/adb_auth_client.cpp +++ b/adb/adb_auth_client.cpp @@ -151,9 +151,7 @@ void adb_auth_confirm_key(unsigned char* key, size_t len, atransport* t) { } static void adb_auth_listener(int fd, unsigned events, void* data) { - sockaddr_storage addr; - socklen_t alen = sizeof(addr); - int s = adb_socket_accept(fd, reinterpret_cast(&addr), &alen); + int s = adb_socket_accept(fd, nullptr, nullptr); if (s < 0) { PLOG(ERROR) << "Failed to accept"; return; diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp index f54603cad..0299be3d3 100644 --- a/adb/adb_listeners.cpp +++ b/adb/adb_listeners.cpp @@ -71,10 +71,7 @@ static ListenerList& listener_list = *new ListenerList(); static void ss_listener_event_func(int _fd, unsigned ev, void *_l) { if (ev & FDE_READ) { - sockaddr_storage ss; - sockaddr* addrp = reinterpret_cast(&ss); - socklen_t alen = sizeof(ss); - int fd = adb_socket_accept(_fd, addrp, &alen); + int fd = adb_socket_accept(_fd, nullptr, nullptr); if (fd < 0) return; int rcv_buf_size = CHUNK_SIZE; @@ -96,13 +93,7 @@ static void listener_event_func(int _fd, unsigned ev, void* _l) asocket *s; if (ev & FDE_READ) { - sockaddr_storage ss; - sockaddr* addrp = reinterpret_cast(&ss); - socklen_t alen; - int fd; - - alen = sizeof(ss); - fd = adb_socket_accept(_fd, addrp, &alen); + int fd = adb_socket_accept(_fd, nullptr, nullptr); if (fd < 0) { return; } diff --git a/adb/jdwp_service.cpp b/adb/jdwp_service.cpp index 7a4480168..027821a7d 100644 --- a/adb/jdwp_service.cpp +++ b/adb/jdwp_service.cpp @@ -444,10 +444,7 @@ static void jdwp_control_event(int s, unsigned events, void* _control) { JdwpControl* control = (JdwpControl*)_control; if (events & FDE_READ) { - sockaddr_storage ss; - sockaddr* addrp = reinterpret_cast(&ss); - socklen_t addrlen = sizeof(ss); - int s = adb_socket_accept(control->listen_socket, addrp, &addrlen); + int s = adb_socket_accept(control->listen_socket, nullptr, nullptr); if (s < 0) { if (errno == ECONNABORTED) { /* oops, the JDWP process died really quick */ diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index d918cc7ac..d620a3766 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -197,9 +197,6 @@ static void client_socket_thread(void* x) { static void server_socket_thread(void* arg) { int serverfd, fd; - sockaddr_storage ss; - sockaddr *addrp = reinterpret_cast(&ss); - socklen_t alen; int port = (int) (uintptr_t) arg; adb_thread_setname("server socket"); @@ -217,9 +214,8 @@ static void server_socket_thread(void* arg) { close_on_exec(serverfd); } - alen = sizeof(ss); D("server: trying to get new connection from %d", port); - fd = adb_socket_accept(serverfd, addrp, &alen); + fd = adb_socket_accept(serverfd, nullptr, nullptr); if(fd >= 0) { D("server: new connection on fd %d", fd); close_on_exec(fd);