Skip to content

Commit 846eb7c

Browse files
CIQ Kernel Automationshreeya-patel98
authored andcommitted
nvme-fc: do not wait in vain when unloading module
jira VULN-37041 cve CVE-2024-26846 commit-author Daniel Wagner <dwagner@suse.de> commit 70fbfc4 The module exit path has race between deleting all controllers and freeing 'left over IDs'. To prevent double free a synchronization between nvme_delete_ctrl and ida_destroy has been added by the initial commit. There is some logic around trying to prevent from hanging forever in wait_for_completion, though it does not handling all cases. E.g. blktests is able to reproduce the situation where the module unload hangs forever. If we completely rely on the cleanup code executed from the nvme_delete_ctrl path, all IDs will be freed eventually. This makes calling ida_destroy unnecessary. We only have to ensure that all nvme_delete_ctrl code has been executed before we leave nvme_fc_exit_module. This is done by flushing the nvme_delete_wq workqueue. While at it, remove the unused nvme_fc_wq workqueue too. Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Daniel Wagner <dwagner@suse.de> Signed-off-by: Keith Busch <kbusch@kernel.org> (cherry picked from commit 70fbfc4) Signed-off-by: CIQ Kernel Automation <ciq_kernel_automation@ciq.com>
1 parent 089c55a commit 846eb7c

1 file changed

Lines changed: 6 additions & 41 deletions

File tree

  • drivers/nvme/host

drivers/nvme/host/fc.c

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,6 @@ static LIST_HEAD(nvme_fc_lport_list);
221221
static DEFINE_IDA(nvme_fc_local_port_cnt);
222222
static DEFINE_IDA(nvme_fc_ctrl_cnt);
223223

224-
static struct workqueue_struct *nvme_fc_wq;
225-
226-
static bool nvme_fc_waiting_to_unload;
227-
static DECLARE_COMPLETION(nvme_fc_unload_proceed);
228-
229224
/*
230225
* These items are short-term. They will eventually be moved into
231226
* a generic FC class. See comments in module init.
@@ -255,8 +250,6 @@ nvme_fc_free_lport(struct kref *ref)
255250
/* remove from transport list */
256251
spin_lock_irqsave(&nvme_fc_lock, flags);
257252
list_del(&lport->port_list);
258-
if (nvme_fc_waiting_to_unload && list_empty(&nvme_fc_lport_list))
259-
complete(&nvme_fc_unload_proceed);
260253
spin_unlock_irqrestore(&nvme_fc_lock, flags);
261254

262255
ida_free(&nvme_fc_local_port_cnt, lport->localport.port_num);
@@ -3899,10 +3892,6 @@ static int __init nvme_fc_init_module(void)
38993892
{
39003893
int ret;
39013894

3902-
nvme_fc_wq = alloc_workqueue("nvme_fc_wq", WQ_MEM_RECLAIM, 0);
3903-
if (!nvme_fc_wq)
3904-
return -ENOMEM;
3905-
39063895
/*
39073896
* NOTE:
39083897
* It is expected that in the future the kernel will combine
@@ -3920,7 +3909,7 @@ static int __init nvme_fc_init_module(void)
39203909
ret = class_register(&fc_class);
39213910
if (ret) {
39223911
pr_err("couldn't register class fc\n");
3923-
goto out_destroy_wq;
3912+
return ret;
39243913
}
39253914

39263915
/*
@@ -3944,8 +3933,6 @@ static int __init nvme_fc_init_module(void)
39443933
device_destroy(&fc_class, MKDEV(0, 0));
39453934
out_destroy_class:
39463935
class_unregister(&fc_class);
3947-
out_destroy_wq:
3948-
destroy_workqueue(nvme_fc_wq);
39493936

39503937
return ret;
39513938
}
@@ -3965,45 +3952,23 @@ nvme_fc_delete_controllers(struct nvme_fc_rport *rport)
39653952
spin_unlock(&rport->lock);
39663953
}
39673954

3968-
static void
3969-
nvme_fc_cleanup_for_unload(void)
3955+
static void __exit nvme_fc_exit_module(void)
39703956
{
39713957
struct nvme_fc_lport *lport;
39723958
struct nvme_fc_rport *rport;
3973-
3974-
list_for_each_entry(lport, &nvme_fc_lport_list, port_list) {
3975-
list_for_each_entry(rport, &lport->endp_list, endp_list) {
3976-
nvme_fc_delete_controllers(rport);
3977-
}
3978-
}
3979-
}
3980-
3981-
static void __exit nvme_fc_exit_module(void)
3982-
{
39833959
unsigned long flags;
3984-
bool need_cleanup = false;
39853960

39863961
spin_lock_irqsave(&nvme_fc_lock, flags);
3987-
nvme_fc_waiting_to_unload = true;
3988-
if (!list_empty(&nvme_fc_lport_list)) {
3989-
need_cleanup = true;
3990-
nvme_fc_cleanup_for_unload();
3991-
}
3962+
list_for_each_entry(lport, &nvme_fc_lport_list, port_list)
3963+
list_for_each_entry(rport, &lport->endp_list, endp_list)
3964+
nvme_fc_delete_controllers(rport);
39923965
spin_unlock_irqrestore(&nvme_fc_lock, flags);
3993-
if (need_cleanup) {
3994-
pr_info("%s: waiting for ctlr deletes\n", __func__);
3995-
wait_for_completion(&nvme_fc_unload_proceed);
3996-
pr_info("%s: ctrl deletes complete\n", __func__);
3997-
}
3966+
flush_workqueue(nvme_delete_wq);
39983967

39993968
nvmf_unregister_transport(&nvme_fc_transport);
40003969

4001-
ida_destroy(&nvme_fc_local_port_cnt);
4002-
ida_destroy(&nvme_fc_ctrl_cnt);
4003-
40043970
device_destroy(&fc_class, MKDEV(0, 0));
40053971
class_unregister(&fc_class);
4006-
destroy_workqueue(nvme_fc_wq);
40073972
}
40083973

40093974
module_init(nvme_fc_init_module);

0 commit comments

Comments
 (0)