linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* bdi: fix use-after-free for dev_name(bdi->dev) v2
@ 2020-04-16 16:54 Christoph Hellwig
  2020-04-16 16:54 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
                   ` (7 more replies)
  0 siblings, 8 replies; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

Hi all,

the first three patches are my take on the proposal from Yufen Yu
to fix the use after free of the device name of the bdi device.

The rest is vaguely related cleanups.

Changes since v1:
 - use a static dev_name buffer inside struct backing_dev_info

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/8] bdi: move bdi_dev_name out of line
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-18 15:35   ` Bart Van Assche
  2020-04-16 16:54 ` [PATCH 2/8] bdi: use bdi_dev_name() to get device name Christoph Hellwig
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

bdi_dev_name is not a fast path function, move it out of line.  This
prepares for using it from modular callers without having to export
an implementation detail like bdi_unknown_name.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/backing-dev.h |  9 +--------
 mm/backing-dev.c            | 10 +++++++++-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index f88197c1ffc2..c9ad5c3b7b4b 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -505,13 +505,6 @@ static inline int bdi_rw_congested(struct backing_dev_info *bdi)
 				  (1 << WB_async_congested));
 }
 
-extern const char *bdi_unknown_name;
-
-static inline const char *bdi_dev_name(struct backing_dev_info *bdi)
-{
-	if (!bdi || !bdi->dev)
-		return bdi_unknown_name;
-	return dev_name(bdi->dev);
-}
+const char *bdi_dev_name(struct backing_dev_info *bdi);
 
 #endif	/* _LINUX_BACKING_DEV_H */
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index c81b4f3a7268..c2c44c89ee5d 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -21,7 +21,7 @@ struct backing_dev_info noop_backing_dev_info = {
 EXPORT_SYMBOL_GPL(noop_backing_dev_info);
 
 static struct class *bdi_class;
-const char *bdi_unknown_name = "(unknown)";
+static const char *bdi_unknown_name = "(unknown)";
 
 /*
  * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
@@ -1043,6 +1043,14 @@ void bdi_put(struct backing_dev_info *bdi)
 }
 EXPORT_SYMBOL(bdi_put);
 
+const char *bdi_dev_name(struct backing_dev_info *bdi)
+{
+	if (!bdi || !bdi->dev)
+		return bdi_unknown_name;
+	return dev_name(bdi->dev);
+}
+EXPORT_SYMBOL_GPL(bdi_dev_name);
+
 static wait_queue_head_t congestion_wqh[2] = {
 		__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[0]),
 		__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[1])
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/8] bdi: use bdi_dev_name() to get device name
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
  2020-04-16 16:54 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-18 15:37   ` Bart Van Assche
  2020-04-16 16:54 ` [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info Christoph Hellwig
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

From: Yufen Yu <yuyufen@huawei.com>

Use the common interface bdi_dev_name() to get device name.

Signed-off-by: Yufen Yu <yuyufen@huawei.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c        | 5 +++--
 block/blk-cgroup.c         | 2 +-
 fs/ceph/debugfs.c          | 2 +-
 include/trace/events/wbt.h | 8 ++++----
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 78ba57efd16b..4d4fe44a9eea 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4976,8 +4976,9 @@ bfq_set_next_ioprio_data(struct bfq_queue *bfqq, struct bfq_io_cq *bic)
 	ioprio_class = IOPRIO_PRIO_CLASS(bic->ioprio);
 	switch (ioprio_class) {
 	default:
-		dev_err(bfqq->bfqd->queue->backing_dev_info->dev,
-			"bfq: bad prio class %d\n", ioprio_class);
+		pr_err("bdi %s: bfq: bad prio class %d\n",
+				bdi_dev_name(bfqq->bfqd->queue->backing_dev_info),
+				ioprio_class);
 		/* fall through */
 	case IOPRIO_CLASS_NONE:
 		/*
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index c5dc833212e1..930212c1a512 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -496,7 +496,7 @@ const char *blkg_dev_name(struct blkcg_gq *blkg)
 {
 	/* some drivers (floppy) instantiate a queue w/o disk registered */
 	if (blkg->q->backing_dev_info->dev)
-		return dev_name(blkg->q->backing_dev_info->dev);
+		return bdi_dev_name(blkg->q->backing_dev_info);
 	return NULL;
 }
 
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 481ac97b4d25..dcaed75de9e6 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -271,7 +271,7 @@ void ceph_fs_debugfs_init(struct ceph_fs_client *fsc)
 				    &congestion_kb_fops);
 
 	snprintf(name, sizeof(name), "../../bdi/%s",
-		 dev_name(fsc->sb->s_bdi->dev));
+		 bdi_dev_name(fsc->sb->s_bdi));
 	fsc->debugfs_bdi =
 		debugfs_create_symlink("bdi",
 				       fsc->client->debugfs_dir,
diff --git a/include/trace/events/wbt.h b/include/trace/events/wbt.h
index 37342a13c9cb..9996420d7ec4 100644
--- a/include/trace/events/wbt.h
+++ b/include/trace/events/wbt.h
@@ -33,7 +33,7 @@ TRACE_EVENT(wbt_stat,
 	),
 
 	TP_fast_assign(
-		strlcpy(__entry->name, dev_name(bdi->dev),
+		strlcpy(__entry->name, bdi_dev_name(bdi),
 			ARRAY_SIZE(__entry->name));
 		__entry->rmean		= stat[0].mean;
 		__entry->rmin		= stat[0].min;
@@ -68,7 +68,7 @@ TRACE_EVENT(wbt_lat,
 	),
 
 	TP_fast_assign(
-		strlcpy(__entry->name, dev_name(bdi->dev),
+		strlcpy(__entry->name, bdi_dev_name(bdi),
 			ARRAY_SIZE(__entry->name));
 		__entry->lat = div_u64(lat, 1000);
 	),
@@ -105,7 +105,7 @@ TRACE_EVENT(wbt_step,
 	),
 
 	TP_fast_assign(
-		strlcpy(__entry->name, dev_name(bdi->dev),
+		strlcpy(__entry->name, bdi_dev_name(bdi),
 			ARRAY_SIZE(__entry->name));
 		__entry->msg	= msg;
 		__entry->step	= step;
@@ -141,7 +141,7 @@ TRACE_EVENT(wbt_timer,
 	),
 
 	TP_fast_assign(
-		strlcpy(__entry->name, dev_name(bdi->dev),
+		strlcpy(__entry->name, bdi_dev_name(bdi),
 			ARRAY_SIZE(__entry->name));
 		__entry->status		= status;
 		__entry->step		= step;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
  2020-04-16 16:54 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
  2020-04-16 16:54 ` [PATCH 2/8] bdi: use bdi_dev_name() to get device name Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-17  8:59   ` Jan Kara
  2020-04-16 16:54 ` [PATCH 4/8] driver core: remove device_create_vargs Christoph Hellwig
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

