Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

USB: fix substandard locking for the sysfs files

This patch straightens out some locking issues in the USB sysfs
interface:

Deauthorization will destroy existing configurations.
Attributes that read from udev->actconfig need to lock the
device to prevent races. Likewise for the rawdescriptor
values.

Attributes that access an interface's current alternate
setting should use ACCESS_ONCE() to obtain the cur_altsetting
pointer, to protect against concurrent altsetting changes.

The supports_autosuspend() attribute routine accesses values
from an interface's driver, so it should lock the interface
(rather than the usb_device) to protect against concurrent
unbinds. Once this is done, the routine can be simplified
considerably.

Scalar values that are stored directly in the usb_device structure are
always available. They do not require any locking. The same is true
of the cached interface string descriptor, because it is not
deallocated until the usb_host_interface structure is destroyed.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Alan Stern and committed by
Greg Kroah-Hartman
232275a0 9a37a503

+26 -27
+26 -27
drivers/usb/core/sysfs.c
··· 23 23 { \ 24 24 struct usb_device *udev; \ 25 25 struct usb_host_config *actconfig; \ 26 + ssize_t rc = 0; \ 26 27 \ 27 28 udev = to_usb_device(dev); \ 29 + usb_lock_device(udev); \ 28 30 actconfig = udev->actconfig; \ 29 31 if (actconfig) \ 30 - return sprintf(buf, format_string, \ 32 + rc = sprintf(buf, format_string, \ 31 33 actconfig->desc.field); \ 32 - else \ 33 - return 0; \ 34 + usb_unlock_device(udev); \ 35 + return rc; \ 34 36 } \ 35 37 36 38 #define usb_actconfig_attr(field, format_string) \ ··· 47 45 { 48 46 struct usb_device *udev; 49 47 struct usb_host_config *actconfig; 48 + ssize_t rc = 0; 50 49 51 50 udev = to_usb_device(dev); 51 + usb_lock_device(udev); 52 52 actconfig = udev->actconfig; 53 - if (!actconfig) 54 - return 0; 55 - return sprintf(buf, "%dmA\n", usb_get_max_power(udev, actconfig)); 53 + if (actconfig) 54 + rc = sprintf(buf, "%dmA\n", usb_get_max_power(udev, actconfig)); 55 + usb_unlock_device(udev); 56 + return rc; 56 57 } 57 58 static DEVICE_ATTR_RO(bMaxPower); 58 59 ··· 64 59 { 65 60 struct usb_device *udev; 66 61 struct usb_host_config *actconfig; 62 + ssize_t rc = 0; 67 63 68 64 udev = to_usb_device(dev); 65 + usb_lock_device(udev); 69 66 actconfig = udev->actconfig; 70 - if ((!actconfig) || (!actconfig->string)) 71 - return 0; 72 - return sprintf(buf, "%s\n", actconfig->string); 67 + if (actconfig && actconfig->string) 68 + rc = sprintf(buf, "%s\n", actconfig->string); 69 + usb_unlock_device(udev); 70 + return rc; 73 71 } 74 72 static DEVICE_ATTR_RO(configuration); 75 73 ··· 772 764 * Following that are the raw descriptor entries for all the 773 765 * configurations (config plus subsidiary descriptors). 774 766 */ 767 + usb_lock_device(udev); 775 768 for (cfgno = -1; cfgno < udev->descriptor.bNumConfigurations && 776 769 nleft > 0; ++cfgno) { 777 770 if (cfgno < 0) { ··· 793 784 off -= srclen; 794 785 } 795 786 } 787 + usb_unlock_device(udev); 796 788 return count - nleft; 797 789 } 798 790 ··· 880 870 char *string; 881 871 882 872 intf = to_usb_interface(dev); 883 - string = intf->cur_altsetting->string; 884 - barrier(); /* The altsetting might change! */ 885 - 873 + string = ACCESS_ONCE(intf->cur_altsetting->string); 886 874 if (!string) 887 875 return 0; 888 876 return sprintf(buf, "%s\n", string); ··· 896 888 897 889 intf = to_usb_interface(dev); 898 890 udev = interface_to_usbdev(intf); 899 - alt = intf->cur_altsetting; 891 + alt = ACCESS_ONCE(intf->cur_altsetting); 900 892 901 893 return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02X" 902 894 "ic%02Xisc%02Xip%02Xin%02X\n", ··· 917 909 struct device_attribute *attr, 918 910 char *buf) 919 911 { 920 - struct usb_interface *intf; 921 - struct usb_device *udev; 922 - int ret; 912 + int s; 923 913 924 - intf = to_usb_interface(dev); 925 - udev = interface_to_usbdev(intf); 926 - 927 - usb_lock_device(udev); 914 + device_lock(dev); 928 915 /* Devices will be autosuspended even when an interface isn't claimed */ 929 - if (!intf->dev.driver || 930 - to_usb_driver(intf->dev.driver)->supports_autosuspend) 931 - ret = sprintf(buf, "%u\n", 1); 932 - else 933 - ret = sprintf(buf, "%u\n", 0); 934 - usb_unlock_device(udev); 916 + s = (!dev->driver || to_usb_driver(dev->driver)->supports_autosuspend); 917 + device_unlock(dev); 935 918 936 - return ret; 919 + return sprintf(buf, "%u\n", s); 937 920 } 938 921 static DEVICE_ATTR_RO(supports_autosuspend); 939 922