lkml.org 
[lkml]   [2020]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v5 5/7] blktrace: fix debugfs use after free
On Wed, May 27, 2020 at 03:12:02AM +0000, Luis Chamberlain wrote:
> You forgot to deal with partitions. Putting similar lipstick on the pig,
> this is what I end up with, let me know if this seems agreeable:

So even with the partition stuff in place, this approach still don't
allow multiple uses of blktrace against a scsi-generic device and its
backend real block device, say TYPE_DISK. A simple example is a scsi
drive hooked up used to allow users to do blktrace /dev/sda *and*
blktrace /dev/sg0, but with the proposed change /dev/sg0 no longer
works beacuse the dentry pertains to the '/dev/sda' name, not
'/dev/sg0'.

We can shoehorn in a solution following the style proposed as follows.
We can keep this only slightly cleaner if we don't care about the
extra dentry even if a user disables CONFIG_CHR_DEV_SG. The cost
would just be an extra dentry on the request_queue.

I'll run this through 0-day and then post a new hopefully final series,
but if you don't think this or would prefer something lease please let
me know.

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 86c107de2836..f46bdc7f6509 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -920,6 +920,9 @@ static void blk_release_queue(struct kobject *kobj)
blk_trace_shutdown(q);

debugfs_remove_recursive(q->debugfs_dir);
+#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE)
+ debugfs_remove_recursive(q->sg_debugfs_dir);
+#endif
if (queue_is_mq(q))
blk_mq_debugfs_unregister(q);

@@ -939,6 +942,21 @@ struct kobj_type blk_queue_ktype = {
.release = blk_release_queue,
};

+#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE)
+/**
+ * blk_sg_debugfs_init - initialize debugs for scsi-generic
+ * @q: the associated queue
+ * @name: name of the scsi-generic device
+ *
+ * To be used by scsi-generic for allowing it to use blktrace.
+ */
+void blk_sg_debugfs_init(struct request_queue *q, const char *name)
+{
+ q->sg_debugfs_dir = debugfs_create_dir(name, blk_debugfs_root);
+}
+EXPORT_SYMBOL_GPL(blk_sg_debugfs_init);
+#endif
+
/**
* blk_register_queue - register a block layer queue with sysfs
* @disk: Disk of which the request queue should be registered with sysfs.
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 20472aaaf630..c87fe1923f3d 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1519,6 +1519,7 @@ static int
sg_add_device(struct device *cl_dev, struct class_interface *cl_intf)
{
struct scsi_device *scsidp = to_scsi_device(cl_dev->parent);
+ struct request_queue *q = scsidp->request_queue;
struct gendisk *disk;
Sg_device *sdp = NULL;
struct cdev * cdev = NULL;
@@ -1573,6 +1574,7 @@ sg_add_device(struct device *cl_dev, struct class_interface *cl_intf)
} else
pr_warn("%s: sg_sys Invalid\n", __func__);

+ blk_sg_debugfs_init(q, disk->disk_name);
sdev_printk(KERN_NOTICE, scsidp, "Attached scsi generic sg%d "
"type %d\n", sdp->index, scsidp->type);

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 5877b03b8117..be5a40d59f60 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -575,6 +575,9 @@ struct request_queue {
struct bio_set bio_split;

struct dentry *debugfs_dir;
+#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE)
+ struct dentry *sg_debugfs_dir;
+#endif
#ifdef CONFIG_BLK_DEBUG_FS
struct dentry *sched_debugfs_dir;
struct dentry *rqos_debugfs_dir;
@@ -858,6 +861,14 @@ static inline void rq_flush_dcache_pages(struct request *rq)

extern int blk_register_queue(struct gendisk *disk);
extern void blk_unregister_queue(struct gendisk *disk);
+#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE)
+extern void blk_sg_debugfs_init(struct request_queue *q, const char *name);
+#else
+static inline void blk_sg_debugfs_init(struct request_queue *q,
+ const char *name)
+{
+}
+#endif
extern blk_qc_t generic_make_request(struct bio *bio);
extern blk_qc_t direct_make_request(struct bio *bio);
extern void blk_rq_init(struct request_queue *q, struct request *rq);
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index a55cbfd060f5..5b0310f38e11 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -511,6 +511,11 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
*/
if (bdev && bdev != bdev->bd_contains) {
dir = bdev->bd_part->debugfs_dir;
+ } else if (q->sg_debugfs_dir &&
+ strlen(buts->name) == strlen(q->sg_debugfs_dir->d_name.name)
+ && strcmp(buts->name, q->sg_debugfs_dir->d_name.name) == 0) {
+ /* scsi-generic requires use of its own directory */
+ dir = q->sg_debugfs_dir;
} else {
/*
* For queues that do not have a gendisk attached to them, that
\
 
 \ /
  Last update: 2020-06-01 19:06    [W:0.117 / U:0.852 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site