Cache a copy of the name for the life time of the backing_dev_info
structure so that we can reference it even after unregistering.

Fixes: 68f23b89067f ("memcg: fix a crash in wb_workfn when a device disappears")
Reported-by: Yufen Yu <yuyufen@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/backing-dev-defs.h | 1 +
 mm/backing-dev.c                 | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 4fc87dee005a..2849bdbb3acb 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -220,6 +220,7 @@ struct backing_dev_info {
 	wait_queue_head_t wb_waitq;
 
 	struct device *dev;
+	char dev_name[64];
 	struct device *owner;
 
 	struct timer_list laptop_mode_wb_timer;
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index c2c44c89ee5d..efc5b83acd2d 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -938,7 +938,8 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
 	if (bdi->dev)	/* The driver needs to use separate queues per device */
 		return 0;
 
-	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
+	vsnprintf(bdi->dev_name, sizeof(bdi->dev_name), fmt, args);
+	dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name);
 	if (IS_ERR(dev))
 		return PTR_ERR(dev);
 
@@ -1047,7 +1048,7 @@ const char *bdi_dev_name(struct backing_dev_info *bdi)
 {
 	if (!bdi || !bdi->dev)
 		return bdi_unknown_name;
-	return dev_name(bdi->dev);
+	return bdi->dev_name;
 }
 EXPORT_SYMBOL_GPL(bdi_dev_name);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/8] driver core: remove device_create_vargs
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-04-16 16:54 ` [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-16 16:54 ` [PATCH 5/8] bdi: unexport bdi_register_va Christoph Hellwig
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

All external users of device_create_vargs are gone, so remove it and
open code it in the only caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/core.c    | 37 ++-----------------------------------
 include/linux/device.h |  4 ----
 2 files changed, 2 insertions(+), 39 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 139cdf7e7327..fb8ae248e5aa 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3188,40 +3188,6 @@ device_create_groups_vargs(struct class *class, struct device *parent,
 	return ERR_PTR(retval);
 }
 
-/**
- * device_create_vargs - creates a device and registers it with sysfs
- * @class: pointer to the struct class that this device should be registered to
- * @parent: pointer to the parent struct device of this new device, if any
- * @devt: the dev_t for the char device to be added
- * @drvdata: the data to be added to the device for callbacks
- * @fmt: string for the device's name
- * @args: va_list for the device's name
- *
- * This function can be used by char device classes.  A struct device
- * will be created in sysfs, registered to the specified class.
- *
- * A "dev" file will be created, showing the dev_t for the device, if
- * the dev_t is not 0,0.
- * If a pointer to a parent struct device is passed in, the newly created
- * struct device will be a child of that device in sysfs.
- * The pointer to the struct device will be returned from the call.
- * Any further sysfs files that might be required can be created using this
- * pointer.
- *
- * Returns &struct device pointer on success, or ERR_PTR() on error.
- *
- * Note: the struct class passed to this function must have previously
- * been created with a call to class_create().
- */
-struct device *device_create_vargs(struct class *class, struct device *parent,
-				   dev_t devt, void *drvdata, const char *fmt,
-				   va_list args)
-{
-	return device_create_groups_vargs(class, parent, devt, drvdata, NULL,
-					  fmt, args);
-}
-EXPORT_SYMBOL_GPL(device_create_vargs);
-
 /**
  * device_create - creates a device and registers it with sysfs
  * @class: pointer to the struct class that this device should be registered to
@@ -3253,7 +3219,8 @@ struct device *device_create(struct class *class, struct device *parent,
 	struct device *dev;
 
 	va_start(vargs, fmt);
-	dev = device_create_vargs(class, parent, devt, drvdata, fmt, vargs);
+	dev = device_create_groups_vargs(class, parent, devt, drvdata, NULL,
+					  fmt, vargs);
 	va_end(vargs);
 	return dev;
 }
diff --git a/include/linux/device.h b/include/linux/device.h
index ac8e37cd716a..15460a5ac024 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -884,10 +884,6 @@ extern bool device_is_bound(struct device *dev);
 /*
  * Easy functions for dynamically creating devices on the fly
  */
-extern __printf(5, 0)
-struct device *device_create_vargs(struct class *cls, struct device *parent,
-				   dev_t devt, void *drvdata,
-				   const char *fmt, va_list vargs);
 extern __printf(5, 6)
 struct device *device_create(struct class *cls, struct device *parent,
 			     dev_t devt, void *drvdata,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 5/8] bdi: unexport bdi_register_va
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-04-16 16:54 ` [PATCH 4/8] driver core: remove device_create_vargs Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-18 15:41   ` Bart Van Assche
  2020-04-16 16:54 ` [PATCH 6/8] bdi: remove bdi_register_owner Christoph Hellwig
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

bdi_register_va is only used by fs/super.c, which can't be modular.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/backing-dev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index efc5b83acd2d..eb6b51e49d11 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -964,7 +964,6 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
 	trace_writeback_bdi_register(bdi);
 	return 0;
 }
-EXPORT_SYMBOL(bdi_register_va);
 
 int bdi_register(struct backing_dev_info *bdi, const char *fmt, ...)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 6/8] bdi: remove bdi_register_owner
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-04-16 16:54 ` [PATCH 5/8] bdi: unexport bdi_register_va Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-18 15:43   ` Bart Van Assche
  2020-04-16 16:54 ` [PATCH 7/8] bdi: simplify bdi_alloc Christoph Hellwig
  2020-04-16 16:54 ` [PATCH 8/8] bdi: remove the name field in struct backing_dev_info Christoph Hellwig
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

Split out a new bdi_set_owner helper to set the owner, and move the policy
for creating the bdi name back into genhd.c, where it belongs.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 block/genhd.c               |  8 +++++---
 include/linux/backing-dev.h |  2 +-
 mm/backing-dev.c            | 12 ++----------
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 06b642b23a07..7d10cfc38c70 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -840,13 +840,15 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 		disk->flags |= GENHD_FL_SUPPRESS_PARTITION_INFO;
 		disk->flags |= GENHD_FL_NO_PART_SCAN;
 	} else {
+		struct backing_dev_info *bdi = disk->queue->backing_dev_info;
+		struct device *dev = disk_to_dev(disk);
 		int ret;
 
 		/* Register BDI before referencing it from bdev */
-		disk_to_dev(disk)->devt = devt;
-		ret = bdi_register_owner(disk->queue->backing_dev_info,
-						disk_to_dev(disk));
+		dev->devt = devt;
+		ret = bdi_register(bdi, "%u:%u", MAJOR(devt), MINOR(devt));
 		WARN_ON(ret);
+		bdi_set_owner(bdi, dev);
 		blk_register_region(disk_devt(disk), disk->minors, NULL,
 				    exact_match, exact_lock, disk);
 	}
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index c9ad5c3b7b4b..4098ed6ba6b4 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -33,7 +33,7 @@ int bdi_register(struct backing_dev_info *bdi, const char *fmt, ...);
 __printf(2, 0)
 int bdi_register_va(struct backing_dev_info *bdi, const char *fmt,
 		    va_list args);
-int bdi_register_owner(struct backing_dev_info *bdi, struct device *owner);
+void bdi_set_owner(struct backing_dev_info *bdi, struct device *owner);
 void bdi_unregister(struct backing_dev_info *bdi);
 
 struct backing_dev_info *bdi_alloc_node(gfp_t gfp_mask, int node_id);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index eb6b51e49d11..bb993f99d424 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -977,20 +977,12 @@ int bdi_register(struct backing_dev_info *bdi, const char *fmt, ...)
 }
 EXPORT_SYMBOL(bdi_register);
 
-int bdi_register_owner(struct backing_dev_info *bdi, struct device *owner)
+void bdi_set_owner(struct backing_dev_info *bdi, struct device *owner)
 {
-	int rc;
-
-	rc = bdi_register(bdi, "%u:%u", MAJOR(owner->devt), MINOR(owner->devt));
-	if (rc)
-		return rc;
-	/* Leaking owner reference... */
-	WARN_ON(bdi->owner);
+	WARN_ON_ONCE(bdi->owner);
 	bdi->owner = owner;
 	get_device(owner);
-	return 0;
 }
-EXPORT_SYMBOL(bdi_register_owner);
 
 /*
  * Remove bdi from bdi_list, and ensure that it is no longer visible
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 7/8] bdi: simplify bdi_alloc
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2020-04-16 16:54 ` [PATCH 6/8] bdi: remove bdi_register_owner Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-18 15:44   ` Bart Van Assche
  2020-04-16 16:54 ` [PATCH 8/8] bdi: remove the name field in struct backing_dev_info Christoph Hellwig
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

