linux-kernel - Re: [PATCH v5 5/7] blktrace: fix debugfs use after free

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]

Date:   Thu, 4 Jun 2020 21:48:43 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Luis Chamberlain <mcgrof@...nel.org>,
        Christoph Hellwig <hch@...radead.org>
Cc:     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, 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,
        Omar Sandoval <osandov@...com>,
        Hannes Reinecke <hare@...e.com>,
        Michal Hocko <mhocko@...nel.org>,
        syzbot+603294af2d01acfdd6da@...kaller.appspotmail.com
Subject: Re: [PATCH v5 5/7] blktrace: fix debugfs use after free

On 2020-06-01 10:05, Luis Chamberlain wrote:
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index a55cbfd060f5..5b0310f38e11 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -511,6 +511,11 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
>  	 */
>  	if (bdev && bdev != bdev->bd_contains) {
>  		dir = bdev->bd_part->debugfs_dir;
> +	} else if (q->sg_debugfs_dir &&
> +		   strlen(buts->name) == strlen(q->sg_debugfs_dir->d_name.name)
> +		   && strcmp(buts->name, q->sg_debugfs_dir->d_name.name) == 0) {
> +		/* scsi-generic requires use of its own directory */
> +		dir = q->sg_debugfs_dir;
>  	} else {
>  		/*
>  		 * For queues that do not have a gendisk attached to them, that
> 

Please Cc Martin Petersen for patches that modify SCSI code.

The string comparison check looks fragile to me. Is the purpose of that
check perhaps to verify whether tracing is being activated through the
SCSI generic interface? If so, how about changing that test into
something like the following?

	MAJOR(dev) == SCSI_GENERIC_MAJOR

Thanks,

Bart.