Skip to contentSkip to conversation

driver core fixes against 2.6.26-rc3

PATCH New Greg KH 1 participant 14 messages May 20, 2008

Subsystems (13)
Memory management SCSI USB Sound +9 more subsystems
lore
Share & tools
lore

Greg KH

author
#0 lore raw

PATCH 1/1 driver core fixes against 2.6.26-rc3

Here are some patches against your 2.6.26-rc3-git tree.

They fix a race condition when device_create is called. At that point in time sysfs files can be created automatically by the class or the driver subsystem, and if those files are opened before the device specific data is set, oopses can happen. This was found and reported for the bdi subsystem by Arthur Jones and he verified that these patches fix the problem.

I then went and audited all users of the api and found other places where this was also a problem. This patch series fixes all of those instances.

The API will be chaned in 2.6.27 so this can not happen in the future by making device_create() look exactly like device_create_drvdata() but to do that now would require a few hundred other changes through the tree. I'll leave that for .27 and have patches queued up right now in -next for this so it gets good testing.

These patches have been in -mm and -next.

Please pull from:
master.kernel.org:/pub/scm/linux/kernel/git/gregkh/driver-2.6.git/

The patchs will be sent as a follow-on to this message to lkml for people to see.

thanks,

greg k-h

------------

 drivers/base/core.c                    |   85 +++++++++++++++++++++++++++++--
 drivers/ide/ide-probe.c                |    5 +-
 drivers/infiniband/core/user_mad.c     |   14 ++---
 drivers/infiniband/core/uverbs_main.c  |   11 ++--
 drivers/leds/led-class.c               |    6 +--
 drivers/power/power_supply_core.c      |    6 +--
 drivers/s390/char/vmlogrdr.c           |    9 ++--
 drivers/scsi/ch.c                      |    7 +--
 drivers/scsi/osst.c                    |    3 +-
 drivers/scsi/sg.c                      |   11 ++--
 drivers/scsi/st.c                      |   12 +++--
 drivers/uio/uio.c                      |    7 +--
 drivers/usb/core/hcd.c                 |    6 +-
 drivers/usb/misc/phidgetkit.c          |    6 +-
 drivers/usb/misc/phidgetmotorcontrol.c |    7 +--
 drivers/usb/misc/phidgetservo.c        |    6 +-
 drivers/video/display/display-sysfs.c  |   10 +++-
 include/linux/device.h                 |   12 +++++
 mm/backing-dev.c                       |   12 +----
 sound/core/sound.c                     |    8 +--
 20 files changed, 157 insertions(+), 86 deletions(-)

---------------

Greg Kroah-Hartman (13):
Driver core: add device_create_vargs and device_create_drvdata
mm: bdi: fix race in bdi_class device creation
fbdev: fix race in device_create
ide: fix race in device_create
IB: fix race in device_create
LEDS: fix race in device_create
Power Supply: fix race in device_create
UIO: fix race in device_create
SOUND: fix race in device_create
s390: fix race in device_create
USB: Phidget: fix race in device_create
USB: Core: fix race in device_create
SCSI: fix race in device_create

#1 lore raw

PATCH 1/13 Driver core: add device_create_vargs and device_create_drvdata

We want to have the drvdata field set properly when creating the device as sysfs callbacks can assume it is present and it can race the later setting of this field.

So, create two new functions, deviec_create_vargs() and device_create_drvdata() that take this new field.

device_create_drvdata() will go away in 2.6.27 as the drvdata field will just be moved to the device_create() call as it should be.

---
 drivers/base/core.c    |   85 ++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/device.h |   12 +++++++
 2 files changed, 91 insertions(+), 6 deletions(-)
