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.
