[PATCH 2/4] nvme: fix reset uninitialized controller

Taehee Yoo
2023-01-03T10:15:40Z

nvme-fabric controllers can be reset by
/sys/class/nvme/nvme#/reset_controller
echo 1 > /sys/class/nvme/nvme#/reset_controller
The above command will call nvme_sysfs_reset().

This function internally calls ctrl->reset_work synchronously or
asynchronously.
At this point, it doesn't sure if the controller will be reset after
initialization.

So kernel panic would occur because ctrl->reset_work dereferences
uninitialized values.

In order to avoid this, nvme_sysfs_reset checks
the NVME_CTRL_STARTED_ONCE flag. This flag indicates the controller is
initialized fully. So, reset logic can be executed safely.

WARNING: CPU: 1 PID: 462 at kernel/workqueue.c:3066 __flush_work+0x74f/0x960
Modules linked in: nvme_tcp xt_conntrack xt_MASQUERADE nf_conntrack_netlink xfrm_user xt_addrtype iptable_filter iptable_nat br_netfilter bridge stp llc crct10dif_pclmul crc32_generic crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 overlay openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 mlx5_ib ib_uverbs ib_core xts cts ecb mlx5_core aesni_intel crypto_simd cryptd mlxfw ptp sch_fq_codel nf_tables nfnetlink ip_tables x_tables unix
CPU: 1 PID: 462 Comm: kworker/u16:5 Not tainted 6.1.0+ #52 1d16bdc3867491ba5cf2147d49bd76d7eacb8fd9
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
Workqueue: nvme-reset-wq nvme_reset_ctrl_work [nvme_tcp]
RIP: 0010:__flush_work+0x74f/0x960
Code: c0 74 6c e8 53 97 17 00 48 c7 c6 c8 4a 1b 84 48 c7 c7 80 70 b9 8e 45 31 f6 e8 cd 53 0f 00 e9 5d fd ff ff 0f 0b e9 56 fd ff ff <0f> 0b 45 31 f6 e9 4c fd ff ff 4c 89 ef e8 4f 81 2a 02 e8 ea 75 16
RSP: 0018:ffff888116507a50 EFLAGS: 00010246
RAX: ffff88800646b490 RBX: 0000000000000011 RCX: 1ffffffff1e59f99
RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffff88800646b490
RBP: ffff888116507be8 R08: 0000000000000000 R09: 0000000000000000
R10: ffffed1000c8d692 R11: 0000000000000001 R12: 1ffff11022ca0f50
R13: 1ffff11022ca0f80 R14: 0000000000000001 R15: ffff88800646b4a8
FS: 0000000000000000(0000) GS:ffff888117a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055953af11fd0 CR3: 0000000106f62001 CR4: 00000000003706e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
? _raw_spin_unlock_irqrestore+0x59/0x70
? queue_delayed_work_on+0xa0/0xa0
? lock_release+0x631/0xe80
? __up_read+0x192/0x730
? up_write+0x520/0x520
? rcu_read_lock_sched_held+0x12/0x80
? lock_release+0x631/0xe80
? rcu_read_lock_sched_held+0x12/0x80
? try_to_grab_pending.part.0+0x23/0x540
__cancel_work_timer+0x2cb/0x3f0
? cancel_delayed_work+0x10/0x10
? rcu_read_lock_sched_held+0x12/0x80
? lock_acquire+0x4f4/0x630
? lockdep_hardirqs_on_prepare+0x410/0x410
? lock_downgrade+0x700/0x700
? finish_task_switch.isra.0+0x23b/0x870
? trace_hardirqs_on+0x3c/0x190
nvme_stop_ctrl+0x17/0x150
nvme_reset_ctrl_work+0x19/0x120 [nvme_tcp aa1d0deebfd175637ed368a54a16dfbb09be290f]
[ ... ]

Fixes: f3ca80fc11c3 ("nvme: move chardev and sysfs interface to common code")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/nvme/host/core.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index cd4c80ca66d4..418bd865c838 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -134,6 +134,14 @@ void nvme_queue_scan(struct nvme_ctrl *ctrl)
queue_work(nvme_wq, &ctrl->scan_work);
}

+void nvme_queue_scan_sync(struct nvme_ctrl *ctrl)
+{
+ if (ctrl->state == NVME_CTRL_LIVE && ctrl->tagset) {
+ queue_work(nvme_wq, &ctrl->scan_work);
+ flush_work(&ctrl->scan_work);
+ }
+}
+
/*
* Use this function to proceed with scheduling reset_work for a controller
* that had previously been set to the resetting state. This is intended for
@@ -1150,10 +1158,8 @@ void nvme_passthru_end(struct nvme_ctrl *ctrl, u32 effects,
dev_info(ctrl->device,
"controller capabilities changed, reset may be required to take effect.\n");
}
- if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC)) {
- nvme_queue_scan(ctrl);
- flush_work(&ctrl->scan_work);
- }
+ if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC))
+ nvme_queue_scan_sync(ctrl);

switch (cmd->common.opcode) {
case nvme_admin_set_features:
@@ -3350,9 +3356,11 @@ static ssize_t nvme_sysfs_reset(struct device *dev,
struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
int ret;

- ret = nvme_reset_ctrl_sync(ctrl);
- if (ret < 0)
- return ret;
+ if (test_bit(NVME_CTRL_STARTED_ONCE, &ctrl->flags)) {
+ ret = nvme_reset_ctrl_sync(ctrl);
+ if (ret < 0)
+ return ret;
+ }
return count;
}
static DEVICE_ATTR(reset_controller, S_IWUSR, NULL, nvme_sysfs_reset);
@@ -4994,16 +5002,18 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl)
* that were missed. We identify persistent discovery controllers by
* checking that they started once before, hence are reconnecting back.
*/
- if (test_and_set_bit(NVME_CTRL_STARTED_ONCE, &ctrl->flags) &&
+ if (test_bit(NVME_CTRL_STARTED_ONCE, &ctrl->flags) &&
nvme_discovery_ctrl(ctrl))
nvme_change_uevent(ctrl, "NVME_EVENT=rediscover");

if (ctrl->queue_count > 1) {
- nvme_queue_scan(ctrl);
+ nvme_queue_scan_sync(ctrl);
nvme_unquiesce_io_queues(ctrl);
nvme_mpath_update(ctrl);
}

+ set_bit(NVME_CTRL_STARTED_ONCE, &ctrl->flags);
+
nvme_change_uevent(ctrl, "NVME_EVENT=connected");
}
EXPORT_SYMBOL_GPL(nvme_start_ctrl);
--
2.34.1