Merge the _node vs normal version and drop the superflous gfp_t argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 block/blk-core.c            | 2 +-
 drivers/mtd/mtdcore.c       | 2 +-
 fs/super.c                  | 2 +-
 include/linux/backing-dev.h | 6 +-----
 mm/backing-dev.c            | 7 +++----
 5 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 7e4a1da0715e..ab87f2833ab2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -484,7 +484,7 @@ struct request_queue *__blk_alloc_queue(int node_id)
 	if (ret)
 		goto fail_id;
 
-	q->backing_dev_info = bdi_alloc_node(GFP_KERNEL, node_id);
+	q->backing_dev_info = bdi_alloc(node_id);
 	if (!q->backing_dev_info)
 		goto fail_split;
 
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 2916674208b3..39ec563d9a14 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -2036,7 +2036,7 @@ static struct backing_dev_info * __init mtd_bdi_init(char *name)
 	struct backing_dev_info *bdi;
 	int ret;
 
-	bdi = bdi_alloc(GFP_KERNEL);
+	bdi = bdi_alloc(NUMA_NO_NODE);
 	if (!bdi)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/super.c b/fs/super.c
index cd352530eca9..dd28fcd706ff 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1598,7 +1598,7 @@ int super_setup_bdi_name(struct super_block *sb, char *fmt, ...)
 	int err;
 	va_list args;
 
-	bdi = bdi_alloc(GFP_KERNEL);
+	bdi = bdi_alloc(NUMA_NO_NODE);
 	if (!bdi)
 		return -ENOMEM;
 
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 4098ed6ba6b4..6b3504bf7a42 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -36,11 +36,7 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt,
 void bdi_set_owner(struct backing_dev_info *bdi, struct device *owner);
 void bdi_unregister(struct backing_dev_info *bdi);
 
-struct backing_dev_info *bdi_alloc_node(gfp_t gfp_mask, int node_id);
-static inline struct backing_dev_info *bdi_alloc(gfp_t gfp_mask)
-{
-	return bdi_alloc_node(gfp_mask, NUMA_NO_NODE);
-}
+struct backing_dev_info *bdi_alloc(int node_id);
 
 void wb_start_background_writeback(struct bdi_writeback *wb);
 void wb_workfn(struct work_struct *work);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index bb993f99d424..1f55d5b8269f 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -865,12 +865,11 @@ static int bdi_init(struct backing_dev_info *bdi)
 	return ret;
 }
 
-struct backing_dev_info *bdi_alloc_node(gfp_t gfp_mask, int node_id)
+struct backing_dev_info *bdi_alloc(int node_id)
 {
 	struct backing_dev_info *bdi;
 
-	bdi = kmalloc_node(sizeof(struct backing_dev_info),
-			   gfp_mask | __GFP_ZERO, node_id);
+	bdi = kzalloc_node(sizeof(*bdi), GFP_KERNEL, node_id);
 	if (!bdi)
 		return NULL;
 
@@ -880,7 +879,7 @@ struct backing_dev_info *bdi_alloc_node(gfp_t gfp_mask, int node_id)
 	}
 	return bdi;
 }
