>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,
+ addl_desc_ptr);
+ if (create)
+ enclosure_component_register(
+ ecomp);
+ }
}
if (desc_ptr)
desc_ptr += len;
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index 9a33c5f..a835d33 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -120,8 +120,9 @@ enclosure_register(struct device *, const char *, int,
struct enclosure_component_callbacks *);
void enclosure_unregister(struct enclosure_device *);
struct enclosure_component *
-enclosure_component_register(struct enclosure_device *, unsigned int,
- enum enclosure_component_type, const char *);
+enclosure_component_alloc(struct enclosure_device *, unsigned int,
+ enum enclosure_component_type, const char *);
+int enclosure_component_register(struct enclosure_component *);
int enclosure_add_device(struct enclosure_device *enclosure, int component,
struct device *dev);
int enclosure_remove_device(struct enclosure_device *, struct device *);
--
1.8.1
Thanks,
Song
> -----Original Message-----
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, September 4, 2014 12:51 AM
> To: Song Liu; linux-scsi@vger.kernel.org
> Cc: Dan Williams; Jens Axboe
> Subject: 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
