USB: create attributes before sending uevent

This patch (as1087d) fixes a long-standing problem in usbcore: Device, interface, and endpoint attributes aren't added until _after_ the creation uevent has already been broadcast. Unfortunately there are a few attributes which cannot be created that early. The "descriptors" attribute is binary and so must be created separately. The power-management attributes can't be created until the dev/power/ group exists. And the interface string can vary from one altsetting to another, so it has to be created dynamically. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Cc: Kay Sievers <kay.sievers@vrfy.org> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

USB: create attributes before sending uevent
This patch (as1087d) fixes a long-standing problem in usbcore: Device, interface, and endpoint attributes aren't added until _after_ the creation uevent has already been broadcast. Unfortunately there are a few attributes which cannot be created that early. The "descriptors" attribute is binary and so must be created separately. The power-management attributes can't be created until the dev/power/ group exists. And the interface string can vary from one altsetting to another, so it has to be created dynamically. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Cc: Kay Sievers <kay.sievers@vrfy.org> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Alan Stern · Greg Kroah-Hartman
1 parent 6986a978ee
Showing 5 changed files with 96 additions and 58 deletions Side-by-side Diff
drivers/usb/core/endpoint.c
drivers/usb/core/message.c
drivers/usb/core/sysfs.c
drivers/usb/core/usb.c
drivers/usb/core/usb.h
@@ -156,6 +156,10 @@
 static struct attribute_group ep_dev_attr_grp = {
 	.attrs = ep_dev_attrs,
 };
+static struct attribute_group *ep_dev_groups[] = {
+	&ep_dev_attr_grp,
+	NULL
+};
  
 static int usb_endpoint_major_init(void)
 {
@@ -298,6 +302,7 @@
  
 	ep_dev->desc = &endpoint->desc;
 	ep_dev->udev = udev;
+	ep_dev->dev.groups = ep_dev_groups;
 	ep_dev->dev.devt = MKDEV(usb_endpoint_major, ep_dev->minor);
 	ep_dev->dev.class = ep_class->class;
 	ep_dev->dev.parent = parent;
@@ -309,9 +314,6 @@
 	retval = device_register(&ep_dev->dev);
 	if (retval)
 		goto error_chrdev;
-	retval = sysfs_create_group(&ep_dev->dev.kobj, &ep_dev_attr_grp);
-	if (retval)
-		goto error_group;
  
 	/* create the symlink to the old-style "ep_XX" directory */
 	sprintf(name, "ep_%02x", endpoint->desc.bEndpointAddress);
@@ -322,8 +324,6 @@
 	return retval;
  
 error_link:
-	sysfs_remove_group(&ep_dev->dev.kobj, &ep_dev_attr_grp);
-error_group:
 	device_unregister(&ep_dev->dev);
 	destroy_endpoint_class();
 	return retval;
@@ -348,7 +348,6 @@
  
 		sprintf(name, "ep_%02x", endpoint->desc.bEndpointAddress);
 		sysfs_remove_link(&ep_dev->dev.parent->kobj, name);
-		sysfs_remove_group(&ep_dev->dev.kobj, &ep_dev_attr_grp);
 		device_unregister(&ep_dev->dev);
 		endpoint->ep_dev = NULL;
 		destroy_endpoint_class();
@@ -1607,6 +1607,7 @@
 		intf->dev.driver = NULL;
 		intf->dev.bus = &usb_bus_type;
 		intf->dev.type = &usb_if_device_type;
+		intf->dev.groups = usb_interface_groups;
 		intf->dev.dma_mask = dev->dev.dma_mask;
 		device_initialize(&intf->dev);
 		mark_quiesced(intf);
@@ -538,6 +538,46 @@
 	.attrs = dev_attrs,
 };
  
