Re: [PATCH] block: fix "Directory XXXXX with parent 'block' already present!"

[Ming Lei <ming.lei@redhat.com>]

On Thu, Apr 21, 2022 at 11:05:13PM -0700, Christoph Hellwig wrote:
> On Fri, Apr 22, 2022 at 11:01:43AM +0800, Ming Lei wrote:
> > Please see the following reasons:
> > 
> > 1) disk_release_mq() calls elevator_exit()/rq_qos_exit(), and the two
> > may trigger UAF if q->debugfs_dir is removed in blk_unregister_queue().
> 
> Well.  The debugfs_remove_recursive already removes all underlying
> entries, so the extra debugfs_remove_recursive calls there can just
> go away.
> > 
> > 2) after deleting disk, blktrace still should/can work for tracing
> > passthrough request.
> > 
> > 3) "debugfs directory deleted with blktrace active" in block/002 could
> > be triggered
> 
> Well, 3 just tests 2, so these are really one.
> 
> But how is blktrace supposed to work after the disk is torn down
> anyway? Pretty much all actual block trace traces reference the
> gendisk and/or block device which are getting freed at that point.
> So doing any blktrace action after the gendisk is released will
> lead to memory corruption.  For everyting but SCSI the race windows
> are probably small enough to not be seen by accident, but if you
> unbind the SSI ULP this should be fairly reproducible.

blktrace opens the bdev, so it is safe to trace until blktrace closes
the bdev. And del_gendisk() does happen before releasing disk, that
is why I don't think it is good to remove q->debugfs_dir inside
del_gendisk().


thanks,
Ming