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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <38240225-e48e-3035-0baa-4929948b23a3@acm.org>
Date:   Sun, 19 Apr 2020 15:57:58 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Luis Chamberlain <mcgrof@...nel.org>, axboe@...nel.dk,
        viro@...iv.linux.org.uk, gregkh@...uxfoundation.org,
        rostedt@...dmis.org, mingo@...hat.com, jack@...e.cz,
        ming.lei@...hat.com, nstange@...e.de, akpm@...ux-foundation.org
Cc:     mhocko@...e.com, yukuai3@...wei.com, linux-block@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 08/10] blktrace: add checks for created debugfs files
 on setup

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@...nel.org>
> ---
>   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.