LU-13508 mdc: chlg device could be used after free

There are some issue of the usage of dynamic devices used by
the changelog in MDC, which could cause the device to be used
after it is freed.

Change-Id: Iacf6fa7c8b612f1a373091cf88e7082c4860cfe4
Signed-off-by: Hongchao Zhang <hongchao@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/38658
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/mdc/mdc_changelog.c b/lustre/mdc/mdc_changelog.c
index 634fbd8..27b92ee 100644 (file)
--- a/lustre/mdc/mdc_changelog.c
+++ b/lustre/mdc/mdc_changelog.c
@@ -62,7 +62,7 @@ struct chlg_registered_dev {
        char                     ced_name[32];
        /* changelog char device */
        struct cdev              ced_cdev;
-       struct device           *ced_device;
+       struct device            ced_device;
        /* OBDs referencing this device (multiple mount point) */
        struct list_head         ced_obds;
        /* Reference counter for proper deregistration */
@@ -113,7 +113,7 @@ enum {
        CDEV_CHLG_MAX_PREFETCH = 1024,
 };
 
-static DEFINE_IDR(chlg_minor_idr);
+DEFINE_IDR(mdc_changelog_minor_idr);
 static DEFINE_SPINLOCK(chlg_minor_lock);
 
 static int chlg_minor_alloc(int *pminor)
@@ -123,7 +123,7 @@ static int chlg_minor_alloc(int *pminor)
 
        idr_preload(GFP_KERNEL);
        spin_lock(&chlg_minor_lock);
-       minor = idr_alloc(&chlg_minor_idr, minor_allocated, 0,
+       minor = idr_alloc(&mdc_changelog_minor_idr, minor_allocated, 0,
                          MDC_CHANGELOG_DEV_COUNT, GFP_NOWAIT);
        spin_unlock(&chlg_minor_lock);
        idr_preload_end();
@@ -138,7 +138,7 @@ static int chlg_minor_alloc(int *pminor)
 static void chlg_minor_free(int minor)
 {
        spin_lock(&chlg_minor_lock);
-       idr_remove(&chlg_minor_idr, minor);
+       idr_remove(&mdc_changelog_minor_idr, minor);
        spin_unlock(&chlg_minor_lock);
 }
 
@@ -156,14 +156,14 @@ static void chlg_device_release(struct device *dev)
 static void chlg_dev_clear(struct kref *kref)
 {
        struct chlg_registered_dev *entry;
-       
+
        ENTRY;
        entry = container_of(kref, struct chlg_registered_dev,
                             ced_refs);
 
        list_del(&entry->ced_link);
-       cdev_del(&entry->ced_cdev);
-       device_destroy(mdc_changelog_class, entry->ced_cdev.dev);
+       cdev_device_del(&entry->ced_cdev, &entry->ced_device);
+       put_device(&entry->ced_device);
        EXIT;
 }
 
@@ -781,8 +781,6 @@ int mdc_changelog_cdev_init(struct obd_device *obd)
 {
        struct chlg_registered_dev *exist;
        struct chlg_registered_dev *entry;
-       struct device *device;
-       dev_t dev;
        int minor, rc;
        ENTRY;
 
@@ -807,32 +805,32 @@ int mdc_changelog_cdev_init(struct obd_device *obd)
        list_add_tail(&obd->u.cli.cl_chg_dev_linkage, &entry->ced_obds);
        list_add_tail(&entry->ced_link, &chlg_registered_devices);
 
-       /* Register new character device */
-       cdev_init(&entry->ced_cdev, &chlg_fops);
-       entry->ced_cdev.owner = THIS_MODULE;
-
        rc = chlg_minor_alloc(&minor);
        if (rc)
                GOTO(out_unlock, rc);
 
-       dev = MKDEV(MAJOR(mdc_changelog_dev), minor);
-       rc = cdev_add(&entry->ced_cdev, dev, 1);
+       device_initialize(&entry->ced_device);
+       entry->ced_device.devt = MKDEV(MAJOR(mdc_changelog_dev), minor);
+       entry->ced_device.class = mdc_changelog_class;
+       entry->ced_device.release = chlg_device_release;
+       dev_set_drvdata(&entry->ced_device, entry);
+       rc = dev_set_name(&entry->ced_device, "%s-%s", MDC_CHANGELOG_DEV_NAME,
+                         entry->ced_name);
        if (rc)
                GOTO(out_minor, rc);
 
-       device = device_create(mdc_changelog_class, NULL, dev, entry, "%s-%s",
-                              MDC_CHANGELOG_DEV_NAME, entry->ced_name);
-       if (IS_ERR(device))
-               GOTO(out_cdev, rc = PTR_ERR(device));
-
-       device->release = chlg_device_release;
-       entry->ced_device = device;
+       /* Register new character device */
+       cdev_init(&entry->ced_cdev, &chlg_fops);
+       entry->ced_cdev.owner = THIS_MODULE;
+       rc = cdev_device_add(&entry->ced_cdev, &entry->ced_device);
+       if (rc)
+               GOTO(out_device_name, rc);
 
        entry = NULL;   /* prevent it from being freed below */
        GOTO(out_unlock, rc = 0);
 
-out_cdev:
-       cdev_del(&entry->ced_cdev);
+out_device_name:
+       kfree_const(entry->ced_device.kobj.name);
 
 out_minor:
        chlg_minor_free(minor);

diff --git a/lustre/mdc/mdc_internal.h b/lustre/mdc/mdc_internal.h
index d599034..c7e4793 100644 (file)
--- a/lustre/mdc/mdc_internal.h
+++ b/lustre/mdc/mdc_internal.h
@@ -153,6 +153,7 @@ enum ldlm_mode mdc_lock_match(struct obd_export *exp, __u64 flags,
 #define MDC_CHANGELOG_DEV_NAME "changelog"
 extern struct class *mdc_changelog_class;
 extern dev_t mdc_changelog_dev;
+extern struct idr mdc_changelog_minor_idr;
 
 int mdc_changelog_cdev_init(struct obd_device *obd);
 

diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c
index f07a0dc..82d1b5e 100644 (file)
--- a/lustre/mdc/mdc_request.c
+++ b/lustre/mdc/mdc_request.c
@@ -2995,10 +2995,12 @@ static int __init mdc_init(void)
        rc = class_register_type(&mdc_obd_ops, &mdc_md_ops, true, NULL,
                                 LUSTRE_MDC_NAME, &mdc_device_type);
        if (rc)
-               goto out_dev;
+               goto out_class;
 
        return 0;
 
+out_class:
+       class_destroy(mdc_changelog_class);
 out_dev:
        unregister_chrdev_region(mdc_changelog_dev, MDC_CHANGELOG_DEV_COUNT);
        return rc;
@@ -3006,9 +3008,10 @@ out_dev:
 
 static void __exit mdc_exit(void)
 {
+       class_unregister_type(LUSTRE_MDC_NAME);
        class_destroy(mdc_changelog_class);
        unregister_chrdev_region(mdc_changelog_dev, MDC_CHANGELOG_DEV_COUNT);
-       class_unregister_type(LUSTRE_MDC_NAME);
+       idr_destroy(&mdc_changelog_minor_idr);
 }
 
 MODULE_AUTHOR("OpenSFS, Inc. <http://www.lustre.org/>");