From: Manuel Estrada Sainz - 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 #include #include +#include #include #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; } _