-EXPORT_SYMBOL(bdi_alloc_node);
+EXPORT_SYMBOL(bdi_alloc);
 
 static struct rb_node **bdi_lookup_rb_node(u64 id, struct rb_node **parentp)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 8/8] bdi: remove the name field in struct backing_dev_info
  2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2020-04-16 16:54 ` [PATCH 7/8] bdi: simplify bdi_alloc Christoph Hellwig
@ 2020-04-16 16:54 ` Christoph Hellwig
  2020-04-18 15:45   ` Bart Van Assche
  7 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16 16:54 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

The name is only printed for a not registered bdi in writeback.  Use the
device name there as is more useful anyway for the unlike case that the
warning triggers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 block/blk-core.c                 | 1 -
 drivers/mtd/mtdcore.c            | 1 -
 fs/fs-writeback.c                | 2 +-
 fs/super.c                       | 2 --
 include/linux/backing-dev-defs.h | 2 --
 mm/backing-dev.c                 | 1 -
 6 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index ab87f2833ab2..f37068c611bf 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -494,7 +494,6 @@ struct request_queue *__blk_alloc_queue(int node_id)
 
 	q->backing_dev_info->ra_pages = VM_READAHEAD_PAGES;
 	q->backing_dev_info->capabilities = BDI_CAP_CGROUP_WRITEBACK;
-	q->backing_dev_info->name = "block";
 	q->node = node_id;
 
 	timer_setup(&q->backing_dev_info->laptop_mode_wb_timer,
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 39ec563d9a14..fcb018ce17c3 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -2040,7 +2040,6 @@ static struct backing_dev_info * __init mtd_bdi_init(char *name)
 	if (!bdi)
 		return ERR_PTR(-ENOMEM);
 
-	bdi->name = name;
 	/*
 	 * We put '-0' suffix to the name to get the same name format as we
 	 * used to get. Since this is called only once, we get a unique name. 
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 76ac9c7d32ec..d85323607b49 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2320,7 +2320,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 
 			WARN(bdi_cap_writeback_dirty(wb->bdi) &&
 			     !test_bit(WB_registered, &wb->state),
-			     "bdi-%s not registered\n", wb->bdi->name);
+			     "bdi-%s not registered\n", bdi_dev_name(wb->bdi));
 
 			inode->dirtied_when = jiffies;
 			if (dirtytime)
diff --git a/fs/super.c b/fs/super.c
index dd28fcd706ff..4991f441988e 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1602,8 +1602,6 @@ int super_setup_bdi_name(struct super_block *sb, char *fmt, ...)
 	if (!bdi)
 		return -ENOMEM;
 
-	bdi->name = sb->s_type->name;
-
 	va_start(args, fmt);
 	err = bdi_register_va(bdi, fmt, args);
 	va_end(args);
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 2849bdbb3acb..011bb8ad0738 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -194,8 +194,6 @@ struct backing_dev_info {
 	congested_fn *congested_fn; /* Function pointer if device is md/dm */
 	void *congested_data;	/* Pointer to aux data for congested func */
 
-	const char *name;
-
 	struct kref refcnt;	/* Reference counter for the structure */
 	unsigned int capabilities; /* Device capabilities */
 	unsigned int min_ratio;
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 1f55d5b8269f..d382272bcc31 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -15,7 +15,6 @@
 #include <trace/events/writeback.h>
 
 struct backing_dev_info noop_backing_dev_info = {
-	.name		= "noop",
 	.capabilities	= BDI_CAP_NO_ACCT_AND_WRITEBACK,
 };
 EXPORT_SYMBOL_GPL(noop_backing_dev_info);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-16 16:54 ` [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info Christoph Hellwig
@ 2020-04-17  8:59   ` Jan Kara
  2020-04-17 13:01     ` Christoph Hellwig
  2020-04-18 15:40     ` Bart Van Assche
  0 siblings, 2 replies; 30+ messages in thread
From: Jan Kara @ 2020-04-17  8:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block,
	linux-kernel

On Thu 16-04-20 18:54:48, Christoph Hellwig wrote:
> Cache a copy of the name for the life time of the backing_dev_info
> structure so that we can reference it even after unregistering.
> 
> Fixes: 68f23b89067f ("memcg: fix a crash in wb_workfn when a device disappears")
> Reported-by: Yufen Yu <yuyufen@huawei.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

...

> @@ -938,7 +938,8 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
>  	if (bdi->dev)	/* The driver needs to use separate queues per device */
>  		return 0;
>  
> -	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
> +	vsnprintf(bdi->dev_name, sizeof(bdi->dev_name), fmt, args);
> +	dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name);
>  	if (IS_ERR(dev))
>  		return PTR_ERR(dev);
>  

This can have a sideeffect not only bdi->dev_name will be truncated to 64
chars (which generally doesn't matter) but possibly also kobject name will
be truncated in the same way.  Which may have user visible effects. E.g.
for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
other way around - i.e., let device_create_vargs() create the device name
and then copy to bdi->dev_name whatever fits?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-17  8:59   ` Jan Kara
@ 2020-04-17 13:01     ` Christoph Hellwig
  2020-04-20 11:41       ` Hans de Goede
  2020-04-18 15:40     ` Bart Van Assche
  1 sibling, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-17 13:01 UTC (permalink / raw)
  To: Jan Kara
  Cc: axboe, yuyufen, tj, bvanassche, tytso, gregkh, linux-block,
	linux-kernel, Hans de Goede

On Fri, Apr 17, 2020 at 10:59:09AM +0200, Jan Kara wrote:
> > -	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
> > +	vsnprintf(bdi->dev_name, sizeof(bdi->dev_name), fmt, args);
> > +	dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name);
> >  	if (IS_ERR(dev))
> >  		return PTR_ERR(dev);
> >  
> 
> This can have a sideeffect not only bdi->dev_name will be truncated to 64
> chars (which generally doesn't matter) but possibly also kobject name will
> be truncated in the same way.  Which may have user visible effects. E.g.
> for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
> other way around - i.e., let device_create_vargs() create the device name
> and then copy to bdi->dev_name whatever fits?

I think having them mismatch is worse, as the kobject name is what
people look for.  Hans, do you know what fc->source typicall contains
and if there is much of a problem if it gets truncated/  Can we switch
to something else that is guranteed to be 64 charaters or less for the
bdi name?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/8] bdi: move bdi_dev_name out of line
  2020-04-16 16:54 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
@ 2020-04-18 15:35   ` Bart Van Assche
  0 siblings, 0 replies; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:35 UTC (permalink / raw)
  To: Christoph Hellwig, axboe
  Cc: yuyufen, tj, jack, tytso, gregkh, linux-block, linux-kernel

On 2020-04-16 09:54, Christoph Hellwig wrote:
> bdi_dev_name is not a fast path function, move it out of line.  This
> prepares for using it from modular callers without having to export
> an implementation detail like bdi_unknown_name.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/8] bdi: use bdi_dev_name() to get device name
  2020-04-16 16:54 ` [PATCH 2/8] bdi: use bdi_dev_name() to get device name Christoph Hellwig
@ 2020-04-18 15:37   ` Bart Van Assche
  0 siblings, 0 replies; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:37 UTC (permalink / raw)
  To: Christoph Hellwig, axboe
  Cc: yuyufen, tj, jack, tytso, gregkh, linux-block, linux-kernel

On 2020-04-16 09:54, Christoph Hellwig wrote:
> From: Yufen Yu <yuyufen@huawei.com>
> 
> Use the common interface bdi_dev_name() to get device name.
> 
> Signed-off-by: Yufen Yu <yuyufen@huawei.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-17  8:59   ` Jan Kara
  2020-04-17 13:01     ` Christoph Hellwig
@ 2020-04-18 15:40     ` Bart Van Assche
  2020-04-19  7:58       ` Christoph Hellwig
  1 sibling, 1 reply; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:40 UTC (permalink / raw)
  To: Jan Kara, Christoph Hellwig
  Cc: axboe, yuyufen, tj, tytso, gregkh, linux-block, linux-kernel

On 2020-04-17 01:59, Jan Kara wrote:
> On Thu 16-04-20 18:54:48, Christoph Hellwig wrote:
>> @@ -938,7 +938,8 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
>>  	if (bdi->dev)	/* The driver needs to use separate queues per device */
>>  		return 0;
>>  
>> -	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
>> +	vsnprintf(bdi->dev_name, sizeof(bdi->dev_name), fmt, args);
>> +	dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name);
>>  	if (IS_ERR(dev))
>>  		return PTR_ERR(dev);
>>  
> 
> This can have a sideeffect not only bdi->dev_name will be truncated to 64
> chars (which generally doesn't matter) but possibly also kobject name will
> be truncated in the same way.  Which may have user visible effects. E.g.
> for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
> other way around - i.e., let device_create_vargs() create the device name
> and then copy to bdi->dev_name whatever fits?

How about using kvasprintf() instead of vsnprintf()?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 5/8] bdi: unexport bdi_register_va
  2020-04-16 16:54 ` [PATCH 5/8] bdi: unexport bdi_register_va Christoph Hellwig
@ 2020-04-18 15:41   ` Bart Van Assche
  0 siblings, 0 replies; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:41 UTC (permalink / raw)
  To: Christoph Hellwig, axboe
  Cc: yuyufen, tj, jack, tytso, gregkh, linux-block, linux-kernel

On 2020-04-16 09:54, Christoph Hellwig wrote:
> bdi_register_va is only used by fs/super.c, which can't be modular.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 6/8] bdi: remove bdi_register_owner
  2020-04-16 16:54 ` [PATCH 6/8] bdi: remove bdi_register_owner Christoph Hellwig
@ 2020-04-18 15:43   ` Bart Van Assche
  0 siblings, 0 replies; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:43 UTC (permalink / raw)
  To: Christoph Hellwig, axboe
  Cc: yuyufen, tj, jack, tytso, gregkh, linux-block, linux-kernel

On 2020-04-16 09:54, Christoph Hellwig wrote:
> Split out a new bdi_set_owner helper to set the owner, and move the policy
> for creating the bdi name back into genhd.c, where it belongs.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 7/8] bdi: simplify bdi_alloc
  2020-04-16 16:54 ` [PATCH 7/8] bdi: simplify bdi_alloc Christoph Hellwig
@ 2020-04-18 15:44   ` Bart Van Assche
  0 siblings, 0 replies; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:44 UTC (permalink / raw)
  To: Christoph Hellwig, axboe
  Cc: yuyufen, tj, jack, tytso, gregkh, linux-block, linux-kernel

On 2020-04-16 09:54, Christoph Hellwig wrote:
> Merge the _node vs normal version and drop the superflous gfp_t argument.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 8/8] bdi: remove the name field in struct backing_dev_info
  2020-04-16 16:54 ` [PATCH 8/8] bdi: remove the name field in struct backing_dev_info Christoph Hellwig
@ 2020-04-18 15:45   ` Bart Van Assche
  0 siblings, 0 replies; 30+ messages in thread
From: Bart Van Assche @ 2020-04-18 15:45 UTC (permalink / raw)
  To: Christoph Hellwig, axboe
  Cc: yuyufen, tj, jack, tytso, gregkh, linux-block, linux-kernel

On 2020-04-16 09:54, Christoph Hellwig wrote:
> The name is only printed for a not registered bdi in writeback.  Use the
> device name there as is more useful anyway for the unlike case that the
> warning triggers.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-18 15:40     ` Bart Van Assche
@ 2020-04-19  7:58       ` Christoph Hellwig
  2020-04-19 15:29         ` Bart Van Assche
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-19  7:58 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jan Kara, Christoph Hellwig, axboe, yuyufen, tj, tytso, gregkh,
	linux-block, linux-kernel

On Sat, Apr 18, 2020 at 08:40:20AM -0700, Bart Van Assche wrote:
> > This can have a sideeffect not only bdi->dev_name will be truncated to 64
> > chars (which generally doesn't matter) but possibly also kobject name will
> > be truncated in the same way.  Which may have user visible effects. E.g.
> > for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
> > other way around - i.e., let device_create_vargs() create the device name
> > and then copy to bdi->dev_name whatever fits?
> 
> How about using kvasprintf() instead of vsnprintf()?

That is what v1 did, see the thread in response to that on why it isn't
a good idea.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-19  7:58       ` Christoph Hellwig
@ 2020-04-19 15:29         ` Bart Van Assche
  2020-04-19 16:06           ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Bart Van Assche @ 2020-04-19 15:29 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jan Kara, axboe, yuyufen, tj, tytso, gregkh, linux-block, linux-kernel

On 4/19/20 12:58 AM, Christoph Hellwig wrote:
> On Sat, Apr 18, 2020 at 08:40:20AM -0700, Bart Van Assche wrote:
>>> This can have a sideeffect not only bdi->dev_name will be truncated to 64
>>> chars (which generally doesn't matter) but possibly also kobject name will
>>> be truncated in the same way.  Which may have user visible effects. E.g.
>>> for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
>>> other way around - i.e., let device_create_vargs() create the device name
>>> and then copy to bdi->dev_name whatever fits?
>>
>> How about using kvasprintf() instead of vsnprintf()?
> 
> That is what v1 did, see the thread in response to that on why it isn't
> a good idea.

Are you perhaps referring to patch "[PATCH 3/8] bdi: add a ->dev_name 
field to struct backing_dev_info" 
(https://lore.kernel.org/linux-block/20200416071519.807660-4-hch@lst.de/) 
and also to the replies to that patch? This is what I found in the 
replies: "When driver try to to re-register bdi but without 
release_bdi(), the old dev_name will be cover directly by the newer in 
bdi_register_va(). So, I am not sure whether it can cause memory leak 
for bdi->dev_name."

Has it been considered to avoid that leak by freeing bdi->dev_name from 
unregister_bdi(), e.g. as follows?

void bdi_unregister(struct backing_dev_info *bdi)
{
+	char *dev_name;

	/* 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) {
		bdi_debug_unregister(bdi);
		device_unregister(bdi->dev);
		bdi->dev = NULL;
+		dev_name = bdi->dev_name;
+		bdi->dev_name = "(unregistered)";
+		kfree(dev_name);
	}

	if (bdi->owner) {
		put_device(bdi->owner);
		bdi->owner = NULL;
	}
}

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-19 15:29         ` Bart Van Assche
@ 2020-04-19 16:06           ` Christoph Hellwig
  2020-04-20  7:48             ` Christoph Hellwig
  2020-04-20  9:49             ` Jan Kara
  0 siblings, 2 replies; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-19 16:06 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Christoph Hellwig, Jan Kara, axboe, yuyufen, tj, tytso, gregkh,
	linux-block, linux-kernel

On Sun, Apr 19, 2020 at 08:29:21AM -0700, Bart Van Assche wrote:
> On 4/19/20 12:58 AM, Christoph Hellwig wrote:
>> On Sat, Apr 18, 2020 at 08:40:20AM -0700, Bart Van Assche wrote:
>>>> This can have a sideeffect not only bdi->dev_name will be truncated to 64
>>>> chars (which generally doesn't matter) but possibly also kobject name will
>>>> be truncated in the same way.  Which may have user visible effects. E.g.
>>>> for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
>>>> other way around - i.e., let device_create_vargs() create the device name
>>>> and then copy to bdi->dev_name whatever fits?
>>>
>>> How about using kvasprintf() instead of vsnprintf()?
>>
>> That is what v1 did, see the thread in response to that on why it isn't
>> a good idea.
>
> Are you perhaps referring to patch "[PATCH 3/8] bdi: add a ->dev_name field 
> to struct backing_dev_info" 
> (https://lore.kernel.org/linux-block/20200416071519.807660-4-hch@lst.de/) 
> and also to the replies to that patch? This is what I found in the replies: 
> "When driver try to to re-register bdi but without release_bdi(), the old 
> dev_name will be cover directly by the newer in bdi_register_va(). So, I am 
> not sure whether it can cause memory leak for bdi->dev_name."
>
> Has it been considered to avoid that leak by freeing bdi->dev_name from 
> unregister_bdi(), e.g. as follows?

We'd need some protection against concurrent accesses as unregister_bdi
can race with them.  But with RCU that could be handled, so let me try
that.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-19 16:06           ` Christoph Hellwig
@ 2020-04-20  7:48             ` Christoph Hellwig
  2020-04-20  9:52               ` Jan Kara
  2020-04-20  9:49             ` Jan Kara
  1 sibling, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-20  7:48 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Christoph Hellwig, Jan Kara, axboe, yuyufen, tj, tytso, gregkh,
	linux-block, linux-kernel

On Sun, Apr 19, 2020 at 06:06:51PM +0200, Christoph Hellwig wrote:
> > (https://lore.kernel.org/linux-block/20200416071519.807660-4-hch@lst.de/) 
> > and also to the replies to that patch? This is what I found in the replies: 
> > "When driver try to to re-register bdi but without release_bdi(), the old 
> > dev_name will be cover directly by the newer in bdi_register_va(). So, I am 
> > not sure whether it can cause memory leak for bdi->dev_name."
> >
> > Has it been considered to avoid that leak by freeing bdi->dev_name from 
> > unregister_bdi(), e.g. as follows?
> 
> We'd need some protection against concurrent accesses as unregister_bdi
> can race with them.  But with RCU that could be handled, so let me try
> that.

I looked into it, and while it seems doable I think this goes in the
wrong direction as it pushed the RCU knowledge into the callers.  I'd
rather get something like this series in ASAP, and then for 5.8 or 5.9
move the bdi pointer to the gendisk and stop re-registering it and thus
solve the problems root cause for real.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-19 16:06           ` Christoph Hellwig
  2020-04-20  7:48             ` Christoph Hellwig
@ 2020-04-20  9:49             ` Jan Kara
  1 sibling, 0 replies; 30+ messages in thread
