| Message ID | 20221114082640.91128-2-yuancan@huawei.com |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | Fix error handling path | expand |
> -----Original Message----- > From: Yuan Can <yuancan@huawei.com> > Sent: Monday, November 14, 2022 12:27 AM > To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L > <anthony.l.nguyen@intel.com>; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; alexander.h.duyck@intel.com; > jeffrey.t.kirsher@intel.com; Vick, Matthew <matthew.vick@intel.com>; Keller, > Jacob E <jacob.e.keller@intel.com>; intel-wired-lan@lists.osuosl.org; > netdev@vger.kernel.org > Cc: yuancan@huawei.com > Subject: [PATCH 1/2] fm10k: Fix error handling in fm10k_init_module() > > A problem about modprobe fm10k failed is triggered with the following log > given: > > Intel(R) Ethernet Switch Host Interface Driver > Copyright(c) 2013 - 2019 Intel Corporation. > debugfs: Directory 'fm10k' with parent '/' already present! > > The reason is that fm10k_init_module() returns fm10k_register_pci_driver() > directly without checking its return value, if fm10k_register_pci_driver() > failed, it returns without removing debugfs and destroy workqueue, > resulting the debugfs of fm10k can never be created later and leaks the > workqueue. > > fm10k_init_module() > alloc_workqueue() > fm10k_dbg_init() # create debugfs > fm10k_register_pci_driver() > pci_register_driver() > driver_register() > bus_add_driver() > priv = kzalloc(...) # OOM happened > # return without remove debugfs and destroy workqueue > > Fix by remove debugfs and destroy workqueue when > fm10k_register_pci_driver() returns error. > > Fixes: 7461fd913afe ("fm10k: Add support for debugfs") > Fixes: b382bb1b3e2d ("fm10k: use separate workqueue for fm10k driver") > Signed-off-by: Yuan Can <yuancan@huawei.com> Makes sense. Thanks! Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > --- > drivers/net/ethernet/intel/fm10k/fm10k_main.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c > b/drivers/net/ethernet/intel/fm10k/fm10k_main.c > index 4a6630586ec9..fc373472e4e1 100644 > --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c > @@ -32,6 +32,8 @@ struct workqueue_struct *fm10k_workqueue; > **/ > static int __init fm10k_init_module(void) > { > + int ret; > + > pr_info("%s\n", fm10k_driver_string); > pr_info("%s\n", fm10k_copyright); > > @@ -43,7 +45,13 @@ static int __init fm10k_init_module(void) > > fm10k_dbg_init(); > > - return fm10k_register_pci_driver(); > + ret = fm10k_register_pci_driver(); > + if (ret) { > + fm10k_dbg_exit(); > + destroy_workqueue(fm10k_workqueue); > + } > + > + return ret; > } > module_init(fm10k_init_module); > > -- > 2.17.1
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c index 4a6630586ec9..fc373472e4e1 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c @@ -32,6 +32,8 @@ struct workqueue_struct *fm10k_workqueue; **/ static int __init fm10k_init_module(void) { + int ret; + pr_info("%s\n", fm10k_driver_string); pr_info("%s\n", fm10k_copyright); @@ -43,7 +45,13 @@ static int __init fm10k_init_module(void) fm10k_dbg_init(); - return fm10k_register_pci_driver(); + ret = fm10k_register_pci_driver(); + if (ret) { + fm10k_dbg_exit(); + destroy_workqueue(fm10k_workqueue); + } + + return ret; } module_init(fm10k_init_module);
A problem about modprobe fm10k failed is triggered with the following log given: Intel(R) Ethernet Switch Host Interface Driver Copyright(c) 2013 - 2019 Intel Corporation. debugfs: Directory 'fm10k' with parent '/' already present! The reason is that fm10k_init_module() returns fm10k_register_pci_driver() directly without checking its return value, if fm10k_register_pci_driver() failed, it returns without removing debugfs and destroy workqueue, resulting the debugfs of fm10k can never be created later and leaks the workqueue. fm10k_init_module() alloc_workqueue() fm10k_dbg_init() # create debugfs fm10k_register_pci_driver() pci_register_driver() driver_register() bus_add_driver() priv = kzalloc(...) # OOM happened # return without remove debugfs and destroy workqueue Fix by remove debugfs and destroy workqueue when fm10k_register_pci_driver() returns error. Fixes: 7461fd913afe ("fm10k: Add support for debugfs") Fixes: b382bb1b3e2d ("fm10k: use separate workqueue for fm10k driver") Signed-off-by: Yuan Can <yuancan@huawei.com> --- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)