From 3af9e40c66839c72c0e4f52ead0c60f20873560a Mon Sep 17 00:00:00 2001
From: Andrew Chant <achant@google.com>
Date: Wed, 1 Nov 2017 17:32:35 -0700
Subject: [PATCH] usblib: fix race & delay waiting for perms on open

in usb_device_open, if permission is denied for a USB
device node, the current code retries read-only, then sleeps
for a second before retrying.

If the permission was changed to +rw between the two file opens,
the device could be inadvertently opened read-only.

Also, change the polling interval to 100ms.  1s is a long wait
for a function on the critical path of a user interaction.

Bug: 68337205
Bug: 68782236

Test: with debug messages enabled, connected & disconnected
a USB audio headset.  Saw the writeable file descriptor
returned.

Change-Id: I06048cc2c09bf6ed1abada5d12b5559be768fbaf
---
 libusbhost/usbhost.c | 46 +++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/libusbhost/usbhost.c b/libusbhost/usbhost.c
index 44b878d2c..4d286bf9d 100644
--- a/libusbhost/usbhost.c
+++ b/libusbhost/usbhost.c
@@ -324,29 +324,39 @@ void usb_host_run(struct usb_host_context *context,
 
 struct usb_device *usb_device_open(const char *dev_name)
 {
-    int fd, did_retry = 0, writeable = 1;
-
+    int fd, attempts, writeable = 1;
+    const int SLEEP_BETWEEN_ATTEMPTS_US = 100000; /* 100 ms */
+    const int64_t MAX_ATTEMPTS = 10;              /* 1s */
     D("usb_device_open %s\n", dev_name);
 
-retry:
-    fd = open(dev_name, O_RDWR);
-    if (fd < 0) {
-        /* if we fail, see if have read-only access */
-        fd = open(dev_name, O_RDONLY);
-        D("usb_device_open open returned %d errno %d\n", fd, errno);
-        if (fd < 0 && (errno == EACCES || errno == ENOENT) && !did_retry) {
-            /* work around race condition between inotify and permissions management */
-            sleep(1);
-            did_retry = 1;
-            goto retry;
+    /* Hack around waiting for permissions to be set on the USB device node.
+     * Should really be a timeout instead of attempt count, and should REALLY
+     * be triggered by the perm change via inotify rather than polling.
+     */
+    for (attempts = 0; attempts < MAX_ATTEMPTS; ++attempts) {
+        if (access(dev_name, R_OK | W_OK) == 0) {
+            writeable = 1;
+            break;
+        } else {
+            if (access(dev_name, R_OK) == 0) {
+                /* double check that write permission didn't just come along too! */
+                writeable = (access(dev_name, R_OK | W_OK) == 0);
+                break;
+            }
         }
-
-        if (fd < 0)
-            return NULL;
-        writeable = 0;
-        D("[ usb open read-only %s fd = %d]\n", dev_name, fd);
+        /* not writeable or readable - sleep and try again. */
+        D("usb_device_open no access sleeping\n");
+        usleep(SLEEP_BETWEEN_ATTEMPTS_US);
     }
 
+    if (writeable) {
+        fd = open(dev_name, O_RDWR);
+    } else {
+        fd = open(dev_name, O_RDONLY);
+    }
+    D("usb_device_open open returned %d writeable %d errno %d\n", fd, writeable, errno);
+    if (fd < 0) return NULL;
+
     struct usb_device* result = usb_device_new(dev_name, fd);
     if (result)
         result->writeable = writeable;