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

[Greg KH]

On Sun, Apr 19, 2020 at 07:45:27PM +0000, 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;

No, this is wrong, please do not ever check the return value of a
debugfs call.  See the zillions of patches I've been doing to the kernel
for this type of thing over the past year for examples of why.

the code is fine as-is.

greg k-h