+/* When modifying this list, be sure to modify dev_string_attrs_are_visible()
+ * accordingly.
+ */
+static struct attribute *dev_string_attrs[] = {
+	&dev_attr_manufacturer.attr,
+	&dev_attr_product.attr,
+	&dev_attr_serial.attr,
+	NULL
+};
+
+static mode_t dev_string_attrs_are_visible(struct kobject *kobj,
+		struct attribute *a, int n)
+{
+	struct usb_device *udev = to_usb_device(
+			container_of(kobj, struct device, kobj));
+
+	if (a == &dev_attr_manufacturer.attr) {
+		if (udev->manufacturer == NULL)
+			return 0;
+	} else if (a == &dev_attr_product.attr) {
+		if (udev->product == NULL)
+			return 0;
+	} else if (a == &dev_attr_serial.attr) {
+		if (udev->serial == NULL)
+			return 0;
+	}
+	return a->mode;
+}
+
+static struct attribute_group dev_string_attr_grp = {
+	.attrs =	dev_string_attrs,
+	.is_visible =	dev_string_attrs_are_visible,
+};
+
+struct attribute_group *usb_device_groups[] = {
+	&dev_attr_grp,
+	&dev_string_attr_grp,
+	NULL
+};
+
 /* Binary descriptors */
  
 static ssize_t
@@ -591,10 +631,9 @@
 	struct device *dev = &udev->dev;
 	int retval;
  
-	retval = sysfs_create_group(&dev->kobj, &dev_attr_grp);
-	if (retval)
-		return retval;
-
+	/* Unforunately these attributes cannot be created before
+	 * the uevent is broadcast.
+	 */
 	retval = device_create_bin_file(dev, &dev_bin_attr_descriptors);
 	if (retval)
 		goto error;
@@ -607,21 +646,6 @@
 	if (retval)
 		goto error;
  
-	if (udev->manufacturer) {
-		retval = device_create_file(dev, &dev_attr_manufacturer);
-		if (retval)
-			goto error;
-	}
-	if (udev->product) {
-		retval = device_create_file(dev, &dev_attr_product);
-		if (retval)
-			goto error;
-	}
-	if (udev->serial) {
-		retval = device_create_file(dev, &dev_attr_serial);
-		if (retval)
-			goto error;
-	}
 	retval = usb_create_ep_files(dev, &udev->ep0, udev);
 	if (retval)
 		goto error;
  
@@ -636,13 +660,9 @@
 	struct device *dev = &udev->dev;
  
 	usb_remove_ep_files(&udev->ep0);
-	device_remove_file(dev, &dev_attr_manufacturer);
-	device_remove_file(dev, &dev_attr_product);
-	device_remove_file(dev, &dev_attr_serial);
 	remove_power_attributes(dev);
 	remove_persist_attributes(dev);
 	device_remove_bin_file(dev, &dev_bin_attr_descriptors);
-	sysfs_remove_group(&dev->kobj, &dev_attr_grp);
 }
  
 /* Interface Accociation Descriptor fields */
  
  
@@ -688,17 +708,15 @@
 		struct device_attribute *attr, char *buf)
 {
 	struct usb_interface *intf;
-	struct usb_device *udev;
-	int len;
+	char *string;
  
 	intf = to_usb_interface(dev);
-	udev = interface_to_usbdev(intf);
-	len = snprintf(buf, 256, "%s", intf->cur_altsetting->string);
-	if (len < 0)
+	string = intf->cur_altsetting->string;
+	barrier();		/* The altsetting might change! */
+
+	if (!string)
 		return 0;
-	buf[len] = '\n';
-	buf[len+1] = 0;
-	return len+1;
+	return sprintf(buf, "%s\n", string);
 }
 static DEVICE_ATTR(interface, S_IRUGO, show_interface_string, NULL);
  
@@ -727,18 +745,6 @@
 }
 static DEVICE_ATTR(modalias, S_IRUGO, show_modalias, NULL);
  
-static struct attribute *intf_assoc_attrs[] = {
-	&dev_attr_iad_bFirstInterface.attr,
-	&dev_attr_iad_bInterfaceCount.attr,
-	&dev_attr_iad_bFunctionClass.attr,
-	&dev_attr_iad_bFunctionSubClass.attr,
-	&dev_attr_iad_bFunctionProtocol.attr,
-	NULL,
-};
-static struct attribute_group intf_assoc_attr_grp = {
-	.attrs = intf_assoc_attrs,
-};
-
 static struct attribute *intf_attrs[] = {
 	&dev_attr_bInterfaceNumber.attr,
 	&dev_attr_bAlternateSetting.attr,
@@ -753,6 +759,37 @@
 	.attrs = intf_attrs,
 };
  
