From d2acbd19312a66cbee2c49f455eddd82b6700d1d Mon Sep 17 00:00:00 2001 From: David Pursell Date: Wed, 2 Dec 2015 15:14:31 -0800 Subject: [PATCH] adb: add help text for USB permission errors. The current permission messages can be confusing for users who don't know about udev and USB access permissions. This CL adds some checks to try to identify common udev problems, and adds a link to online documentation. Example messages: 1) adb server is in plugdev group but access is still denied: $ adb devices List of devices attached 082f59270073e1e3 no permissions (verify udev rules); see [developer.android.com/tools/device.html] 2) plugdev group exists but adb server is not in it: $ adb shell error: USB permission failure: udev requires plugdev group membership. See [developer.android.com/tools/device.html] for more information. 3) plugdev group does not exist: $ adb shell error: USB permission failure. See [developer.android.com/tools/device.html] for more information. Bug: http://b/25777880 Change-Id: I536565adc12ab657c75151309795674181205db0 --- adb/adb.h | 9 +++++-- adb/transport.cpp | 32 +++++++++++++++---------- adb/transport.h | 2 +- adb/usb_linux.cpp | 52 ++++++++++++++++++++++++++++++++++++++++ adb/usb_linux_client.cpp | 9 +++++++ adb/usb_osx.cpp | 9 +++++++ adb/usb_windows.cpp | 9 +++++++ 7 files changed, 107 insertions(+), 15 deletions(-) diff --git a/adb/adb.h b/adb/adb.h index 5187c8198..a0c3b5024 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -214,18 +214,23 @@ void local_init(int port); void local_connect(int port); int local_connect_arbitrary_ports(int console_port, int adb_port, std::string* error); -/* usb host/client interface */ +// USB host/client interface. void usb_init(); int usb_write(usb_handle *h, const void *data, int len); int usb_read(usb_handle *h, void *data, int len); int usb_close(usb_handle *h); void usb_kick(usb_handle *h); -/* used for USB device detection */ +// USB device detection. #if ADB_HOST int is_adb_interface(int vid, int pid, int usb_class, int usb_subclass, int usb_protocol); #endif +// USB permission error help text. The short version will be one line, long may be multi-line. +// Returns a string message to print, or an empty string if no problems could be found. +std::string UsbNoPermissionsShortHelpText(); +std::string UsbNoPermissionsLongHelpText(); + int adb_commandline(int argc, const char **argv); ConnectionState connection_state(atransport *t); diff --git a/adb/transport.cpp b/adb/transport.cpp index 2f18f2011..6a030d399 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -674,7 +674,11 @@ atransport* acquire_one_transport(TransportType type, const char* serial, adb_mutex_lock(&transport_lock); for (const auto& t : transport_list) { if (t->connection_state == kCsNoPerm) { - *error_out = "insufficient permissions for device"; + *error_out = UsbNoPermissionsLongHelpText(); + // If we couldn't figure out a reasonable help message default to something generic. + if (error_out->empty()) { + *error_out = "insufficient permissions for device"; + } continue; } @@ -748,17 +752,20 @@ atransport* acquire_one_transport(TransportType type, const char* serial, return result; } -const char* atransport::connection_state_name() const { +const std::string atransport::connection_state_name() const { switch (connection_state) { - case kCsOffline: return "offline"; - case kCsBootloader: return "bootloader"; - case kCsDevice: return "device"; - case kCsHost: return "host"; - case kCsRecovery: return "recovery"; - case kCsNoPerm: return "no permissions"; - case kCsSideload: return "sideload"; - case kCsUnauthorized: return "unauthorized"; - default: return "unknown"; + case kCsOffline: return "offline"; + case kCsBootloader: return "bootloader"; + case kCsDevice: return "device"; + case kCsHost: return "host"; + case kCsRecovery: return "recovery"; + case kCsNoPerm: { + std::string message = UsbNoPermissionsShortHelpText(); + return message.empty() ? "no permissions" : message; + } + case kCsSideload: return "sideload"; + case kCsUnauthorized: return "unauthorized"; + default: return "unknown"; } } @@ -866,7 +873,8 @@ static void append_transport(const atransport* t, std::string* result, *result += '\t'; *result += t->connection_state_name(); } else { - android::base::StringAppendF(result, "%-22s %s", serial, t->connection_state_name()); + android::base::StringAppendF(result, "%-22s %s", serial, + t->connection_state_name().c_str()); append_transport_info(result, "", t->devpath, false); append_transport_info(result, "product:", t->product, false); diff --git a/adb/transport.h b/adb/transport.h index d9845b608..76d6afa14 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -90,7 +90,7 @@ public: fdevent auth_fde; size_t failed_auth_attempts = 0; - const char* connection_state_name() const; + const std::string connection_state_name() const; void update_version(int version, size_t payload); int get_protocol_version() const; diff --git a/adb/usb_linux.cpp b/adb/usb_linux.cpp index 0358b6203..7cb154af3 100644 --- a/adb/usb_linux.cpp +++ b/adb/usb_linux.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -595,3 +596,54 @@ void usb_init() { fatal_errno("cannot create device_poll thread"); } } + +static const char kPermissionsHelpUrl[] = "developer.android.com/tools/device.html"; + +// Returns a message describing any potential problems we find with udev, or nullptr if we can't +// find plugdev information (i.e. udev is not installed). +static const char* GetUdevProblem() { + errno = 0; + group* plugdev_group = getgrnam("plugdev"); + + if (plugdev_group == nullptr) { + if (errno != 0) { + D("failed to read plugdev group info: %s", strerror(errno)); + } + // We can't give any generally useful advice here, just let the caller print the help URL. + return nullptr; + } + + // getgroups(2) indicates that the group_member() may not check the egid so we check it + // additionally just to be sure. + if (group_member(plugdev_group->gr_gid) || getegid() == plugdev_group->gr_gid) { + // The user is in plugdev so the problem is likely with the udev rules. + return "verify udev rules"; + } + return "udev requires plugdev group membership"; +} + +// Short help text must be a single line, and will look something like: +// no permissions (reason); see +std::string UsbNoPermissionsShortHelpText() { + std::string help_text = "no permissions"; + + const char* problem = GetUdevProblem(); + if (problem != nullptr) { + help_text += android::base::StringPrintf(" (%s)", problem); + } + + return android::base::StringPrintf("%s; see [%s]", help_text.c_str(), kPermissionsHelpUrl); +} + +// Long help text can span multiple lines and should provide more detailed information. +std::string UsbNoPermissionsLongHelpText() { + std::string header = "USB permission failure"; + + const char* problem = GetUdevProblem(); + if (problem != nullptr) { + header += android::base::StringPrintf(": %s", problem); + } + + return android::base::StringPrintf("%s.\nSee [%s] for more information.", + header.c_str(), kPermissionsHelpUrl); +} diff --git a/adb/usb_linux_client.cpp b/adb/usb_linux_client.cpp index 3495a71cc..6f45b24c2 100644 --- a/adb/usb_linux_client.cpp +++ b/adb/usb_linux_client.cpp @@ -587,3 +587,12 @@ void usb_kick(usb_handle *h) { h->kick(h); } + +// kCsNoPerm is a host-side issue, we can just ignore it here. +std::string UsbNoPermissionsShortHelpText() { + return ""; +} + +std::string UsbNoPermissionsLongHelpText() { + return ""; +} diff --git a/adb/usb_osx.cpp b/adb/usb_osx.cpp index e0dcc756a..f9275e642 100644 --- a/adb/usb_osx.cpp +++ b/adb/usb_osx.cpp @@ -552,3 +552,12 @@ void usb_kick(usb_handle *handle) handle->interface = 0; } } + +// kCsNoPerm is Linux-only. +std::string UsbNoPermissionsShortHelpText() { + return ""; +} + +std::string UsbNoPermissionsLongHelpText() { + return ""; +} diff --git a/adb/usb_windows.cpp b/adb/usb_windows.cpp index 8d3501e24..91246852f 100644 --- a/adb/usb_windows.cpp +++ b/adb/usb_windows.cpp @@ -659,3 +659,12 @@ static void kick_devices() { } adb_mutex_unlock(&usb_lock); } + +// kCsNoPerm is Linux-only. +std::string UsbNoPermissionsShortHelpText() { + return ""; +} + +std::string UsbNoPermissionsLongHelpText() { + return ""; +}