diff mbox series

[v4,2/6] bdi: protect bdi->dev with spinlock

Message ID 20200325123843.47452-3-yuyufen@huawei.com (mailing list archive)
State New, archived
Headers show
Series bdi: fix use-after-free for bdi device | expand

Commit Message

Yufen Yu March 25, 2020, 12:38 p.m. UTC
We have reported a kernel crash in __blkg_prfill_rwstat() for
use-after-free the bdi->dev. It tries to get the device name by
'bdi->dev', while the device and kobj->name has been freed by
bdi_unregister().

In fact, similar bug have been reported in wb_workfn when
bdi_unregister(). To fix it, commit 68f23b89067f ("memcg: fix a
crash in wb_workfn when a device disappears") has created bdi_dev_name()
to handle bdi->dev beening NULL. But, bdi->dev can be freed after
bdi_dev_name() return successly, which can cause use-after-free for
dev or kobj->name.

In this patch, we try to protect the 'bdi->dev' with spinlock to
avoid the race. A new helper function bdi_get_dev_name() is defined.
Then, the caller can get device name with this helper safely.

This patch is prepare for following patch to fix use-after-free bug.

Signed-off-by: Yufen Yu <yuyufen@huawei.com>
---
 include/linux/backing-dev-defs.h |  1 +
 include/linux/backing-dev.h      | 28 ++++++++++++++++++++++++++++
 mm/backing-dev.c                 |  9 +++++++--
 3 files changed, 36 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 4fc87dee005a..c98dac4a7953 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -227,6 +227,7 @@  struct backing_dev_info {
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debug_dir;
 #endif
+	spinlock_t lock;		/* protects dev */
 };
 
 enum {
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index f88197c1ffc2..010d80956442 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -19,6 +19,8 @@ 
 #include <linux/backing-dev-defs.h>
 #include <linux/slab.h>
 
+#define BDI_DEV_NAME_LEN	32
+
 static inline struct backing_dev_info *bdi_get(struct backing_dev_info *bdi)
 {
 	kref_get(&bdi->refcnt);
@@ -514,4 +516,30 @@  static inline const char *bdi_dev_name(struct backing_dev_info *bdi)
 	return dev_name(bdi->dev);
 }
 
+/**
+ * bdi_get_dev_name - copy bdi device name into buffer
+ * @bdi: target bdi
+ * @dname: Where to copy the device name to
+ * @len: size of destination buffer
+ */
+static inline char *bdi_get_dev_name(struct backing_dev_info *bdi,
+			char *dname, int len)
+{
+	if (!bdi)
+		goto unknown;
+
+	spin_lock_irq(&bdi->lock);
+	if (!bdi->dev) {
+		spin_unlock_irq(&bdi->lock);
+		goto unknown;
+	}
+
+	strlcpy(dname, dev_name(bdi->dev), len);
+	spin_unlock_irq(&bdi->lock);
+	return dname;
+
+unknown:
+	strlcpy(dname, bdi_unknown_name, len);
+	return NULL;
+}
 #endif	/* _LINUX_BACKING_DEV_H */
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 62f05f605fb5..aa9ba7dcc2d9 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -859,6 +859,7 @@  static int bdi_init(struct backing_dev_info *bdi)
 	INIT_LIST_HEAD(&bdi->bdi_list);
 	INIT_LIST_HEAD(&bdi->wb_list);
 	init_waitqueue_head(&bdi->wb_waitq);
+	spin_lock_init(&bdi->lock);
 
 	ret = cgwb_bdi_init(bdi);
 
@@ -1007,15 +1008,19 @@  static void bdi_remove_from_list(struct backing_dev_info *bdi)
 
 void bdi_unregister(struct backing_dev_info *bdi)
 {
+	struct device *dev = bdi->dev;
+
 	/* make sure nobody finds us on the bdi_list anymore */
 	bdi_remove_from_list(bdi);
 	wb_shutdown(&bdi->wb);
 	cgwb_bdi_unregister(bdi);
 
-	if (bdi->dev) {
+	if (dev) {
 		bdi_debug_unregister(bdi);
-		device_unregister(bdi->dev);
+		spin_lock_irq(&bdi->lock);
 		bdi->dev = NULL;
+		spin_unlock_irq(&bdi->lock);
+		device_unregister(dev);
 	}
 
 	if (bdi->owner) {