Commit 8bea2c84 authored by Alan Stern's avatar Alan Stern Committed by James Bottomley

[PATCH] Fix scsi host attributes

The shost_attrs stuff looks fine, expect for two points.

	1. The scsi_sysfs_modify_shost_attribute() and
scsi_sysfs_modify_sdev_attribute() functions appear to be written a bit
carelessly.  Below is a patch that: permits modification of the first
attribute in the list, allocates a new list with entries having the
correct size, copies the correct number of entries from the old list, and
wraps excessively long source lines.

	2. More importantly, the current organization of the code has a
serious problem.  The SCSI core does not modify the host driver when the
reference count for either shost->class_dev or shost->host_gendev drops to
0.  Without knowing that, it is unsafe for the driver ever to deallocate a
private host data structure, since a user process may continue to hold a
reference to an open attribute file indefinitely, even after
scsi_unregister() has returned.
parent 071a9d0f
...@@ -382,30 +382,36 @@ void scsi_sysfs_remove_host(struct Scsi_Host *shost) ...@@ -382,30 +382,36 @@ void scsi_sysfs_remove_host(struct Scsi_Host *shost)
* *
* returns zero if successful or error if not * returns zero if successful or error if not
**/ **/
int scsi_sysfs_modify_shost_attribute(struct class_device_attribute ***class_attrs, int scsi_sysfs_modify_shost_attribute(
struct class_device_attribute *attr) struct class_device_attribute ***class_attrs,
struct class_device_attribute *attr)
{ {
int modify = 0; int modify = -1;
int num_attrs; int num_attrs;
if(*class_attrs == NULL) if(*class_attrs == NULL)
*class_attrs = scsi_sysfs_shost_attrs; *class_attrs = scsi_sysfs_shost_attrs;
for(num_attrs=0; (*class_attrs)[num_attrs] != NULL; num_attrs++) for(num_attrs=0; (*class_attrs)[num_attrs] != NULL; num_attrs++)
if(strcmp((*class_attrs)[num_attrs]->attr.name, attr->attr.name) == 0) if(strcmp((*class_attrs)[num_attrs]->attr.name,
attr->attr.name) == 0)
modify = num_attrs; modify = num_attrs;
if(*class_attrs == scsi_sysfs_shost_attrs || !modify) { if(*class_attrs == scsi_sysfs_shost_attrs || modify < 0) {
/* note: need space for null at the end as well */ /* note: need space for null at the end as well */
struct class_device_attribute **tmp_attrs = kmalloc(sizeof(struct class_device_attribute)*(num_attrs + (modify ? 1 : 2)), GFP_KERNEL); struct class_device_attribute **tmp_attrs =
kmalloc(sizeof(*tmp_attrs) *
(num_attrs + (modify >= 0 ? 1 : 2)),
GFP_KERNEL);
if(tmp_attrs == NULL) if(tmp_attrs == NULL)
return -ENOMEM; return -ENOMEM;
memcpy(tmp_attrs, *class_attrs, sizeof(struct class_device_attribute)*num_attrs); memcpy(tmp_attrs, *class_attrs, sizeof(*tmp_attrs) *
(num_attrs + 1));
if(*class_attrs != scsi_sysfs_shost_attrs) if(*class_attrs != scsi_sysfs_shost_attrs)
kfree(*class_attrs); kfree(*class_attrs);
*class_attrs = tmp_attrs; *class_attrs = tmp_attrs;
} }
if(modify) { if(modify >= 0) {
/* spare the caller from having to copy things it's /* spare the caller from having to copy things it's
* not interested in */ * not interested in */
struct class_device_attribute *old_attr = struct class_device_attribute *old_attr =
...@@ -438,27 +444,32 @@ EXPORT_SYMBOL(scsi_sysfs_modify_shost_attribute); ...@@ -438,27 +444,32 @@ EXPORT_SYMBOL(scsi_sysfs_modify_shost_attribute);
int scsi_sysfs_modify_sdev_attribute(struct device_attribute ***dev_attrs, int scsi_sysfs_modify_sdev_attribute(struct device_attribute ***dev_attrs,
struct device_attribute *attr) struct device_attribute *attr)
{ {
int modify = 0; int modify = -1;
int num_attrs; int num_attrs;
if(*dev_attrs == NULL) if(*dev_attrs == NULL)
*dev_attrs = scsi_sysfs_sdev_attrs; *dev_attrs = scsi_sysfs_sdev_attrs;
for(num_attrs=0; (*dev_attrs)[num_attrs] != NULL; num_attrs++) for(num_attrs=0; (*dev_attrs)[num_attrs] != NULL; num_attrs++)
if(strcmp((*dev_attrs)[num_attrs]->attr.name, attr->attr.name) == 0) if(strcmp((*dev_attrs)[num_attrs]->attr.name,
attr->attr.name) == 0)
modify = num_attrs; modify = num_attrs;
if(*dev_attrs == scsi_sysfs_sdev_attrs || !modify) { if(*dev_attrs == scsi_sysfs_sdev_attrs || modify < 0) {
/* note: need space for null at the end as well */ /* note: need space for null at the end as well */
struct device_attribute **tmp_attrs = kmalloc(sizeof(struct device_attribute)*(num_attrs + (modify ? 1 : 2)), GFP_KERNEL); struct device_attribute **tmp_attrs =
kmalloc(sizeof(*tmp_attrs) *
(num_attrs + (modify >= 0 ? 1 : 2)),
GFP_KERNEL);
if(tmp_attrs == NULL) if(tmp_attrs == NULL)
return -ENOMEM; return -ENOMEM;
memcpy(tmp_attrs, *dev_attrs, sizeof(struct device_attribute)*num_attrs); memcpy(tmp_attrs, *dev_attrs, sizeof(*tmp_attrs) *
(num_attrs + 1));
if(*dev_attrs != scsi_sysfs_sdev_attrs) if(*dev_attrs != scsi_sysfs_sdev_attrs)
kfree(*dev_attrs); kfree(*dev_attrs);
*dev_attrs = tmp_attrs; *dev_attrs = tmp_attrs;
} }
if(modify) { if(modify >= 0) {
/* spare the caller from having to copy things it's /* spare the caller from having to copy things it's
* not interested in */ * not interested in */
struct device_attribute *old_attr = struct device_attribute *old_attr =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment