lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 20 May 2022 21:56:35 +0800
From:   Ming Lei <ming.lei@...hat.com>
To:     "yukuai (C)" <yukuai3@...wei.com>
Cc:     axboe@...nel.dk, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, yi.zhang@...wei.com,
        ming.lei@...hat.com
Subject: Re: [PATCH -next v2] blk-mq: fix panic during blk_mq_run_work_fn()

On Fri, May 20, 2022 at 08:01:31PM +0800, yukuai (C) wrote:
> 在 2022/05/20 19:39, Ming Lei 写道:
> 
> > 
> > In short:
> > 
> > 1) run queue can be in-progress during cleanup queue, or returns from
> > cleanup queue; we drain it in both blk_cleanup_queue() and
> > disk_release_mq(), see commit 2a19b28f7929 ("blk-mq: cancel blk-mq dispatch
> > work in both blk_cleanup_queue and disk_release()")
> I understand that, however, there is no garantee new 'hctx->run_work'
> won't be queued after 'drain it', for this crash, I think this is how

No, run queue activity will be shutdown after both disk_release_mq()
and blk_cleanup_queue() are done.

disk_release_mq() is called after all FS IOs are done, so there isn't
any run queue from FS IO code path, either sync or async.

In blk_cleanup_queue(), we only focus on passthrough request, and
passthrough request is always explicitly allocated & freed by
its caller, so once queue is frozen, all sync dispatch activity
for passthrough request has been done, then it is enough to just cancel
dispatch work for avoiding any dispatch activity.

That is why both request queue and hctx can be released safely
after the two are done.

> it triggered:
> 
> assum that there is no io, while some bfq_queue is still busy:
> 
> blk_cleanup_queue
>  blk_freeze_queue
>  blk_mq_cancel_work_sync
>  cancel_delayed_work_sync(hctx1)
> 				blk_mq_run_work_fn -> hctx2
> 				 __blk_mq_run_hw_queue
> 				  blk_mq_sched_dispatch_requests
> 				   __blk_mq_do_dispatch_sched
> 				    blk_mq_delay_run_hw_queues
> 				     blk_mq_delay_run_hw_queue
> 				      -> add hctx1->run_work again
>  cancel_delayed_work_sync(hctx2)

Yes, even blk_mq_delay_run_hw_queues() can be called after all
hctx->run_work are canceled since __blk_mq_run_hw_queue() could be
running in sync io code path, not via ->run_work.

And my patch will fix the issue, won't it?


Thanks,
Ming

Powered by blists - more mailing lists