linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>, Jan Kara <jack@suse.cz>,
	axboe@kernel.dk, viro@zeniv.linux.org.uk, bvanassche@acm.org,
	gregkh@linuxfoundation.org, rostedt@goodmis.org,
	mingo@redhat.com, ming.lei@redhat.com, nstange@suse.de,
	akpm@linux-foundation.org, mhocko@suse.com, yukuai3@huawei.com,
	martin.petersen@oracle.com, jejb@linux.ibm.com,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Omar Sandoval <osandov@fb.com>, Hannes Reinecke <hare@suse.com>,
	Michal Hocko <mhocko@kernel.org>,
	syzbot+603294af2d01acfdd6da@syzkaller.appspotmail.com
Subject: Re: [PATCH v6 6/6] blktrace: fix debugfs use after free
Date: Tue, 9 Jun 2020 10:32:18 -0700	[thread overview]
Message-ID: <20200609173218.GA7968@infradead.org> (raw)
In-Reply-To: <20200609172922.GP11244@42.do-not-panic.com>

On Tue, Jun 09, 2020 at 05:29:22PM +0000, Luis Chamberlain wrote:
> Is scsi-generic is the only unwanted ugly child blktrace has to deal
> with? For some reason I thought drivers/md/md.c was one but it seems
> like it is not. Do we have an easy way to search for these? I think
> this would just affect how we express the comment only.

grep for blk_trace_setup.  For all blk devices that setup comes in
through the block device ioctl path, and that relies on having a
struct block_device and queue.  sg on the other hand calls
blk_trace_setup directly with a NULL bdev argument.

> >  		 */
> > -		dir = q->sg_debugfs_dir;
> > +		dir = debugfs_create_dir(buts->name, blk_debugfs_root);
> > +		bt->dir = dir;
> 
> The other chicken and egg problem to consider at least in the comments
> is that the debugfs directory for these types of devices *have* an
> exposed path, but the data structure is rather opaque to the device and
> even blktrace.  Fortunately given the recent set of changes around the
> q->blk_trace and clarifications around its use we have made it clear now
> that so long as hold the q->blk_trace_mutex *and* check q->blk_trace we
> *should* not race against two separate creations of debugfs directories,
> so I think this is safe, so long as these indpendent drivers don't end
> up re-using the same path for some other things later in the future, and
> since we have control over what goes under debugfsroot block / I think
> we should be good.
> 
> But I think that the concern for race on names may still be worth
> explaining a bit here.

Feel free to add more comments, but please try to keep them short
and crisp.  At the some point long comments really distract from what
is going on.


  reply	other threads:[~2020-06-09 17:32 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-08 17:01 [PATCH v6 0/8] block: fix blktrace " Luis Chamberlain
2020-06-08 17:01 ` [PATCH v6 1/6] block: add docs for gendisk / request_queue refcount helpers Luis Chamberlain
2020-06-08 17:01 ` [PATCH v6 2/6] block: clarify context for refcount increment helpers Luis Chamberlain
2020-06-08 17:01 ` [PATCH v6 3/6] block: revert back to synchronous request_queue removal Luis Chamberlain
2020-06-13  1:53   ` Bart Van Assche
2020-06-19 20:23     ` Luis Chamberlain
2020-06-08 17:01 ` [PATCH v6 4/6] blktrace: annotate required lock on do_blk_trace_setup() Luis Chamberlain
2020-06-09 14:18   ` Christoph Hellwig
2020-06-13  1:54   ` Bart Van Assche
2020-06-08 17:01 ` [PATCH v6 5/6] loop: be paranoid on exit and prevent new additions / removals Luis Chamberlain
2020-06-08 17:01 ` [PATCH v6 6/6] blktrace: fix debugfs use after free Luis Chamberlain
2020-06-09 15:06   ` Christoph Hellwig
2020-06-09 17:29     ` Luis Chamberlain
2020-06-09 17:32       ` Christoph Hellwig [this message]
2020-06-09 17:53         ` Luis Chamberlain
2020-06-10  6:42           ` Christoph Hellwig
2020-06-10 21:09             ` Luis Chamberlain
2020-06-10 21:52               ` Luis Chamberlain
2020-06-10 23:31                 ` Luis Chamberlain
2020-06-11  5:40                   ` Christoph Hellwig
2020-06-13  2:42   ` Bart Van Assche
2020-06-19 15:36     ` Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200609173218.GA7968@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hare@suse.com \
    --cc=jack@suse.cz \
    --cc=jejb@linux.ibm.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=martin.petersen@oracle.com \
    --cc=mcgrof@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mhocko@suse.com \
    --cc=ming.lei@redhat.com \
    --cc=mingo@redhat.com \
    --cc=nstange@suse.de \
    --cc=osandov@fb.com \
    --cc=rostedt@goodmis.org \
    --cc=syzbot+603294af2d01acfdd6da@syzkaller.appspotmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yukuai3@huawei.com \
    --subject='Re: [PATCH v6 6/6] blktrace: fix debugfs use after free' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).