+static struct attribute *intf_assoc_attrs[] = {
+	&dev_attr_iad_bFirstInterface.attr,
+	&dev_attr_iad_bInterfaceCount.attr,
+	&dev_attr_iad_bFunctionClass.attr,
+	&dev_attr_iad_bFunctionSubClass.attr,
+	&dev_attr_iad_bFunctionProtocol.attr,
+	NULL,
+};
+
+static mode_t intf_assoc_attrs_are_visible(struct kobject *kobj,
+		struct attribute *a, int n)
+{
+	struct usb_interface *intf = to_usb_interface(
+			container_of(kobj, struct device, kobj));
+
+	if (intf->intf_assoc == NULL)
+		return 0;
+	return a->mode;
+}
+
+static struct attribute_group intf_assoc_attr_grp = {
+	.attrs =	intf_assoc_attrs,
+	.is_visible =	intf_assoc_attrs_are_visible,
+};
+
+struct attribute_group *usb_interface_groups[] = {
+	&intf_attr_grp,
+	&intf_assoc_attr_grp,
+	NULL
+};
+
 static inline void usb_create_intf_ep_files(struct usb_interface *intf,
 		struct usb_device *udev)
 {
  
  
  
@@ -777,23 +814,21 @@
  
 int usb_create_sysfs_intf_files(struct usb_interface *intf)
 {
-	struct device *dev = &intf->dev;
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct usb_host_interface *alt = intf->cur_altsetting;
 	int retval;
  
 	if (intf->sysfs_files_created)
 		return 0;
-	retval = sysfs_create_group(&dev->kobj, &intf_attr_grp);
-	if (retval)
-		return retval;
  
+	/* The interface string may be present in some altsettings
+	 * and missing in others.  Hence its attribute cannot be created
+	 * before the uevent is broadcast.
+	 */
 	if (alt->string == NULL)
 		alt->string = usb_cache_string(udev, alt->desc.iInterface);
 	if (alt->string)
-		retval = device_create_file(dev, &dev_attr_interface);
-	if (intf->intf_assoc)
-		retval = sysfs_create_group(&dev->kobj, &intf_assoc_attr_grp);
+		retval = device_create_file(&intf->dev, &dev_attr_interface);
 	usb_create_intf_ep_files(intf, udev);
 	intf->sysfs_files_created = 1;
 	return 0;
@@ -807,8 +842,6 @@
 		return;
 	usb_remove_intf_ep_files(intf);
 	device_remove_file(dev, &dev_attr_interface);
-	sysfs_remove_group(&dev->kobj, &intf_attr_grp);
-	sysfs_remove_group(&intf->dev.kobj, &intf_assoc_attr_grp);
 	intf->sysfs_files_created = 0;
 }
@@ -291,6 +291,7 @@
 	device_initialize(&dev->dev);
 	dev->dev.bus = &usb_bus_type;
 	dev->dev.type = &usb_device_type;
+	dev->dev.groups = usb_device_groups;
 	dev->dev.dma_mask = bus->controller->dma_mask;
 	set_dev_node(&dev->dev, dev_to_node(bus->controller));
 	dev->state = USB_STATE_ATTACHED;
@@ -130,6 +130,10 @@
 /* for labeling diagnostics */
 extern const char *usbcore_name;
  
+/* sysfs stuff */
+extern struct attribute_group *usb_device_groups[];
+extern struct attribute_group *usb_interface_groups[];
+
 /* usbfs stuff */
 extern struct mutex usbfs_mutex;
 extern struct usb_driver usbfs_driver;
...	...	@@ -156,6 +156,10 @@
156	156	static struct attribute_group ep_dev_attr_grp = {
157	157	.attrs = ep_dev_attrs,
158	158	};
	159	+static struct attribute_group *ep_dev_groups[] = {
	160	+ &ep_dev_attr_grp,
	161	+ NULL
	162	+};
159	163
160	164	static int usb_endpoint_major_init(void)
161	165	{
...	...	@@ -298,6 +302,7 @@
298	302
299	303	ep_dev->desc = &endpoint->desc;
300	304	ep_dev->udev = udev;
	305	+ ep_dev->dev.groups = ep_dev_groups;
301	306	ep_dev->dev.devt = MKDEV(usb_endpoint_major, ep_dev->minor);
302	307	ep_dev->dev.class = ep_class->class;
303	308	ep_dev->dev.parent = parent;
...	...	@@ -309,9 +314,6 @@
309	314	retval = device_register(&ep_dev->dev);
310	315	if (retval)
311	316	goto error_chrdev;
312		- retval = sysfs_create_group(&ep_dev->dev.kobj, &ep_dev_attr_grp);
313		- if (retval)
314		- goto error_group;
315	317
316	318	/* create the symlink to the old-style "ep_XX" directory */
317	319	sprintf(name, "ep_%02x", endpoint->desc.bEndpointAddress);
...	...	@@ -322,8 +324,6 @@
322	324	return retval;
323	325
324	326	error_link:
325		- sysfs_remove_group(&ep_dev->dev.kobj, &ep_dev_attr_grp);
326		-error_group:
327	327	device_unregister(&ep_dev->dev);
328	328	destroy_endpoint_class();
329	329	return retval;
...	...	@@ -348,7 +348,6 @@
348	348
349	349	sprintf(name, "ep_%02x", endpoint->desc.bEndpointAddress);
350	350	sysfs_remove_link(&ep_dev->dev.parent->kobj, name);
351		- sysfs_remove_group(&ep_dev->dev.kobj, &ep_dev_attr_grp);
352	351	device_unregister(&ep_dev->dev);
353	352	endpoint->ep_dev = NULL;
354	353	destroy_endpoint_class();
...	...	@@ -1607,6 +1607,7 @@
1607	1607	intf->dev.driver = NULL;
1608	1608	intf->dev.bus = &usb_bus_type;
1609	1609	intf->dev.type = &usb_if_device_type;
	1610	+ intf->dev.groups = usb_interface_groups;
1610	1611	intf->dev.dma_mask = dev->dev.dma_mask;
1611	1612	device_initialize(&intf->dev);
1612	1613	mark_quiesced(intf);
...	...	@@ -538,6 +538,46 @@
538	538	.attrs = dev_attrs,
539	539	};
540	540
	541	+/* When modifying this list, be sure to modify dev_string_attrs_are_visible()
	542	+ * accordingly.
	543	+ */
	544	+static struct attribute *dev_string_attrs[] = {
	545	+ &dev_attr_manufacturer.attr,
	546	+ &dev_attr_product.attr,
	547	+ &dev_attr_serial.attr,
	548	+ NULL
	549	+};
	550	+
	551	+static mode_t dev_string_attrs_are_visible(struct kobject *kobj,
	552	+ struct attribute *a, int n)
	553	+{
	554	+ struct usb_device *udev = to_usb_device(
	555	+ container_of(kobj, struct device, kobj));
	556	+
	557	+ if (a == &dev_attr_manufacturer.attr) {
	558	+ if (udev->manufacturer == NULL)
	559	+ return 0;
	560	+ } else if (a == &dev_attr_product.attr) {
	561	+ if (udev->product == NULL)
	562	+ return 0;
	563	+ } else if (a == &dev_attr_serial.attr) {
	564	+ if (udev->serial == NULL)
	565	+ return 0;
	566	+ }
	567	+ return a->mode;
	568	+}
	569	+
	570	+static struct attribute_group dev_string_attr_grp = {
	571	+ .attrs = dev_string_attrs,
	572	+ .is_visible = dev_string_attrs_are_visible,
	573	+};
	574	+
	575	+struct attribute_group *usb_device_groups[] = {
	576	+ &dev_attr_grp,
	577	+ &dev_string_attr_grp,
	578	+ NULL
	579	+};
	580	+
541	581	/* Binary descriptors */
542	582
543	583	static ssize_t
...	...	@@ -591,10 +631,9 @@
591	631	struct device *dev = &udev->dev;
592	632	int retval;
593	633
594		- retval = sysfs_create_group(&dev->kobj, &dev_attr_grp);
595		- if (retval)
596		- return retval;
597		-
	634	+ /* Unforunately these attributes cannot be created before
	635	+ * the uevent is broadcast.
	636	+ */
598	637	retval = device_create_bin_file(dev, &dev_bin_attr_descriptors);
599	638	if (retval)
600	639	goto error;
...	...	@@ -607,21 +646,6 @@
607	646	if (retval)
608	647	goto error;
609	648
610		- if (udev->manufacturer) {
611		- retval = device_create_file(dev, &dev_attr_manufacturer);
612		- if (retval)
613		- goto error;
614		- }
615		- if (udev->product) {
616		- retval = device_create_file(dev, &dev_attr_product);
617		- if (retval)
618		- goto error;
619		- }
620		- if (udev->serial) {
621		- retval = device_create_file(dev, &dev_attr_serial);
622		- if (retval)
623		- goto error;
624		- }
625	649	retval = usb_create_ep_files(dev, &udev->ep0, udev);
626	650	if (retval)
627	651	goto error;
628	652
...	...	@@ -636,13 +660,9 @@
636	660	struct device *dev = &udev->dev;
637	661
638	662	usb_remove_ep_files(&udev->ep0);
639		- device_remove_file(dev, &dev_attr_manufacturer);
640		- device_remove_file(dev, &dev_attr_product);
641		- device_remove_file(dev, &dev_attr_serial);
642	663	remove_power_attributes(dev);
643	664	remove_persist_attributes(dev);
644	665	device_remove_bin_file(dev, &dev_bin_attr_descriptors);
645		- sysfs_remove_group(&dev->kobj, &dev_attr_grp);
646	666	}
647	667
648	668	/* Interface Accociation Descriptor fields */
649	669
650	670
...	...	@@ -688,17 +708,15 @@
688	708	struct device_attribute attr, char buf)
689	709	{
690	710	struct usb_interface *intf;
691		- struct usb_device *udev;
692		- int len;
	711	+ char *string;
693	712
694	713	intf = to_usb_interface(dev);
695		- udev = interface_to_usbdev(intf);
696		- len = snprintf(buf, 256, "%s", intf->cur_altsetting->string);
697		- if (len < 0)
	714	+ string = intf->cur_altsetting->string;
	715	+ barrier(); /* The altsetting might change! */
	716	+
	717	+ if (!string)
698	718	return 0;
699		- buf[len] = '\n';
700		- buf[len+1] = 0;
701		- return len+1;
	719	+ return sprintf(buf, "%s\n", string);
702	720	}
703	721	static DEVICE_ATTR(interface, S_IRUGO, show_interface_string, NULL);
704	722
...	...	@@ -727,18 +745,6 @@
727	745	}
728	746	static DEVICE_ATTR(modalias, S_IRUGO, show_modalias, NULL);
729	747
730		-static struct attribute *intf_assoc_attrs[] = {
731		- &dev_attr_iad_bFirstInterface.attr,
732		- &dev_attr_iad_bInterfaceCount.attr,
733		- &dev_attr_iad_bFunctionClass.attr,
734		- &dev_attr_iad_bFunctionSubClass.attr,
735		- &dev_attr_iad_bFunctionProtocol.attr,
736		- NULL,
737		-};
738		-static struct attribute_group intf_assoc_attr_grp = {
739		- .attrs = intf_assoc_attrs,
740		-};
741		-
742	748	static struct attribute *intf_attrs[] = {
743	749	&dev_attr_bInterfaceNumber.attr,
744	750	&dev_attr_bAlternateSetting.attr,
...	...	@@ -753,6 +759,37 @@
753	759	.attrs = intf_attrs,
754	760	};
755	761
	762	+static struct attribute *intf_assoc_attrs[] = {
	763	+ &dev_attr_iad_bFirstInterface.attr,
	764	+ &dev_attr_iad_bInterfaceCount.attr,
	765	+ &dev_attr_iad_bFunctionClass.attr,
	766	+ &dev_attr_iad_bFunctionSubClass.attr,
	767	+ &dev_attr_iad_bFunctionProtocol.attr,
	768	+ NULL,
	769	+};
	770	+
	771	+static mode_t intf_assoc_attrs_are_visible(struct kobject *kobj,
	772	+ struct attribute *a, int n)
	773	+{
	774	+ struct usb_interface *intf = to_usb_interface(
	775	+ container_of(kobj, struct device, kobj));
	776	+
	777	+ if (intf->intf_assoc == NULL)
	778	+ return 0;
	779	+ return a->mode;
	780	+}
	781	+
	782	+static struct attribute_group intf_assoc_attr_grp = {
	783	+ .attrs = intf_assoc_attrs,
	784	+ .is_visible = intf_assoc_attrs_are_visible,
	785	+};
	786	+
	787	+struct attribute_group *usb_interface_groups[] = {
	788	+ &intf_attr_grp,
	789	+ &intf_assoc_attr_grp,
	790	+ NULL
	791	+};
	792	+
