Re: [PATCH v3 4/6] blktrace: fix debugfs use after free

On Wed, Apr 29, 2020 at 04:50:51AM -0700, Christoph Hellwig wrote:

On Wed, Apr 29, 2020 at 11:45:42AM +0000, Luis Chamberlain wrote:

quoted

On Wed, Apr 29, 2020 at 04:26:37AM -0700, Christoph Hellwig wrote:

quoted

I can't say I'm a fan of all these long backtraces in commit logs..

quoted

+static struct dentry *blk_debugfs_dir_register(const char *name)
+{
+	return debugfs_create_dir(name, blk_debugfs_root);
+}

I don't think we really need this helper.

We don't export blk_debugfs_root, didn't think we'd want to, and
since only a few scew funky drivers would use the struct gendisk
and also support BLKTRACE, I didn't think we'd want to export it
now.

A new block private symbol namespace alright?

Err, that function is static and has two callers.

Yes but that is to make it easier to look for who is creating the
debugfs_dir for either the request_queue or partition. I'll export
blk_debugfs_root and we'll open code all this.

quoted

quoted

This could be simplified down to:

	if (bdev && bdev != bdev->bd_contains)
		return bdev->bd_part->debugfs_dir;
	return q->debugfs_dir;

Given that bd_part is in __blkdev_get very near bd_contains.

Ah neat.

quoted

Also given that this patch completely rewrites blk_trace_debugfs_dir is
there any point in the previous patch?

Still think it helps with making this patch easier to read, but I don't
care, lemme know if I should just fold it.

In fact I'm not even sure we need the helper.  Modulo the comment
this just becomes a:

	if (bdev && bdev != bdev->bd_contains)
 		dir = bdev->bd_part->debugfs_dir;
	else
	 	dir = q->debugfs_dir;

in do_blk_trace_setup.

True, alright will remove that patch.

  Luis