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

[Greg KH]

On Wed, Apr 29, 2020 at 07:46:25AM +0000, Luis Chamberlain wrote:
> --- a/block/blk-debugfs.c
> +++ b/block/blk-debugfs.c
> @@ -13,3 +13,32 @@ void blk_debugfs_register(void)
>  {
>  	blk_debugfs_root = debugfs_create_dir("block", NULL);
>  }
> +
> +static struct dentry *blk_debugfs_dir_register(const char *name)
> +{
> +	return debugfs_create_dir(name, blk_debugfs_root);
> +}

Nit, that function is not needed at all, just spell out the call to
debugfs_create_dir() in the 2 places below you call it.  That will
result in less lines of code overall :)

> -	dir = blk_trace_debugfs_dir(buts, bt);
> +	dir = blk_trace_debugfs_dir(bdev, q);
> +	if (WARN_ON(!dir))
> +		goto err;

With panic-on-warn you just rebooted the box, lovely :(

I said previously, that if you _REALLY_ wanted to warn about this, or do
something different based on the result of a debugfs call, then you can,
but you need to comment the heck out of it as to why you are doing so,
otherwise I'm just going to catch it in my tree-wide sweeps and end up
removing it.

Other than those two nits, this looks _much_ better, thanks for doing
this.

greg k-h