756	793	static inline void usb_create_intf_ep_files(struct usb_interface *intf,
757	794	struct usb_device *udev)
758	795	{
759	796
760	797
761	798
...	...	@@ -777,23 +814,21 @@
777	814
778	815	int usb_create_sysfs_intf_files(struct usb_interface *intf)
779	816	{
780		- struct device *dev = &intf->dev;
781	817	struct usb_device *udev = interface_to_usbdev(intf);
782	818	struct usb_host_interface *alt = intf->cur_altsetting;
783	819	int retval;
784	820
785	821	if (intf->sysfs_files_created)
786	822	return 0;
787		- retval = sysfs_create_group(&dev->kobj, &intf_attr_grp);
788		- if (retval)
789		- return retval;
790	823
	824	+ /* The interface string may be present in some altsettings
	825	+ * and missing in others. Hence its attribute cannot be created
	826	+ * before the uevent is broadcast.
	827	+ */
791	828	if (alt->string == NULL)
792	829	alt->string = usb_cache_string(udev, alt->desc.iInterface);
793	830	if (alt->string)
794		- retval = device_create_file(dev, &dev_attr_interface);
795		- if (intf->intf_assoc)
796		- retval = sysfs_create_group(&dev->kobj, &intf_assoc_attr_grp);
	831	+ retval = device_create_file(&intf->dev, &dev_attr_interface);
797	832	usb_create_intf_ep_files(intf, udev);
798	833	intf->sysfs_files_created = 1;
799	834	return 0;
...	...	@@ -807,8 +842,6 @@
807	842	return;
808	843	usb_remove_intf_ep_files(intf);
809	844	device_remove_file(dev, &dev_attr_interface);
810		- sysfs_remove_group(&dev->kobj, &intf_attr_grp);
811		- sysfs_remove_group(&intf->dev.kobj, &intf_assoc_attr_grp);
812	845	intf->sysfs_files_created = 0;
813	846	}
...	...	@@ -291,6 +291,7 @@
291	291	device_initialize(&dev->dev);
292	292	dev->dev.bus = &usb_bus_type;
293	293	dev->dev.type = &usb_device_type;
	294	+ dev->dev.groups = usb_device_groups;
294	295	dev->dev.dma_mask = bus->controller->dma_mask;
295	296	set_dev_node(&dev->dev, dev_to_node(bus->controller));
296	297	dev->state = USB_STATE_ATTACHED;
...	...	@@ -130,6 +130,10 @@
130	130	/* for labeling diagnostics */
131	131	extern const char *usbcore_name;
132	132
	133	+/* sysfs stuff */
	134	+extern struct attribute_group *usb_device_groups[];
	135	+extern struct attribute_group *usb_interface_groups[];
	136	+
133	137	/* usbfs stuff */
134	138	extern struct mutex usbfs_mutex;
135	139	extern struct usb_driver usbfs_driver;