linux-kernel - Re: [PATCH v6 6/6] 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:   Wed, 10 Jun 2020 22:40:05 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Christoph Hellwig <hch@...radead.org>, Jan Kara <jack@...e.cz>,
        axboe@...nel.dk, viro@...iv.linux.org.uk, bvanassche@....org,
        gregkh@...uxfoundation.org, rostedt@...dmis.org, mingo@...hat.com,
        ming.lei@...hat.com, nstange@...e.de, akpm@...ux-foundation.org,
        mhocko@...e.com, yukuai3@...wei.com, martin.petersen@...cle.com,
        jejb@...ux.ibm.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 v6 6/6] blktrace: fix debugfs use after free

On Wed, Jun 10, 2020 at 11:31:16PM +0000, Luis Chamberlain wrote:
> On Wed, Jun 10, 2020 at 09:52:13PM +0000, Luis Chamberlain wrote:
> > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> > index 7ff2ea5cd05e..5cea04c05e09 100644
> > --- a/kernel/trace/blktrace.c
> > +++ b/kernel/trace/blktrace.c
> > @@ -524,10 +524,16 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
> >  	if (!bt->msg_data)
> >  		goto err;
> >  
> > -	ret = -ENOENT;
> > -
> > -	dir = debugfs_lookup(buts->name, blk_debugfs_root);
> > -	if (!dir)
> > +	/*
> > +	 * When tracing whole make_request drivers (multiqueue) block devices,
> > +	 * reuse the existing debugfs directory created by the block layer on
> > +	 * init. For request-based block devices, all partitions block devices,
> > +	 * and scsi-generic block devices we create a temporary new debugfs
> > +	 * directory that will be removed once the trace ends.
> > +	 */
> > +	if (queue_is_mq(q))
> 
> And this should be instead:
> 
> if (queue_is_mq(q) && bdev && bdev == bdev->bd_contains)

Yes.  But I think keeping the queue_is_mq out and always creating the
debugfs dir is an improvement as we'll sooner or later grow more debugfs
files and than the whole set of problmes reappears.  But I'd be fine
with doing the above in the first patch that is tiny and backportable,
and then have another patch that always creates the debugfs directory.