From: Manuel Estrada Sainz <ranty@ranty.pantax.net>
- Remove races related to the handling and release of 'struct firmware'
---
25-akpm/drivers/base/firmware_class.c | 100 ++++++++++++++++++++++------------
1 files changed, 66 insertions(+), 34 deletions(-)
diff -puN drivers/base/firmware_class.c~request_firmware-05-release-race-fixes drivers/base/firmware_class.c
--- 25/drivers/base/firmware_class.c~request_firmware-05-release-race-fixes Tue Feb 24 17:55:52 2004
+++ 25-akpm/drivers/base/firmware_class.c Tue Feb 24 17:55:52 2004
@@ -14,6 +14,7 @@
#include <linux/vmalloc.h>
#include <asm/hardirq.h>
#include <linux/bitops.h>
+#include <asm/semaphore.h>
#include <linux/firmware.h>
#include "base.h"
@@ -24,11 +25,16 @@ MODULE_LICENSE("GPL");
enum {
FW_STATUS_LOADING,
+ FW_STATUS_DONE,
FW_STATUS_ABORT,
};
static int loading_timeout = 10; /* In seconds */
+/* fw_lock could be moved to 'struct firmware_priv' but since it is just
+ * guarding for corner cases a global lock should be OK */
+static DECLARE_MUTEX(fw_lock);
+
struct firmware_priv {
char fw_id[FIRMWARE_NAME_MAX];
struct completion completion;
@@ -126,11 +132,13 @@ firmware_loading_store(struct class_devi
switch (loading) {
case 1:
+ down(&fw_lock);
vfree(fw_priv->fw->data);
fw_priv->fw->data = NULL;
fw_priv->fw->size = 0;
fw_priv->alloc_size = 0;
set_bit(FW_STATUS_LOADING, &fw_priv->status);
+ up(&fw_lock);
break;
case 0:
if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) {
@@ -160,15 +168,26 @@ firmware_data_read(struct kobject *kobj,
{
struct class_device *class_dev = to_class_dev(kobj);
struct firmware_priv *fw_priv = class_get_devdata(class_dev);
- struct firmware *fw = fw_priv->fw;
+ struct firmware *fw;
+ ssize_t ret_count = count;
- if (offset > fw->size)
- return 0;
- if (offset + count > fw->size)
- count = fw->size - offset;
+ down(&fw_lock);
+ fw = fw_priv->fw;
+ if (test_bit(FW_STATUS_DONE, &fw_priv->status)) {
+ ret_count = -ENODEV;
+ goto out;
+ }
+ if (offset > fw->size) {
+ ret_count = 0;
+ goto out;
+ }
+ if (offset + ret_count > fw->size)
+ ret_count = fw->size - offset;
- memcpy(buffer, fw->data + offset, count);
- return count;
+ memcpy(buffer, fw->data + offset, ret_count);
+out:
+ up(&fw_lock);
+ return ret_count;
}
static int
fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size)
@@ -209,18 +228,26 @@ firmware_data_write(struct kobject *kobj
{
struct class_device *class_dev = to_class_dev(kobj);
struct firmware_priv *fw_priv = class_get_devdata(class_dev);
- struct firmware *fw = fw_priv->fw;
- int retval;
+ struct firmware *fw;
+ ssize_t retval;
+ down(&fw_lock);
+ fw = fw_priv->fw;
+ if (test_bit(FW_STATUS_DONE, &fw_priv->status)) {
+ retval = -ENODEV;
+ goto out;
+ }
retval = fw_realloc_buffer(fw_priv, offset + count);
if (retval)
- return retval;
+ goto out;
memcpy(fw->data + offset, buffer, count);
fw->size = max_t(size_t, offset + count, fw->size);
-
- return count;
+ retval = count;
+out:
+ up(&fw_lock);
+ return retval;
}
static struct bin_attribute firmware_attr_data_tmpl = {
.attr = {.name = "data", .mode = 0644},
@@ -252,7 +279,7 @@ fw_setup_class_device_id(struct class_de
strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
}
static int
-fw_setup_class_device(struct class_device **class_dev_p,
+fw_setup_class_device(struct firmware *fw, struct class_device **class_dev_p,
const char *fw_name, struct device *device)
{
int retval = 0;
@@ -290,6 +317,8 @@ fw_setup_class_device(struct class_devic
goto error_kfree;
}
+ fw_priv->fw = fw;
+
retval = sysfs_create_bin_file(&class_dev->kobj, &fw_priv->attr_data);
if (retval) {
printk(KERN_ERR "%s: sysfs_create_bin_file failed\n",
@@ -305,20 +334,9 @@ fw_setup_class_device(struct class_devic
goto error_remove_data;
}
- fw_priv->fw = kmalloc(sizeof (struct firmware), GFP_KERNEL);
- if (!fw_priv->fw) {
- printk(KERN_ERR "%s: kmalloc(struct firmware) failed\n",
- __FUNCTION__);
- retval = -ENOMEM;
- goto error_remove_loading;
- }
- memset(fw_priv->fw, 0, sizeof (*fw_priv->fw));
-
*class_dev_p = class_dev;
goto out;
-error_remove_loading:
- class_device_remove_file(class_dev, &class_device_attr_loading);
error_remove_data:
sysfs_remove_bin_file(&class_dev->kobj, &fw_priv->attr_data);
error_unreg_class_dev:
@@ -354,21 +372,29 @@ fw_remove_class_device(struct class_devi
* firmware image for this or any other device.
**/
int
-request_firmware(const struct firmware **firmware, const char *name,
+request_firmware(const struct firmware **firmware_p, const char *name,
struct device *device)
{
struct class_device *class_dev;
struct firmware_priv *fw_priv;
+ struct firmware *firmware;
int retval;
- if (!firmware)
+ if (!firmware_p)
return -EINVAL;
- *firmware = NULL;
+ *firmware_p = firmware = kmalloc(sizeof (struct firmware), GFP_KERNEL);
+ if (!firmware) {
+ printk(KERN_ERR "%s: kmalloc(struct firmware) failed\n",
+ __FUNCTION__);
+ retval = -ENOMEM;
+ goto out;
+ }
+ memset(firmware, 0, sizeof (*firmware));
- retval = fw_setup_class_device(&class_dev, name, device);
+ retval = fw_setup_class_device(firmware, &class_dev, name, device);
if (retval)
- goto out;
+ goto error_kfree_fw;
fw_priv = class_get_devdata(class_dev);
@@ -378,17 +404,23 @@ request_firmware(const struct firmware *
}
wait_for_completion(&fw_priv->completion);
+ set_bit(FW_STATUS_DONE, &fw_priv->status);
del_timer_sync(&fw_priv->timeout);
- if (fw_priv->fw->size && !test_bit(FW_STATUS_ABORT, &fw_priv->status)) {
- *firmware = fw_priv->fw;
- } else {
+ down(&fw_lock);
+ if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) {
retval = -ENOENT;
- vfree(fw_priv->fw->data);
- kfree(fw_priv->fw);
+ release_firmware(fw_priv->fw);
+ *firmware_p = NULL;
}
+ fw_priv->fw = NULL;
+ up(&fw_lock);
fw_remove_class_device(class_dev);
+ goto out;
+
+error_kfree_fw:
+ kfree(firmware);
out:
return retval;
}
_