On Thu, Apr 13, 2017 at 11:05:32PM +0000, Bart Van Assche wrote:
> On Thu, 2017-04-13 at 16:01 -0700, Omar Sandoval wrote:
> > On Tue, Apr 11, 2017 at 01:58:37PM -0700, Bart Van Assche wrote:
> > > The blk-mq debugfs attributes are removed after blk_cleanup_queue()
> > > has finished. Since running a queue after a queue has entered the
> > > "dead" state is not allowed, disallow this. This patch avoids that
> > > an attempt to run a dead queue triggers a kernel crash.
> > >
> > > Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> > > Cc: Omar Sandoval <osandov@xxxxxx>
> > > Cc: Hannes Reinecke <hare@xxxxxxxx>
> > > ---
> > > block/blk-mq-debugfs.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > > index df9b688b877c..a1ce823578c7 100644
> > > --- a/block/blk-mq-debugfs.c
> > > +++ b/block/blk-mq-debugfs.c
> > > @@ -111,6 +111,14 @@ static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
> > > struct request_queue *q = file_inode(file)->i_private;
> > > char op[16] = { }, *s;
> > >
> > > + /*
> > > + * The debugfs attributes are removed after blk_cleanup_queue() has
> > > + * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set
> > > + * to avoid triggering a use-after-free.
> > > + */
> > > + if (blk_queue_dead(q))
> > > + return -ENOENT;
> > > +
> > > len = min(len, sizeof(op) - 1);
> > > if (copy_from_user(op, ubuf, len))
> > > return -EFAULT;
> >
> > Looking at this, I think we have similar issues with most of the other
> > debugfs files. Should we move the debugfs cleanup earlier?
>
> Hello Omar,
>
> That's a good question. However, while I was debugging it was very convenient
> to be able to access the queue state after it had reached the "dead" state.
> Performing the cleanup earlier would be an alternative solution but would
> make debugging a bit harder ...
>
> Bart.
What useful information were you getting out of debugfs once the queue
was already dead? Wasn't the interesting stuff freed at that point?