drivers/base/core.c +79 -6
index be288b5..f861c2b 100644
@@ -1084,11 +1084,13 @@ static void device_create_release(struct device *dev)
 }
 
 /**
- * device_create - creates a device and registers it with sysfs
+ * device_create_vargs - creates a device and registers it with sysfs
  * @class: pointer to the struct class that this device should be registered to
  * @parent: pointer to the parent struct device of this new device, if any
  * @devt: the dev_t for the char device to be added
+ * @drvdata: the data to be added to the device for callbacks
  * @fmt: string for the device's name
+ * @args: va_list for the device's name
  *
  * This function can be used by char device classes.  A struct device
  * will be created in sysfs, registered to the specified class.
@@ -1104,10 +1106,10 @@ static void device_create_release(struct device *dev)
  * Note: the struct class passed to this function must have previously
  * been created with a call to class_create().
  */
-struct device *device_create(struct class *class, struct device *parent,
-			     dev_t devt, const char *fmt, ...)
+struct device *device_create_vargs(struct class *class, struct device *parent,
+				   dev_t devt, void *drvdata, const char *fmt,
+				   va_list args)
 {
-	va_list args;
 	struct device *dev = NULL;
 	int retval = -ENODEV;
 
@@ -1124,10 +1126,9 @@ struct device *device_create(struct class *class, struct device *parent,
 	dev->class = class;
 	dev->parent = parent;
 	dev->release = device_create_release;
+	dev_set_drvdata(dev, drvdata);
 
-	va_start(args, fmt);
 	vsnprintf(dev->bus_id, BUS_ID_SIZE, fmt, args);
-	va_end(args);
 	retval = device_register(dev);
 	if (retval)
 		goto error;
@@ -1138,6 +1139,78 @@ error:
 	kfree(dev);
 	return ERR_PTR(retval);
 }
+EXPORT_SYMBOL_GPL(device_create_vargs);
+
+/**
+ * device_create_drvdata - creates a device and registers it with sysfs
+ * @class: pointer to the struct class that this device should be registered to
+ * @parent: pointer to the parent struct device of this new device, if any
+ * @devt: the dev_t for the char device to be added
+ * @drvdata: the data to be added to the device for callbacks
+ * @fmt: string for the device's name
+ *
+ * This function can be used by char device classes.  A struct device
+ * will be created in sysfs, registered to the specified class.
+ *
+ * A "dev" file will be created, showing the dev_t for the device, if
+ * the dev_t is not 0,0.
+ * If a pointer to a parent struct device is passed in, the newly created
+ * struct device will be a child of that device in sysfs.
+ * The pointer to the struct device will be returned from the call.
+ * Any further sysfs files that might be required can be created using this
+ * pointer.
+ *
+ * Note: the struct class passed to this function must have previously
+ * been created with a call to class_create().
+ */
+struct device *device_create_drvdata(struct class *class,
+				     struct device *parent,
+				     dev_t devt,
+				     void *drvdata,
+				     const char *fmt, ...)
+{
+	va_list vargs;
+	struct device *dev;
+
+	va_start(vargs, fmt);
+	dev = device_create_vargs(class, parent, devt, drvdata, fmt, vargs);
+	va_end(vargs);
+	return dev;
+}
+EXPORT_SYMBOL_GPL(device_create_drvdata);
+
+/**
+ * device_create - creates a device and registers it with sysfs
+ * @class: pointer to the struct class that this device should be registered to
+ * @parent: pointer to the parent struct device of this new device, if any
+ * @devt: the dev_t for the char device to be added
+ * @fmt: string for the device's name
+ *
+ * This function can be used by char device classes.  A struct device
+ * will be created in sysfs, registered to the specified class.
+ *
+ * A "dev" file will be created, showing the dev_t for the device, if
+ * the dev_t is not 0,0.
+ * If a pointer to a parent struct device is passed in, the newly created
+ * struct device will be a child of that device in sysfs.
+ * The pointer to the struct device will be returned from the call.
+ * Any further sysfs files that might be required can be created using this
+ * pointer.
+ *
+ * Note: the struct class passed to this function must have previously
+ * been created with a call to class_create().
+ */
+struct device *device_create(struct class *class, struct device *parent,
+			     dev_t devt, const char *fmt, ...)
+{
+	va_list vargs;
+	struct device *dev;
+
+	va_start(vargs, fmt);
+	dev = device_create_vargs(class, parent, devt, NULL, fmt, vargs);
+	va_end(vargs);
+	return dev;
+}
 EXPORT_SYMBOL_GPL(device_create);
 
 static int __match_devt(struct device *dev, void *data)
include/linux/device.h +12 -0
index 15e9fa3..14616e8 100644
@@ -449,9 +449,21 @@ extern int __must_check device_reprobe(struct device *dev);
 /*
  * Easy functions for dynamically creating devices on the fly
  */
+extern struct device *device_create_vargs(struct class *cls,
+					  struct device *parent,
+					  dev_t devt,
+					  void *drvdata,
+					  const char *fmt,
+					  va_list vargs);
 extern struct device *device_create(struct class *cls, struct device *parent,
 				    dev_t devt, const char *fmt, ...)
 				    __attribute__((format(printf, 4, 5)));
+extern struct device *device_create_drvdata(struct class *cls,
+					    struct device *parent,
+					    dev_t devt,
+					    void *drvdata,
+					    const char *fmt, ...)
+				    __attribute__((format(printf, 5, 6)));
 extern void device_destroy(struct class *cls, dev_t devt);
 
 /*
Signature · 1 line
1.5.5.1
#2 lore raw

PATCH 2/13 mm: bdi: fix race in bdi_class device creation

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_vargs().

Many thanks to Arthur Jones <[email protected]> for reporting the bug, and testing patches out.

Cc Kay Sievers <[email protected]>
Cc Arthur Jones <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Miklos Szeredi <[email protected]>
---
 mm/backing-dev.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)
mm/backing-dev.c +2 -10
index 7c4f9e0..f2e574d 100644
@@ -172,30 +172,22 @@ postcore_initcall(bdi_class_init);
 int bdi_register(struct backing_dev_info *bdi, struct device *parent,
 		const char *fmt, ...)
 {
-	char *name;
 	va_list args;
 	int ret = 0;
 	struct device *dev;
 
 	va_start(args, fmt);
-	name = kvasprintf(GFP_KERNEL, fmt, args);
+	dev = device_create_vargs(bdi_class, parent, MKDEV(0, 0), bdi, fmt, args);
 	va_end(args);
-
-	if (!name)
-		return -ENOMEM;
-
-	dev = device_create(bdi_class, parent, MKDEV(0, 0), name);
 	if (IS_ERR(dev)) {
 		ret = PTR_ERR(dev);
 		goto exit;
 	}
 
 	bdi->dev = dev;
-	dev_set_drvdata(bdi->dev, bdi);
-	bdi_debug_register(bdi, name);
+	bdi_debug_register(bdi, dev_name(dev));
 
 exit:
-	kfree(name);
 	return ret;
 }
 EXPORT_SYMBOL(bdi_register);
Signature · 1 line
1.5.5.1
#3 lore raw

PATCH 3/13 fbdev: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

Cc Kay Sievers <[email protected]>
Cc James Simmons <[email protected]>
---
 drivers/video/display/display-sysfs.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
drivers/video/display/display-sysfs.c +7 -3
index 3547717..6ef800b 100644
@@ -26,6 +26,7 @@
 #include <linux/ctype.h>
 #include <linux/idr.h>
 #include <linux/err.h>
+#include <linux/kdev_t.h>
 
 static ssize_t display_show_name(struct device *dev,
 				struct device_attribute *attr, char *buf)
@@ -152,10 +153,13 @@ struct display_device *display_device_register(struct display_driver *driver,
 		mutex_unlock(&allocated_dsp_lock);
 
 		if (!ret) {
-			new_dev->dev = device_create(display_class, parent, 0,
-						"display%d", new_dev->idx);
+			new_dev->dev = device_create_drvdata(display_class,
+							     parent,
+							     MKDEV(0,0),
+							     new_dev,
+							     "display%d",
+							     new_dev->idx);
 			if (!IS_ERR(new_dev->dev)) {
-				dev_set_drvdata(new_dev->dev, new_dev);
 				new_dev->parent = parent;
 				new_dev->driver = driver;
 				mutex_init(&new_dev->lock);
Signature · 1 line
1.5.5.1
#4 lore raw

PATCH 4/13 ide: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

---
 drivers/ide/ide-probe.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
drivers/ide/ide-probe.c +2 -3
index 34b0d4f..655ec7e 100644
@@ -648,13 +648,12 @@ static int ide_register_port(ide_hwif_t *hwif)
 
 	get_device(&hwif->gendev);
 
-	hwif->portdev = device_create(ide_port_class, &hwif->gendev,
-				      MKDEV(0, 0), hwif->name);
+	hwif->portdev = device_create_drvdata(ide_port_class, &hwif->gendev,
+					      MKDEV(0, 0), hwif, hwif->name);
 	if (IS_ERR(hwif->portdev)) {
 		ret = PTR_ERR(hwif->portdev);
 		device_unregister(&hwif->gendev);
 	}
-	dev_set_drvdata(hwif->portdev, hwif);
 out:
 	return ret;
 }
Signature · 1 line
1.5.5.1
#5 lore raw

PATCH 5/13 IB: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

---
 drivers/infiniband/core/user_mad.c    |   14 ++++++--------
 drivers/infiniband/core/uverbs_main.c |   11 ++++++-----
 2 files changed, 12 insertions(+), 13 deletions(-)
drivers/infiniband/core/user_mad.c +6 -8
index 3aa2db5..840ede9 100644
@@ -1005,8 +1005,9 @@ static int ib_umad_init_port(struct ib_device *device, int port_num,
 	if (cdev_add(port->cdev, base_dev + port->dev_num, 1))
 		goto err_cdev;
 
-	port->dev = device_create(umad_class, device->dma_device,
-				  port->cdev->dev, "umad%d", port->dev_num);
+	port->dev = device_create_drvdata(umad_class, device->dma_device,
+					  port->cdev->dev, port,
+					  "umad%d", port->dev_num);
 	if (IS_ERR(port->dev))
 		goto err_cdev;
 
@@ -1024,15 +1025,12 @@ static int ib_umad_init_port(struct ib_device *device, int port_num,
 	if (cdev_add(port->sm_cdev, base_dev + port->dev_num + IB_UMAD_MAX_PORTS, 1))
 		goto err_sm_cdev;
 
-	port->sm_dev = device_create(umad_class, device->dma_device,
-				     port->sm_cdev->dev,
-				     "issm%d", port->dev_num);
+	port->sm_dev = device_create_drvdata(umad_class, device->dma_device,
+					     port->sm_cdev->dev, port,
+					     "issm%d", port->dev_num);
 	if (IS_ERR(port->sm_dev))
 		goto err_sm_cdev;
 
-	dev_set_drvdata(port->dev,    port);
-	dev_set_drvdata(port->sm_dev, port);
-
 	if (device_create_file(port->sm_dev, &dev_attr_ibdev))
 		goto err_sm_dev;
 	if (device_create_file(port->sm_dev, &dev_attr_port))
drivers/infiniband/core/uverbs_main.c +6 -5
index cc1afa2..f806da1 100644
@@ -755,14 +755,15 @@ static void ib_uverbs_add_one(struct ib_device *device)
 	if (cdev_add(uverbs_dev->cdev, IB_UVERBS_BASE_DEV + uverbs_dev->devnum, 1))
 		goto err_cdev;
 
-	uverbs_dev->dev = device_create(uverbs_class, device->dma_device,
-					uverbs_dev->cdev->dev,
-					"uverbs%d", uverbs_dev->devnum);
+	uverbs_dev->dev = device_create_drvdata(uverbs_class,
+						device->dma_device,
+						uverbs_dev->cdev->dev,
+						uverbs_dev,
+						"uverbs%d",
+						uverbs_dev->devnum);
 	if (IS_ERR(uverbs_dev->dev))
 		goto err_cdev;
 
-	dev_set_drvdata(uverbs_dev->dev, uverbs_dev);
-
 	if (device_create_file(uverbs_dev->dev, &dev_attr_ibdev))
 		goto err_class;
 	if (device_create_file(uverbs_dev->dev, &dev_attr_abi_version))
Signature · 1 line
1.5.5.1
#6 lore raw

PATCH 6/13 LEDS: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

Cc Kay Sievers <[email protected]>
Cc Richard Purdie <[email protected]>
---
 drivers/leds/led-class.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/leds/led-class.c +2 -4
index b3c54be..559a408 100644
@@ -103,13 +103,11 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 {
 	int rc;
 
-	led_cdev->dev = device_create(leds_class, parent, 0, "%s",
-					    led_cdev->name);
+	led_cdev->dev = device_create_drvdata(leds_class, parent, 0, led_cdev,
+					      "%s", led_cdev->name);
 	if (IS_ERR(led_cdev->dev))
 		return PTR_ERR(led_cdev->dev);
 
-	dev_set_drvdata(led_cdev->dev, led_cdev);
-
 	/* register the attributes */
 	rc = device_create_file(led_cdev->dev, &dev_attr_brightness);
 	if (rc)
Signature · 1 line
1.5.5.1
#7 lore raw

PATCH 7/13 Power Supply: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

Cc Kay Sievers <[email protected]>
Cc Anton Vorontsov <[email protected]>
Cc David Woodhouse <[email protected]>
---
 drivers/power/power_supply_core.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/power/power_supply_core.c +2 -4
index 138dd76..af1633e 100644
@@ -91,15 +91,13 @@ int power_supply_register(struct device *parent, struct power_supply *psy)
 {
 	int rc = 0;
 
-	psy->dev = device_create(power_supply_class, parent, 0,
-				 "%s", psy->name);
+	psy->dev = device_create_drvdata(power_supply_class, parent, 0,
+					 psy, "%s", psy->name);
 	if (IS_ERR(psy->dev)) {
 		rc = PTR_ERR(psy->dev);
 		goto dev_create_failed;
 	}
 
-	dev_set_drvdata(psy->dev, psy);
-
 	INIT_WORK(&psy->changed_work, power_supply_changed_work);
 
 	rc = power_supply_create_attrs(psy);
Signature · 1 line
1.5.5.1
#8 lore raw

PATCH 8/13 UIO: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

---
 drivers/uio/uio.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
drivers/uio/uio.c +3 -4
index 55cc7b8..0a12e90 100644
@@ -649,15 +649,14 @@ int __uio_register_device(struct module *owner,
 	if (ret)
 		goto err_get_minor;
 
-	idev->dev = device_create(uio_class->class, parent,
-				  MKDEV(uio_major, idev->minor),
-				  "uio%d", idev->minor);
+	idev->dev = device_create_drvdata(uio_class->class, parent,
+					  MKDEV(uio_major, idev->minor), idev,
+					  "uio%d", idev->minor);
 	if (IS_ERR(idev->dev)) {
 		printk(KERN_ERR "UIO: device register failed\n");
 		ret = PTR_ERR(idev->dev);
 		goto err_device_create;
 	}
-	dev_set_drvdata(idev->dev, idev);
 
 	ret = uio_dev_add_attributes(idev);
 	if (ret)
Signature · 1 line
1.5.5.1
#9 lore raw

PATCH 9/13 SOUND: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

Cc Kay Sievers <[email protected]>
Cc Jaroslav Kysela <[email protected]>
---
 sound/core/sound.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
sound/core/sound.c +3 -5
index 812f91b..6c8ab48 100644
@@ -259,8 +259,9 @@ int snd_register_device_for_dev(int type, struct snd_card *card, int dev,
 		return minor;
 	}
 	snd_minors[minor] = preg;
-	preg->dev = device_create(sound_class, device, MKDEV(major, minor),
-				  "%s", name);
+	preg->dev = device_create_drvdata(sound_class, device,
+					  MKDEV(major, minor),
+					  private_data, "%s", name);
 	if (IS_ERR(preg->dev)) {
 		snd_minors[minor] = NULL;
 		mutex_unlock(&sound_mutex);
@@ -269,9 +270,6 @@ int snd_register_device_for_dev(int type, struct snd_card *card, int dev,
 		return minor;
 	}
 
-	if (preg->dev)
-		dev_set_drvdata(preg->dev, private_data);
-
 	mutex_unlock(&sound_mutex);
 	return 0;
 }
Signature · 1 line
1.5.5.1
#10 lore raw

PATCH 10/13 s390: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

Cc Kay Sievers <[email protected]>
Cc Martin Schwidefsky <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Cornelia Huck <[email protected]>
---
 drivers/s390/char/vmlogrdr.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)
drivers/s390/char/vmlogrdr.c +4 -5
index e848734..2c2428c 100644
@@ -762,10 +762,10 @@ static int vmlogrdr_register_device(struct vmlogrdr_priv_t *priv)
 		device_unregister(dev);
 		return ret;
 	}
-	priv->class_device = device_create(vmlogrdr_class, dev,
-					   MKDEV(vmlogrdr_major,
-						 priv->minor_num),
-					   "%s", dev->bus_id);
+	priv->class_device = device_create_drvdata(vmlogrdr_class, dev,
+						   MKDEV(vmlogrdr_major,
+							 priv->minor_num),
+						   priv, "%s", dev->bus_id);
 	if (IS_ERR(priv->class_device)) {
 		ret = PTR_ERR(priv->class_device);
 		priv->class_device=NULL;
@@ -773,7 +773,6 @@ static int vmlogrdr_register_device(struct vmlogrdr_priv_t *priv)
 		device_unregister(dev);
 		return ret;
 	}
-	dev->driver_data = priv;
 	priv->device = dev;
 	return 0;
 }
