Re: [PATCH v2 08/10] blktrace: add checks for created debugfs files on setup

[Bart Van Assche]

On 4/19/20 12:45 PM, Luis Chamberlain wrote:
> Even though debugfs can be disabled, enabling BLK_DEV_IO_TRACE will
> select DEBUG_FS, and blktrace exposes an API which userspace uses
> relying on certain files created in debugfs. If files are not created
> blktrace will not work correctly, so we do want to ensure that a
> blktrace setup creates these files properly, and otherwise inform
> userspace.
>
> Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> ---
>   kernel/trace/blktrace.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 9cc0153849c3..fc32a8665ce8 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -552,17 +552,19 @@ static int blk_trace_create_debugfs_files(struct blk_user_trace_setup *buts,
>   					  struct dentry *dir,
>   					  struct blk_trace *bt)
>   {
> -	int ret = -EIO;
> -
>   	bt->dropped_file = debugfs_create_file("dropped", 0444, dir, bt,
>   					       &blk_dropped_fops);
> +	if (!bt->dropped_file)
> +		return -ENOMEM;
>   
>   	bt->msg_file = debugfs_create_file("msg", 0222, dir, bt, &blk_msg_fops);
> +	if (!bt->msg_file)
> +		return -ENOMEM;
>   
>   	bt->rchan = relay_open("trace", dir, buts->buf_size,
>   				buts->buf_nr, &blk_relay_callbacks, bt);
>   	if (!bt->rchan)
> -		return ret;
> +		return -EIO;
>   
>   	return 0;
>   }

I should have had a look at this patch before I replied to the previous 
patch.

Do you agree that the following code can be triggered by 
debugfs_create_file() and also that debugfs_create_file() never returns 
NULL?

static struct dentry *failed_creating(struct dentry *dentry)
{
	inode_unlock(d_inode(dentry->d_parent));
	dput(dentry);
	simple_release_fs(&debugfs_mount, &debugfs_mount_count);
	return ERR_PTR(-ENOMEM);
}

Thanks,

Bart.