Re: [PATCH 1/5] ses: close potential registration race
On 12/30/2014 11:46 PM, Song Liu wrote: From: Dan Williams dan.j.willi...@intel.com The slot and address fields have a small window of instability when userspace can read them before initialization. Separate enclosure_component allocation from registration. Signed-off-by: Dan Williams dan.j.willi...@intel.com Signed-off-by: Song Liu songliubrav...@fb.com Reviewed-by: Jens Axboe ax...@fb.com Cc: Hannes Reinecke h...@suse.de Reviewed-by: Hannes Reinecke h...@suse.de Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] ses: close potential registration race
From: Dan Williams dan.j.willi...@intel.com
The slot and address fields have a small window of instability when
userspace can read them before initialization. Separate
enclosure_component
allocation from registration.
Signed-off-by: Dan Williams dan.j.willi...@intel.com
Signed-off-by: Song Liu songliubrav...@fb.com
Reviewed-by: Jens Axboe ax...@fb.com
Cc: Hannes Reinecke h...@suse.de
---
drivers/misc/enclosure.c | 35 +--
drivers/scsi/ses.c| 21 ++---
include/linux/enclosure.h | 5 +++--
3 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 180a544..d566f0f 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -273,23 +273,22 @@ enclosure_component_find_by_name(struct enclosure_device
*edev,
static const struct attribute_group *enclosure_component_groups[];
/**
- * enclosure_component_register - add a particular component to an enclosure
+ * enclosure_component_alloc - prepare a new enclosure component
* @edev: the enclosure to add the component
* @num: the device number
* @type: the type of component being added
* @name: an optional name to appear in sysfs (leave NULL if none)
*
- * Registers the component. The name is optional for enclosures that
- * give their components a unique name. If not, leave the field NULL
- * and a name will be assigned.
+ * The name is optional for enclosures that give their components a unique
+ * name. If not, leave the field NULL and a name will be assigned.
*
* Returns a pointer to the enclosure component or an error.
*/
struct enclosure_component *
-enclosure_component_register(struct enclosure_device *edev,
-unsigned int number,
-enum enclosure_component_type type,
-const char *name)
+enclosure_component_alloc(struct enclosure_device *edev,
+ unsigned int number,
+ enum enclosure_component_type type,
+ const char *name)
{
struct enclosure_component *ecomp;
struct device *cdev;
@@ -327,14 +326,30 @@ enclosure_component_register(struct enclosure_device
*edev,
cdev-release = enclosure_component_release;
cdev-groups = enclosure_component_groups;
+ return ecomp;
+}
+EXPORT_SYMBOL_GPL(enclosure_component_alloc);
+
+/**
+ * enclosure_component_register - publishes an initialized enclosure component
+ * @ecomp: component to add
+ *
+ * Returns 0 on successful registration, releases the component otherwise
+ */
+int enclosure_component_register(struct enclosure_component *ecomp)
+{
+ struct device *cdev;
+ int err;
+
+ cdev = ecomp-cdev;
err = device_register(cdev);
if (err) {
ecomp-number = -1;
put_device(cdev);
- return ERR_PTR(err);
+ return err;
}
- return ecomp;
+ return 0;
}
EXPORT_SYMBOL_GPL(enclosure_component_register);
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index b7e79e7..7dd9cf5 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -423,16 +423,23 @@ static void ses_enclosure_data_process(struct
enclosure_device *edev,
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
if (create)
- ecomp =
enclosure_component_register(edev,
-
components++,
-
type_ptr[0],
-
name);
+ ecomp = enclosure_component_alloc(
+ edev,
+ components++,
+ type_ptr[0],
+ name);
else
ecomp = edev-component[components++];
- if (!IS_ERR(ecomp) addl_desc_ptr)
- ses_process_descriptor(ecomp,
- addl_desc_ptr);
+ if (!IS_ERR(ecomp)) {
+ if (addl_desc_ptr)
+ ses_process_descriptor(
+ ecomp,
+ addl_desc_ptr);
+ if (create)
+ enclosure_component_register(
+ ecomp);
+
RE: [PATCH 1/5] SES: close potential registration race
From 22d9dbcd783b315aaf59ad3eb928addaf0fba5fa Mon Sep 17 00:00:00 2001
From: Dan Williams dan.j.willi...@intel.com
Date: Thu, 21 Aug 2014 11:42:46 -0700
Subject: [PATCH 1/5] SES: close potential registration race
The slot and address fields have a small window of instability when
userspace can read them before initialization. Separate
enclosure_component
allocation from registration.
Signed-off-by: Dan Williams dan.j.willi...@intel.com
Signed-off-by: Song Liu songliubrav...@fb.com
Reviewed-by: Jens Axboe ax...@fb.com
Cc: Hannes Reinecke h...@suse.de
---
drivers/misc/enclosure.c | 36 +---
drivers/scsi/ses.c| 21 ++---
include/linux/enclosure.h | 5 +++--
3 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 2cf2bbc..15faf61 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -249,27 +249,25 @@ static void enclosure_component_release(struct device
*dev)
static const struct attribute_group *enclosure_component_groups[];
/**
- * enclosure_component_register - add a particular component to an enclosure
+ * enclosure_component_alloc - prepare a new enclosure component
* @edev: the enclosure to add the component
* @num: the device number
* @type: the type of component being added
* @name: an optional name to appear in sysfs (leave NULL if none)
*
- * Registers the component. The name is optional for enclosures that
- * give their components a unique name. If not, leave the field NULL
- * and a name will be assigned.
+ * The name is optional for enclosures that give their components a unique
+ * name. If not, leave the field NULL and a name will be assigned.
*
* Returns a pointer to the enclosure component or an error.
*/
struct enclosure_component *
-enclosure_component_register(struct enclosure_device *edev,
-unsigned int number,
-enum enclosure_component_type type,
-const char *name)
+enclosure_component_alloc(struct enclosure_device *edev,
+ unsigned int number,
+ enum enclosure_component_type type,
+ const char *name)
{
struct enclosure_component *ecomp;
struct device *cdev;
- int err;
if (number = edev-components)
return ERR_PTR(-EINVAL);
@@ -291,14 +289,30 @@ enclosure_component_register(struct enclosure_device
*edev,
cdev-release = enclosure_component_release;
cdev-groups = enclosure_component_groups;
+ return ecomp;
+}
+EXPORT_SYMBOL_GPL(enclosure_component_alloc);
+
+/**
+ * enclosure_component_register - publishes an initialized enclosure component
+ * @ecomp: component to add
+ *
+ * Returns 0 on successful registration, releases the component otherwise
+ */
+int enclosure_component_register(struct enclosure_component *ecomp)
+{
+ struct device *cdev;
+ int err;
+
+ cdev = ecomp-cdev;
err = device_register(cdev);
if (err) {
ecomp-number = -1;
put_device(cdev);
- return ERR_PTR(err);
+ return err;
}
- return ecomp;
+ return 0;
}
EXPORT_SYMBOL_GPL(enclosure_component_register);
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 80bfece..c52fd98 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -423,16 +423,23 @@ static void ses_enclosure_data_process(struct
enclosure_device *edev,
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
if (create)
- ecomp =
enclosure_component_register(edev,
-
components++,
-
type_ptr[0],
-
name);
+ ecomp = enclosure_component_alloc(
+ edev,
+ components++,
+ type_ptr[0],
+ name);
else
ecomp = edev-component[components++];
- if (!IS_ERR(ecomp) addl_desc_ptr)
- ses_process_descriptor(ecomp,
- addl_desc_ptr);
+ if (!IS_ERR(ecomp)) {
+ if (addl_desc_ptr)
+ ses_process_descriptor(
+ ecomp
Re: [PATCH 1/5] SES: close potential registration race
On 08/25/2014 07:34 PM, Song Liu wrote: From: Song Liu [mailto:songliubrav...@fb.com] Sent: Monday, August 25, 2014 10:26 AM To: Song Liu Cc: Dan Williams; Hannes Reinecke Subject: [PATCH 1/5] SES: close potential registration race From: Dan Williams dan.j.willi...@intel.com The slot and address fields have a small window of instability when userspace can read them before initialization. Separate enclosure_component allocation from registration. Signed-off-by: Dan Williams dan.j.willi...@intel.com Signed-off-by: Song Liu songliubrav...@fb.com Reviewed-by: Jens Axboe ax...@fb.com Cc: Hannes Reinecke h...@suse.de Please run the patch against checkpatch.pl. It does have quite some coding-style issues. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
