[lustre-devel] [PATCH 08/18] lustre: mdc: chlg device could be used after free
James Simmons
jsimmons at infradead.org
Wed Jul 1 17:04:48 PDT 2020
From: Hongchao Zhang <hongchao at whamcloud.com>
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.
WC-bug-id: https://jira.whamcloud.com/browse/LU-13508
Lustre-commit: 1e992e94eaf8a ("LU-13508 mdc: chlg device could be used after free")
Signed-off-by: Hongchao Zhang <hongchao at whamcloud.com>
Reviewed-on: https://review.whamcloud.com/38658
Reviewed-by: John L. Hammond <jhammond at whamcloud.com>
Reviewed-by: James Simmons <jsimmons at infradead.org>
Reviewed-by: Oleg Drokin <green at whamcloud.com>
Signed-off-by: James Simmons <jsimmons at infradead.org>
---
fs/lustre/mdc/mdc_changelog.c | 46 ++++++++++++++++++++-----------------------
fs/lustre/mdc/mdc_internal.h | 1 +
fs/lustre/mdc/mdc_request.c | 8 +++++---
3 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/fs/lustre/mdc/mdc_changelog.c b/fs/lustre/mdc/mdc_changelog.c
index 3aace7e..8531edb 100644
--- a/fs/lustre/mdc/mdc_changelog.c
+++ b/fs/lustre/mdc/mdc_changelog.c
@@ -61,7 +61,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 */
@@ -112,7 +112,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)
@@ -122,7 +122,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();
@@ -137,7 +137,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);
}
@@ -160,8 +160,8 @@ static void chlg_dev_clear(struct kref *kref)
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);
}
static inline struct obd_device *chlg_obd_get(struct chlg_registered_dev *dev)
@@ -790,8 +790,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 = kzalloc(sizeof(*entry), GFP_KERNEL);
@@ -816,35 +814,33 @@ 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;
- 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;
- device = device_create(mdc_changelog_class, NULL, dev, entry, "%s-%s",
- MDC_CHANGELOG_DEV_NAME, entry->ced_name);
- if (IS_ERR(device)) {
- rc = PTR_ERR(device);
- goto out_cdev;
- }
-
- 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;
entry = NULL; /* prevent it from being freed below */
rc = 0;
goto out_unlock;
-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/fs/lustre/mdc/mdc_internal.h b/fs/lustre/mdc/mdc_internal.h
index 9656231..b7ccc58 100644
--- a/fs/lustre/mdc/mdc_internal.h
+++ b/fs/lustre/mdc/mdc_internal.h
@@ -142,6 +142,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/fs/lustre/mdc/mdc_request.c b/fs/lustre/mdc/mdc_request.c
index 369114b..d6d9f43 100644
--- a/fs/lustre/mdc/mdc_request.c
+++ b/fs/lustre/mdc/mdc_request.c
@@ -3013,10 +3013,12 @@ static int __init mdc_init(void)
rc = class_register_type(&mdc_obd_ops, &mdc_md_ops,
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;
@@ -3024,9 +3026,9 @@ static int __init mdc_init(void)
static void __exit mdc_exit(void)
{
- class_destroy(mdc_changelog_class);
- unregister_chrdev_region(mdc_changelog_dev, MDC_CHANGELOG_DEV_COUNT);
class_unregister_type(LUSTRE_MDC_NAME);
+ class_destroy(mdc_changelog_class);
+ idr_destroy(&mdc_changelog_minor_idr);
}
MODULE_AUTHOR("OpenSFS, Inc. <http://www.lustre.org/>");
--
1.8.3.1
More information about the lustre-devel
mailing list