Re: [RFC 0/3] block: address blktrace use-after-free

[Luis Chamberlain]

On Fri, Apr 03, 2020 at 04:19:29PM +0800, Ming Lei wrote:
> On Wed, Apr 01, 2020 at 11:59:59PM +0000, Luis Chamberlain wrote:
> > Upstream kernel.org korg#205713 contends that there is a UAF in
> > the core debugfs debugfs_remove() function, and has gone through
> > pushing for a CVE for this, CVE-2019-19770.
> > 
> > If correct then parent dentries are not positive, and this would
> > have implications far beyond this bug report. Thankfully, upon review
> > with Nicolai, he wasn't buying it. His suspicions that this was just
> > a blktrace issue were spot on, and this patch series demonstrates
> > that, provides a reproducer, and provides a solution to the issue.
> > 
> > We there would like to contend CVE-2019-19770 as invalid. The
> > implications suggested are not correct, and this issue is only
> > triggerable with root, by shooting yourself on the foot by misuing
> > blktrace.
> > 
> > If you want this on a git tree, you can get it from linux-next
> > 20200401-blktrace-fix-uaf branch [2].
> > 
> > Wider review, testing, and rants are appreciated.
> > 
> > [0] https://bugzilla.kernel.org/show_bug.cgi?id=205713
> > [1] https://nvd.nist.gov/vuln/detail/CVE-2019-19770
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20200401-blktrace-fix-uaf
> > 
> > Luis Chamberlain (3):
> >   block: move main block debugfs initialization to its own file
> >   blktrace: fix debugfs use after free
> >   block: avoid deferral of blk_release_queue() work
> > 
> >  block/Makefile               |  1 +
> >  block/blk-core.c             |  9 +--------
> >  block/blk-debugfs.c          | 27 +++++++++++++++++++++++++++
> >  block/blk-mq-debugfs.c       |  5 -----
> >  block/blk-sysfs.c            | 21 ++++++++-------------
> >  block/blk.h                  | 17 +++++++++++++++++
> >  include/linux/blktrace_api.h |  1 -
> >  kernel/trace/blktrace.c      | 19 ++++++++-----------
> >  8 files changed, 62 insertions(+), 38 deletions(-)
> >  create mode 100644 block/blk-debugfs.c
> 
> BTW, Yu Kuai posted one patch for this issue, looks that approach
> is simpler:
> 
> https://lore.kernel.org/linux-block/20200324132315.22133-1-yukuai3@xxxxxxxxxx/

I cannot see how renaming the possible target directory to a temporary
directory instead of unifying it for both SQ and MQ could be any
simpler. IMHO this keeps the mess and fragile nature of the issue.

The approach taken here unifies the directory we should use for both SQ
and MQ and makes the deferral issue a completely separate issue
addressed in the last patch.

  Luis