From: Jan Kara @ 2020-04-20  9:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Bart Van Assche, Jan Kara, axboe, yuyufen, tj, tytso, gregkh,
	linux-block, linux-kernel

On Sun 19-04-20 18:06:51, Christoph Hellwig wrote:
> On Sun, Apr 19, 2020 at 08:29:21AM -0700, Bart Van Assche wrote:
> > On 4/19/20 12:58 AM, Christoph Hellwig wrote:
> >> On Sat, Apr 18, 2020 at 08:40:20AM -0700, Bart Van Assche wrote:
> >>>> This can have a sideeffect not only bdi->dev_name will be truncated to 64
> >>>> chars (which generally doesn't matter) but possibly also kobject name will
> >>>> be truncated in the same way.  Which may have user visible effects. E.g.
> >>>> for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
> >>>> other way around - i.e., let device_create_vargs() create the device name
> >>>> and then copy to bdi->dev_name whatever fits?
> >>>
> >>> How about using kvasprintf() instead of vsnprintf()?
> >>
> >> That is what v1 did, see the thread in response to that on why it isn't
> >> a good idea.
> >
> > Are you perhaps referring to patch "[PATCH 3/8] bdi: add a ->dev_name field 
> > to struct backing_dev_info" 
> > (https://lore.kernel.org/linux-block/20200416071519.807660-4-hch@lst.de/) 
> > and also to the replies to that patch? This is what I found in the replies: 
> > "When driver try to to re-register bdi but without release_bdi(), the old 
> > dev_name will be cover directly by the newer in bdi_register_va(). So, I am 
> > not sure whether it can cause memory leak for bdi->dev_name."
> >
> > Has it been considered to avoid that leak by freeing bdi->dev_name from 
> > unregister_bdi(), e.g. as follows?
> 
> We'd need some protection against concurrent accesses as unregister_bdi
> can race with them.  But with RCU that could be handled, so let me try
> that.

Yeah, that's what Yufen tried in his series some time ago and what I think
you personally didn't like :).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-20  7:48             ` Christoph Hellwig
@ 2020-04-20  9:52               ` Jan Kara
  0 siblings, 0 replies; 30+ messages in thread
From: Jan Kara @ 2020-04-20  9:52 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Bart Van Assche, Jan Kara, axboe, yuyufen, tj, tytso, gregkh,
	linux-block, linux-kernel

On Mon 20-04-20 09:48:01, Christoph Hellwig wrote:
> On Sun, Apr 19, 2020 at 06:06:51PM +0200, Christoph Hellwig wrote:
> > > (https://lore.kernel.org/linux-block/20200416071519.807660-4-hch@lst.de/) 
> > > and also to the replies to that patch? This is what I found in the replies: 
> > > "When driver try to to re-register bdi but without release_bdi(), the old 
> > > dev_name will be cover directly by the newer in bdi_register_va(). So, I am 
> > > not sure whether it can cause memory leak for bdi->dev_name."
> > >
> > > Has it been considered to avoid that leak by freeing bdi->dev_name from 
> > > unregister_bdi(), e.g. as follows?
> > 
> > We'd need some protection against concurrent accesses as unregister_bdi
> > can race with them.  But with RCU that could be handled, so let me try
> > that.
> 
> I looked into it, and while it seems doable I think this goes in the
> wrong direction as it pushed the RCU knowledge into the callers.  I'd
> rather get something like this series in ASAP, and then for 5.8 or 5.9
> move the bdi pointer to the gendisk and stop re-registering it and thus
> solve the problems root cause for real.

Yeah, if it could be done it would be a nice solution. Because
re-registering of BDIs is a long-term source of troubles...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-17 13:01     ` Christoph Hellwig
@ 2020-04-20 11:41       ` Hans de Goede
  2020-04-20 11:58         ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Hans de Goede @ 2020-04-20 11:41 UTC (permalink / raw)
  To: Christoph Hellwig, Jan Kara
  Cc: axboe, yuyufen, tj, bvanassche, tytso, gregkh, linux-block, linux-kernel

Hi,

<A lot of context has been trimmed here before I got added to the Cc, so
  I'm assuming that we are talking about the vboxsf code here.>

On 4/17/20 3:01 PM, Christoph Hellwig wrote:
> On Fri, Apr 17, 2020 at 10:59:09AM +0200, Jan Kara wrote:
>>> -	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
>>> +	vsnprintf(bdi->dev_name, sizeof(bdi->dev_name), fmt, args);
>>> +	dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name);
>>>   	if (IS_ERR(dev))
>>>   		return PTR_ERR(dev);
>>>   
>>
>> This can have a sideeffect not only bdi->dev_name will be truncated to 64
>> chars (which generally doesn't matter) but possibly also kobject name will
>> be truncated in the same way.  Which may have user visible effects. E.g.
>> for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
>> other way around - i.e., let device_create_vargs() create the device name
>> and then copy to bdi->dev_name whatever fits?
> 
> I think having them mismatch is worse, as the kobject name is what
> people look for.  Hans, do you know what fc->source typicall contains
> and if there is much of a problem if it gets truncated/  Can we switch
> to something else that is guranteed to be 64 charaters or less for the
> bdi name?

It contains the name the user has given to the shared-folder when
exporting it from the host/hypervisor. Typically this will be the
last element of the directory path, e.g. if I export /home/hans/projects/linux
then the default/suggested share name and this the source name to pass to
the host when mounting the shared-folder will be "linux". But the user can
put anything there.

AFAICT for vboxsf the bdi-name can be anything as long as it is unique, hence
the "vboxsf-" prefix to make this unique vs other block-devices and the
".%d" postfix is necessary because the same export can be mounted multiple
times (without using bind mounts), see:
https://github.com/jwrdegoede/vboxsf/issues/3

The presence of the source inside the bdi-name is only for informational
purposes really, so truncating that should be fine, maybe switch to:

"vboxsf%d-%s" as format string and swap the sbi->bdi_id and fc->source
in the args, then if we truncate anything it will be the source (which
as said is only there for informational purposes) and the name will
still be guaranteed to be unique.

Regards,

Hans


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-20 11:41       ` Hans de Goede
@ 2020-04-20 11:58         ` Christoph Hellwig
  2020-04-21 12:42           ` Hans de Goede
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-20 11:58 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Christoph Hellwig, Jan Kara, axboe, yuyufen, tj, bvanassche,
	tytso, gregkh, linux-block, linux-kernel

On Mon, Apr 20, 2020 at 01:41:57PM +0200, Hans de Goede wrote:
> AFAICT for vboxsf the bdi-name can be anything as long as it is unique, hence
> the "vboxsf-" prefix to make this unique vs other block-devices and the
> ".%d" postfix is necessary because the same export can be mounted multiple
> times (without using bind mounts), see:
> https://github.com/jwrdegoede/vboxsf/issues/3

Shouldn't vboxsf switch to get_tree_single instead of get_tree_nodev?
Having two independent dentry trees for a single actual file system
can be pretty dangerous.

>
> The presence of the source inside the bdi-name is only for informational
> purposes really, so truncating that should be fine, maybe switch to:
>
> "vboxsf%d-%s" as format string and swap the sbi->bdi_id and fc->source
> in the args, then if we truncate anything it will be the source (which
> as said is only there for informational purposes) and the name will
> still be guaranteed to be unique.

Can we just switch to vboxsf%d where %d іs a simple monotonically
incrementing count?  That is what various other file systems (e.g. ceph)
do.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info
  2020-04-20 11:58         ` Christoph Hellwig
@ 2020-04-21 12:42           ` Hans de Goede
  0 siblings, 0 replies; 30+ messages in thread
From: Hans de Goede @ 2020-04-21 12:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jan Kara, axboe, yuyufen, tj, bvanassche, tytso, gregkh,
	linux-block, linux-kernel

Hi,

On 4/20/20 1:58 PM, Christoph Hellwig wrote:
> On Mon, Apr 20, 2020 at 01:41:57PM +0200, Hans de Goede wrote:
>> AFAICT for vboxsf the bdi-name can be anything as long as it is unique, hence
>> the "vboxsf-" prefix to make this unique vs other block-devices and the
>> ".%d" postfix is necessary because the same export can be mounted multiple
>> times (without using bind mounts), see:
>> https://github.com/jwrdegoede/vboxsf/issues/3
> 
> Shouldn't vboxsf switch to get_tree_single instead of get_tree_nodev?
> Having two independent dentry trees for a single actual file system
> can be pretty dangerous.

That is a good point I will look into this.

> 
>>
>> The presence of the source inside the bdi-name is only for informational
>> purposes really, so truncating that should be fine, maybe switch to:
>>
>> "vboxsf%d-%s" as format string and swap the sbi->bdi_id and fc->source
>> in the args, then if we truncate anything it will be the source (which
>> as said is only there for informational purposes) and the name will
>> still be guaranteed to be unique.
> 
> Can we just switch to vboxsf%d where %d іs a simple monotonically
> incrementing count?  That is what various other file systems (e.g. ceph)
> do.

Yes that is fine with me.

Regards,

Hans


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/8] bdi: move bdi_dev_name out of line
  2020-04-16  7:15 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
  2020-04-16  7:52   ` Greg KH
@ 2020-04-16 12:32   ` Jan Kara
  1 sibling, 0 replies; 30+ messages in thread
From: Jan Kara @ 2020-04-16 12:32 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block,
	linux-kernel

On Thu 16-04-20 09:15:12, Christoph Hellwig wrote:
> bdi_dev_name is not a fast path function, move it out of line.  This
> prepares for using it from modular callers without having to export
> an implementation detail like bdi_unknown_name.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/backing-dev.h |  9 +--------
>  mm/backing-dev.c            | 10 +++++++++-
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index f88197c1ffc2..c9ad5c3b7b4b 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -505,13 +505,6 @@ static inline int bdi_rw_congested(struct backing_dev_info *bdi)
>  				  (1 << WB_async_congested));
>  }
>  
> -extern const char *bdi_unknown_name;
> -
> -static inline const char *bdi_dev_name(struct backing_dev_info *bdi)
> -{
> -	if (!bdi || !bdi->dev)
> -		return bdi_unknown_name;
> -	return dev_name(bdi->dev);
> -}
> +const char *bdi_dev_name(struct backing_dev_info *bdi);
>  
>  #endif	/* _LINUX_BACKING_DEV_H */
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index c81b4f3a7268..c2c44c89ee5d 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -21,7 +21,7 @@ struct backing_dev_info noop_backing_dev_info = {
>  EXPORT_SYMBOL_GPL(noop_backing_dev_info);
>  
>  static struct class *bdi_class;
> -const char *bdi_unknown_name = "(unknown)";
> +static const char *bdi_unknown_name = "(unknown)";
>  
>  /*
>   * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
> @@ -1043,6 +1043,14 @@ void bdi_put(struct backing_dev_info *bdi)
>  }
>  EXPORT_SYMBOL(bdi_put);
>  
> +const char *bdi_dev_name(struct backing_dev_info *bdi)
> +{
> +	if (!bdi || !bdi->dev)
> +		return bdi_unknown_name;
> +	return dev_name(bdi->dev);
> +}
> +EXPORT_SYMBOL_GPL(bdi_dev_name);
> +
>  static wait_queue_head_t congestion_wqh[2] = {
>  		__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[0]),
>  		__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[1])
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/8] bdi: move bdi_dev_name out of line
  2020-04-16  7:15 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
@ 2020-04-16  7:52   ` Greg KH
  2020-04-16 12:32   ` Jan Kara
  1 sibling, 0 replies; 30+ messages in thread
From: Greg KH @ 2020-04-16  7:52 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, yuyufen, tj, jack, bvanassche, tytso, linux-block, linux-kernel

On Thu, Apr 16, 2020 at 09:15:12AM +0200, Christoph Hellwig wrote:
> bdi_dev_name is not a fast path function, move it out of line.  This
> prepares for using it from modular callers without having to export
> an implementation detail like bdi_unknown_name.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/8] bdi: move bdi_dev_name out of line
  2020-04-16  7:15 bdi: fix use-after-free for dev_name(bdi->dev) Christoph Hellwig
@ 2020-04-16  7:15 ` Christoph Hellwig
  2020-04-16  7:52   ` Greg KH
  2020-04-16 12:32   ` Jan Kara
  0 siblings, 2 replies; 30+ messages in thread
From: Christoph Hellwig @ 2020-04-16  7:15 UTC (permalink / raw)
  To: axboe
  Cc: yuyufen, tj, jack, bvanassche, tytso, gregkh, linux-block, linux-kernel

bdi_dev_name is not a fast path function, move it out of line.  This
prepares for using it from modular callers without having to export
an implementation detail like bdi_unknown_name.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/backing-dev.h |  9 +--------
 mm/backing-dev.c            | 10 +++++++++-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index f88197c1ffc2..c9ad5c3b7b4b 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -505,13 +505,6 @@ static inline int bdi_rw_congested(struct backing_dev_info *bdi)
 				  (1 << WB_async_congested));
 }
 
-extern const char *bdi_unknown_name;
-
-static inline const char *bdi_dev_name(struct backing_dev_info *bdi)
-{
-	if (!bdi || !bdi->dev)
-		return bdi_unknown_name;
-	return dev_name(bdi->dev);
-}
+const char *bdi_dev_name(struct backing_dev_info *bdi);
 
 #endif	/* _LINUX_BACKING_DEV_H */
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index c81b4f3a7268..c2c44c89ee5d 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -21,7 +21,7 @@ struct backing_dev_info noop_backing_dev_info = {
 EXPORT_SYMBOL_GPL(noop_backing_dev_info);
 
 static struct class *bdi_class;
-const char *bdi_unknown_name = "(unknown)";
+static const char *bdi_unknown_name = "(unknown)";
 
 /*
  * bdi_lock protects bdi_tree and updates to bdi_list. bdi_list has RCU
@@ -1043,6 +1043,14 @@ void bdi_put(struct backing_dev_info *bdi)
 }
 EXPORT_SYMBOL(bdi_put);
 
+const char *bdi_dev_name(struct backing_dev_info *bdi)
+{
+	if (!bdi || !bdi->dev)
+		return bdi_unknown_name;
+	return dev_name(bdi->dev);
+}
+EXPORT_SYMBOL_GPL(bdi_dev_name);
+
 static wait_queue_head_t congestion_wqh[2] = {
 		__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[0]),
 		__WAIT_QUEUE_HEAD_INITIALIZER(congestion_wqh[1])
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2020-04-21 12:42 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 16:54 bdi: fix use-after-free for dev_name(bdi->dev) v2 Christoph Hellwig
2020-04-16 16:54 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
2020-04-18 15:35   ` Bart Van Assche
2020-04-16 16:54 ` [PATCH 2/8] bdi: use bdi_dev_name() to get device name Christoph Hellwig
2020-04-18 15:37   ` Bart Van Assche
2020-04-16 16:54 ` [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info Christoph Hellwig
2020-04-17  8:59   ` Jan Kara
2020-04-17 13:01     ` Christoph Hellwig
2020-04-20 11:41       ` Hans de Goede
2020-04-20 11:58         ` Christoph Hellwig
2020-04-21 12:42           ` Hans de Goede
2020-04-18 15:40     ` Bart Van Assche
2020-04-19  7:58       ` Christoph Hellwig
2020-04-19 15:29         ` Bart Van Assche
2020-04-19 16:06           ` Christoph Hellwig
2020-04-20  7:48             ` Christoph Hellwig
2020-04-20  9:52               ` Jan Kara
2020-04-20  9:49             ` Jan Kara
2020-04-16 16:54 ` [PATCH 4/8] driver core: remove device_create_vargs Christoph Hellwig
2020-04-16 16:54 ` [PATCH 5/8] bdi: unexport bdi_register_va Christoph Hellwig
2020-04-18 15:41   ` Bart Van Assche
2020-04-16 16:54 ` [PATCH 6/8] bdi: remove bdi_register_owner Christoph Hellwig
2020-04-18 15:43   ` Bart Van Assche
2020-04-16 16:54 ` [PATCH 7/8] bdi: simplify bdi_alloc Christoph Hellwig
2020-04-18 15:44   ` Bart Van Assche
2020-04-16 16:54 ` [PATCH 8/8] bdi: remove the name field in struct backing_dev_info Christoph Hellwig
2020-04-18 15:45   ` Bart Van Assche
  -- strict thread matches above, loose matches on Subject: below --
2020-04-16  7:15 bdi: fix use-after-free for dev_name(bdi->dev) Christoph Hellwig
2020-04-16  7:15 ` [PATCH 1/8] bdi: move bdi_dev_name out of line Christoph Hellwig
2020-04-16  7:52   ` Greg KH
2020-04-16 12:32   ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).