232275a USB: fix substandard locking for the sysfs files
introduced needed locking into sysfs operations on USB devices
It, however, uses uninterruptible sleep and if the error
handling is on extreme cases of sleep lengths of 10s of seconds
are possible. Unless we are removing the device we should use
interruptible sleep.
Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
---
drivers/usb/core/sysfs.c | 69 ++++++++++++++++++++++++++++++++++--------------
include/linux/device.h | 5 ++++
include/linux/usb.h | 7 ++---
3 files changed, 58 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 65b6e6b..69da1d5 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -23,10 +23,12 @@ static ssize_t field##_show(struct device *dev, \
{ \
struct usb_device *udev; \
struct usb_host_config *actconfig; \
- ssize_t rc = 0; \
+ ssize_t rc; \
\
udev = to_usb_device(dev); \
- usb_lock_device(udev); \
+ rc = usb_lock_device_interruptible(udev); \
+ if (rc < 0) \
+ return -EINTR; \
actconfig = udev->actconfig; \
if (actconfig) \
rc = sprintf(buf, format_string, \
@@ -47,10 +49,12 @@ static ssize_t bMaxPower_show(struct device *dev,
{
struct usb_device *udev;
struct usb_host_config *actconfig;
- ssize_t rc = 0;
+ ssize_t rc;
udev = to_usb_device(dev);
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
actconfig = udev->actconfig;
if (actconfig)
rc = sprintf(buf, "%dmA\n", usb_get_max_power(udev, actconfig));
@@ -64,10 +68,12 @@ static ssize_t configuration_show(struct device *dev,
{
struct usb_device *udev;
struct usb_host_config *actconfig;
- ssize_t rc = 0;
+ ssize_t rc;
udev = to_usb_device(dev);
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
actconfig = udev->actconfig;
if (actconfig && actconfig->string)
rc = sprintf(buf, "%s\n", actconfig->string);
@@ -84,11 +90,13 @@ static ssize_t bConfigurationValue_store(struct device *dev,
const char *buf, size_t count)
{
struct usb_device *udev = to_usb_device(dev);
- int config, value;
+ int config, value, rc;
if (sscanf(buf, "%d", &config) != 1 || config < -1 || config > 255)
return -EINVAL;
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
value = usb_set_configuration(udev, config);
usb_unlock_device(udev);
return (value < 0) ? value : count;
@@ -105,7 +113,9 @@ static ssize_t name##_show(struct device *dev, \
int retval; \
\
udev = to_usb_device(dev); \
- usb_lock_device(udev); \
+ retval = usb_lock_device_interruptible(udev); \
+ if (retval < 0) \
+ return -EINTR; \
retval = sprintf(buf, "%s\n", udev->name); \
usb_unlock_device(udev); \
return retval; \
@@ -224,11 +234,13 @@ static ssize_t avoid_reset_quirk_store(struct device *dev,
const char *buf, size_t count)
{
struct usb_device *udev = to_usb_device(dev);
- int val;
+ int val, rc;
if (sscanf(buf, "%d", &val) != 1 || val < 0 || val > 1)
return -EINVAL;
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
if (val)
udev->quirks |= USB_QUIRK_RESET;
else
@@ -294,7 +306,7 @@ static ssize_t persist_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct usb_device *udev = to_usb_device(dev);
- int value;
+ int value, rc;
/* Hubs are always enabled for USB_PERSIST */
if (udev->descriptor.bDeviceClass == USB_CLASS_HUB)
@@ -303,7 +315,9 @@ static ssize_t persist_store(struct device *dev, struct device_attribute *attr,
if (sscanf(buf, "%d", &value) != 1)
return -EINVAL;
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
udev->persist_enabled = !!value;
usb_unlock_device(udev);
return count;
@@ -420,13 +434,16 @@ static ssize_t level_store(struct device *dev, struct device_attribute *attr,
int len = count;
char *cp;
int rc = count;
+ int rv;
warn_level();
cp = memchr(buf, '\n', count);
if (cp)
len = cp - buf;
- usb_lock_device(udev);
+ rv = usb_lock_device_interruptible(udev);
+ if (rv < 0)
+ return -EINTR;
if (len == sizeof on_string - 1 &&
strncmp(buf, on_string, len) == 0)
@@ -466,7 +483,9 @@ static ssize_t usb2_hardware_lpm_store(struct device *dev,
bool value;
int ret;
- usb_lock_device(udev);
+ ret = usb_lock_device_interruptible(udev);
+ if (ret < 0)
+ return -EINTR;
ret = strtobool(buf, &value);
@@ -536,8 +555,11 @@ static ssize_t usb3_hardware_lpm_u1_show(struct device *dev,
{
struct usb_device *udev = to_usb_device(dev);
const char *p;
+ int rc;
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
if (udev->usb3_lpm_u1_enabled)
p = "enabled";
@@ -555,8 +577,11 @@ static ssize_t usb3_hardware_lpm_u2_show(struct device *dev,
{
struct usb_device *udev = to_usb_device(dev);
const char *p;
+ int rc;
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
if (udev->usb3_lpm_u2_enabled)
p = "enabled";
@@ -815,14 +840,16 @@ read_descriptors(struct file *filp, struct kobject *kobj,
struct usb_device *udev = to_usb_device(dev);
size_t nleft = count;
size_t srclen, n;
- int cfgno;
+ int cfgno, rc;
void *src;
/* The binary attribute begins with the device descriptor.
* Following that are the raw descriptor entries for all the
* configurations (config plus subsidiary descriptors).
*/
- usb_lock_device(udev);
+ rc = usb_lock_device_interruptible(udev);
+ if (rc < 0)
+ return -EINTR;
for (cfgno = -1; cfgno < udev->descriptor.bNumConfigurations &&
nleft > 0; ++cfgno) {
if (cfgno < 0) {
@@ -969,7 +996,9 @@ static ssize_t supports_autosuspend_show(struct device *dev,
{
int s;
- device_lock(dev);
+ s = device_lock_interruptible(dev);
+ if (s < 0)
+ return -EINTR;
/* Devices will be autosuspended even when an interface isn't claimed */
s = (!dev->driver || to_usb_driver(dev->driver)->supports_autosuspend);
device_unlock(dev);
diff --git a/include/linux/device.h b/include/linux/device.h
index b8f411b..d6eded7 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -957,6 +957,11 @@ static inline void device_lock(struct device *dev)
mutex_lock(&dev->mutex);
}
+static inline int device_lock_interruptible(struct device *dev)
+{
+ return mutex_lock_interruptible(&dev->mutex);
+}
+
static inline int device_trylock(struct device *dev)
{
return mutex_trylock(&dev->mutex);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 89533ba..8a3d81f 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -642,9 +642,10 @@ extern struct usb_device *usb_hub_find_child(struct usb_device *hdev,
if (!child) continue; else
/* USB device locking */
-#define usb_lock_device(udev) device_lock(&(udev)->dev)
-#define usb_unlock_device(udev) device_unlock(&(udev)->dev)
-#define usb_trylock_device(udev) device_trylock(&(udev)->dev)
+#define usb_lock_device(udev) device_lock(&(udev)->dev)
+#define usb_unlock_device(udev) device_unlock(&(udev)->dev)
+#define usb_lock_device_interruptible(udev) device_lock_interruptible(&(udev)->dev)
+#define usb_trylock_device(udev) device_trylock(&(udev)->dev)
extern int usb_lock_device_for_reset(struct usb_device *udev,
const struct usb_interface *iface);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