Signature · 1 line
1.5.5.1
#11 lore raw

PATCH 11/13 USB: Phidget: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata(). It fixes all 3 phidget drivers, which all have the same problem.

---
 drivers/usb/misc/phidgetkit.c          |    6 +++---
 drivers/usb/misc/phidgetmotorcontrol.c |    7 +++----
 drivers/usb/misc/phidgetservo.c        |    6 +++---
 3 files changed, 9 insertions(+), 10 deletions(-)
drivers/usb/misc/phidgetkit.c +3 -3
index 24230c6..4cfa25b 100644
@@ -595,14 +595,14 @@ static int interfacekit_probe(struct usb_interface *intf, const struct usb_devic
         } while(value);
         kit->dev_no = bit;
 
-        kit->dev = device_create(phidget_class, &kit->udev->dev, 0,
-               		"interfacekit%d", kit->dev_no);
+	kit->dev = device_create_drvdata(phidget_class, &kit->udev->dev,
+					MKDEV(0, 0), kit,
+					"interfacekit%d", kit->dev_no);
         if (IS_ERR(kit->dev)) {
                 rc = PTR_ERR(kit->dev);
                 kit->dev = NULL;
                 goto out;
         }
-	dev_set_drvdata(kit->dev, kit);
 
 	if (usb_submit_urb(kit->irq, GFP_KERNEL)) {
 		rc = -EIO;
drivers/usb/misc/phidgetmotorcontrol.c +3 -4
index f0113c1..9b4696f 100644
@@ -365,16 +365,15 @@ static int motorcontrol_probe(struct usb_interface *intf, const struct usb_devic
 	} while(value);
 	mc->dev_no = bit;
 
-	mc->dev = device_create(phidget_class, &mc->udev->dev, 0,
-				"motorcontrol%d", mc->dev_no);
+	mc->dev = device_create_drvdata(phidget_class, &mc->udev->dev,
+					MKDEV(0, 0), mc,
+					"motorcontrol%d", mc->dev_no);
 	if (IS_ERR(mc->dev)) {
 		rc = PTR_ERR(mc->dev);
 		mc->dev = NULL;
 		goto out;
 	}
 
-	dev_set_drvdata(mc->dev, mc);
-
 	if (usb_submit_urb(mc->irq, GFP_KERNEL)) {
 		rc = -EIO;
 		goto out;
drivers/usb/misc/phidgetservo.c +3 -3
index 7d590c0..1ca7ddb 100644
@@ -275,14 +275,14 @@ servo_probe(struct usb_interface *interface, const struct usb_device_id *id)
         } while (value);
 	dev->dev_no = bit;
 
-	dev->dev = device_create(phidget_class, &dev->udev->dev, 0,
-				 "servo%d", dev->dev_no);
+	dev->dev = device_create_drvdata(phidget_class, &dev->udev->dev,
+					 MKDEV(0, 0), dev,
+					 "servo%d", dev->dev_no);
 	if (IS_ERR(dev->dev)) {
 		rc = PTR_ERR(dev->dev);
 		dev->dev = NULL;
 		goto out;
 	}
-	dev_set_drvdata(dev->dev, dev);
 
 	servo_count = dev->type & SERVO_COUNT_QUAD ? 4 : 1;
 
Signature · 1 line
1.5.5.1
#12 lore raw

PATCH 12/13 USB: Core: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata().

---
 drivers/usb/core/hcd.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/usb/core/hcd.c +3 -3
index bf10e9c..09a53e7 100644
@@ -818,12 +818,12 @@ static int usb_register_bus(struct usb_bus *bus)
 	set_bit (busnum, busmap.busmap);
 	bus->busnum = busnum;
 
-	bus->dev = device_create(usb_host_class, bus->controller, MKDEV(0, 0),
-				 "usb_host%d", busnum);
+	bus->dev = device_create_drvdata(usb_host_class, bus->controller,
+					 MKDEV(0, 0), bus,
+					 "usb_host%d", busnum);
 	result = PTR_ERR(bus->dev);
 	if (IS_ERR(bus->dev))
 		goto error_create_class_dev;
-	dev_set_drvdata(bus->dev, bus);
 
 	/* Add it to the local list of buses */
 	list_add (&bus->bus_list, &usb_bus_list);
Signature · 1 line
1.5.5.1
#13 lore raw

PATCH 13/13 SCSI: fix race in device_create

In reply to Greg KH

There is a race from when a device is created with device_create() and then the drvdata is set with a call to dev_set_drvdata() in which a sysfs file could be open, yet the drvdata will be NULL, causing all sorts of bad things to happen.

This patch fixes the problem by using the new function, device_create_drvdata(). It fixes the problem in all of the scsi drivers that need it.

Cc Kay Sievers <[email protected]>
Cc Doug Gilbert <[email protected]>
Cc James E.J. Bottomley <[email protected]>
---
 drivers/scsi/ch.c   |    7 +++----
 drivers/scsi/osst.c |    3 +--
 drivers/scsi/sg.c   |   11 ++++++-----
 drivers/scsi/st.c   |   12 +++++++-----
 4 files changed, 17 insertions(+), 16 deletions(-)
drivers/scsi/ch.c +3 -4
index 75c84d7..c4b938b 100644
@@ -910,9 +910,9 @@ static int ch_probe(struct device *dev)
 	ch->minor = minor;
 	sprintf(ch->name,"ch%d",ch->minor);
 
-	class_dev = device_create(ch_sysfs_class, dev,
-				  MKDEV(SCSI_CHANGER_MAJOR,ch->minor),
-				  "s%s", ch->name);
+	class_dev = device_create_drvdata(ch_sysfs_class, dev,
+					  MKDEV(SCSI_CHANGER_MAJOR, ch->minor),
+					  ch, "s%s", ch->name);
 	if (IS_ERR(class_dev)) {
 		printk(KERN_WARNING "ch%d: device_create failed\n",
 		       ch->minor);
@@ -926,7 +926,6 @@ static int ch_probe(struct device *dev)
 	if (init)
 		ch_init_elem(ch);
 
-	dev_set_drvdata(dev, ch);
 	sdev_printk(KERN_INFO, sd, "Attached scsi changer %s\n", ch->name);
 
 	return 0;
drivers/scsi/osst.c +1 -2
index 31f7aec..243d8be 100644
@@ -5695,13 +5695,12 @@ static int osst_sysfs_add(dev_t dev, struct device *device, struct osst_tape * S
 	struct device *osst_member;
 	int err;
 
-	osst_member = device_create(osst_sysfs_class, device, dev, "%s", name);
+	osst_member = device_create_drvdata(osst_sysfs_class, device, dev, STp, "%s", name);
 	if (IS_ERR(osst_member)) {
 		printk(KERN_WARNING "osst :W: Unable to add sysfs class member %s\n", name);
 		return PTR_ERR(osst_member);
 	}
 
-	dev_set_drvdata(osst_member, STp);
 	err = device_create_file(osst_member, &dev_attr_ADR_rev);
 	if (err)
 		goto err_out;
drivers/scsi/sg.c +6 -5
index c9d7f72..ea0edd1 100644
@@ -1441,17 +1441,18 @@ sg_add(struct device *cl_dev, struct class_interface *cl_intf)
 	if (sg_sysfs_valid) {
 		struct device *sg_class_member;
 
-		sg_class_member = device_create(sg_sysfs_class, cl_dev->parent,
-						MKDEV(SCSI_GENERIC_MAJOR,
-						      sdp->index),
-						"%s", disk->disk_name);
+		sg_class_member = device_create_drvdata(sg_sysfs_class,
+							cl_dev->parent,
+							MKDEV(SCSI_GENERIC_MAJOR,
+							      sdp->index),
+							sdp,
+							"%s", disk->disk_name);
 		if (IS_ERR(sg_class_member)) {
 			printk(KERN_ERR "sg_add: "
 			       "device_create failed\n");
 			error = PTR_ERR(sg_class_member);
 			goto cdev_add_err;
 		}
-		dev_set_drvdata(sg_class_member, sdp);
 		error = sysfs_create_link(&scsidp->sdev_gendev.kobj,
 					  &sg_class_member->kobj, "generic");
 		if (error)
drivers/scsi/st.c +7 -5
index e8db66a..6e5a5bb 100644
@@ -4424,17 +4424,19 @@ static int do_create_class_files(struct scsi_tape *STp, int dev_num, int mode)
 		snprintf(name, 10, "%s%s%s", rew ? "n" : "",
 			 STp->disk->disk_name, st_formats[i]);
 		st_class_member =
-			device_create(st_sysfs_class, &STp->device->sdev_gendev,
-				      MKDEV(SCSI_TAPE_MAJOR,
-						TAPE_MINOR(dev_num, mode, rew)),
-				      "%s", name);
+			device_create_drvdata(st_sysfs_class,
+					      &STp->device->sdev_gendev,
+					      MKDEV(SCSI_TAPE_MAJOR,
+						    TAPE_MINOR(dev_num,
+							      mode, rew)),
+					      &STp->modes[mode],
+					      "%s", name);
 		if (IS_ERR(st_class_member)) {
 			printk(KERN_WARNING "st%d: device_create failed\n",
 			       dev_num);
 			error = PTR_ERR(st_class_member);
 			goto out;
 		}
-		dev_set_drvdata(st_class_member, &STp->modes[mode]);
 
 		error = device_create_file(st_class_member,
 					   &dev_attr_defined);
Signature · 1 line
1.5.5.1