Re: [PATCH] block: fix NPE when resuming SCSI devices using blk-mq
From: Patrick Steinhardt <hidden>
Date: 2018-07-16 15:21:08
Also in:
lkml
On Fri, Jul 13, 2018 at 09:41:41PM +0800, Ming Lei wrote:
On Fri, Jul 13, 2018 at 9:29 PM, Patrick Steinhardt [off-list ref] wrote:quoted
When power management for SCSI is enabled and if a device uses blk-mq, it is possible to trigger a `NULL` pointer exception when resuming that device. The NPE is triggered when trying to dereference the `request_fn` function pointer of the device's `request_queue`: __blk_run_queue_uncond:470 __blk_run_queue:490 blk_post_runtime_resume:3889 sdev_runtime_resume:263 scsi_runtime_resume:275 When the SCSI device is being allocated by `scsi_alloc_sdev`, the device's request queue will either be initialized via `scsi_mq_alloc_queue` or `scsi_old_alloc_queue`. But the `request_fn` member of the request queue is in fact only being set in `scsi_old_alloc_queue`, which will then later cause the mentioned NPE. Fix the issue by checking whether the `request_fn` is set in `__blk_run_queue_uncond`. In case it is unset, we'll silently return and not try to invoke the callback, thus fixing the NPE. Signed-off-by: Patrick Steinhardt <redacted> --- Since at least v4.14, I am easily able to trigger above NPE by unplugging USB mass storage devices on my computer (Skylake, ASUS Z170I) with CONFIG_SCSI_MQ_DEFAULT=y. The attached patch fixes the issue, but keep in mind that this is my first patch, so the proposed fix may not be appropriate at all. Feedback would be highly appreciated. block/blk-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/block/blk-core.c b/block/blk-core.c index f84a9b7b6f5a..0a2041660cd9 100644 --- a/block/blk-core.c +++ b/block/blk-core.c@@ -456,7 +456,7 @@ inline void __blk_run_queue_uncond(struct request_queue *q) lockdep_assert_held(q->queue_lock); WARN_ON_ONCE(q->mq_ops); - if (unlikely(blk_queue_dead(q))) + if (unlikely(!q->request_fn) || unlikely(blk_queue_dead(q))) return;Now runtime PM is disabled for blk-mq/scsi_mq, not sure how this issue is triggered on your machine. Could you share the steps for reproducing this issue?
I bet that the issue stems from custom hotplugging scripts then, which change the value of power/control. See the attachment of this mail for all sysfs changes that are being performed after plugging in the USB stick. Basically, the reproduction steps on my machine are: 1. plug in USB stick (assumed to be /dev/sdb now) 2. wait a short amount of time 3. dd if=/dev/sdb of=/dev/null bs=4M 4. wait a short amount of time 5. unplug the USB stick, which immediately crashes the system The issue isn't always reproducible, I think there is some variance depending on how much time passes by at step 2 and/or 4. It probably is related to the autosuspend delay. From my experience the crash becomes more likely the longer I wait after step 4. Regards Patrick
Attachments
- signature.asc [application/pgp-signature] 833 bytes