Re: [PATCH v7 6/8] blktrace: fix debugfs use after free
From: Bart Van Assche <bvanassche@acm.org>
Date: 2020-06-20 17:31:58
Also in:
linux-fsdevel, linux-mm, lkml
On 2020-06-19 13:47, Luis Chamberlain wrote:
This goes tested with:
^^^^
got?
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 7ff2ea5cd05e..e6e2d25fdbd6 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c@@ -524,10 +524,18 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, if (!bt->msg_data) goto err; - ret = -ENOENT; - - dir = debugfs_lookup(buts->name, blk_debugfs_root); - if (!dir) +#ifdef CONFIG_BLK_DEBUG_FS + /* + * When tracing whole make_request drivers (multiqueue) block devices, + * reuse the existing debugfs directory created by the block layer on + * init. For request-based block devices, all partitions block devices,
^^^^^^^^^^^^^^^^^^^^^ It seems like a word is missing from the comment? Or did you perhaps want to refer to "all partition block devices"?
+ * and scsi-generic block devices we create a temporary new debugfs + * directory that will be removed once the trace ends. + */ + if (queue_is_mq(q) && bdev && bdev == bdev->bd_contains) + dir = q->debugfs_dir; + else +#endif bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root);
Can it happen that two different threads each try to set up block tracing and hence that debugfs_create_dir() fails because a directory with name buts->name already exists?
quoted hunk ↗ jump to hunk
bt->dev = dev;@@ -565,8 +573,6 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, ret = 0; err: - if (dir && !bt->dir) - dput(dir); if (ret) blk_trace_free(bt); return ret;
Shouldn't bt->dir be removed in this error path for make_request drivers? Thanks, Bart.