| Message ID | 20221122101346.80461-1-yuancan@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: cpqphp: Fix error handling in cpqhpc_init() | expand |
On Tue, Nov 22, 2022 at 10:13:46AM +0000, Yuan Can wrote: > The cpqhpc_init() returns the result of pci_register_driver() without > checking it, if pci_register_driver() failed, the debugfs created in > cpqhp_initialize_debugfs() is not removed, resulting the debugfs of > cpqhp can never be created later. > Fix by calling cpqhp_shutdown_debugfs() when pci_register_driver() failed. Add a blank line between paragraphs. > Fixes: 9f3f4681291f ("[PATCH] PCI Hotplug: fix up the sysfs file in the compaq pci hotplug driver") > Signed-off-by: Yuan Can <yuancan@huawei.com> > --- > drivers/pci/hotplug/cpqphp_core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c > index c94b40e64baf..c47981ef92ea 100644 > --- a/drivers/pci/hotplug/cpqphp_core.c > +++ b/drivers/pci/hotplug/cpqphp_core.c > @@ -1389,6 +1389,8 @@ static int __init cpqhpc_init(void) > info(DRIVER_DESC " version: " DRIVER_VERSION "\n"); > cpqhp_initialize_debugfs(); > result = pci_register_driver(&cpqhpc_driver); > + if (result) > + cpqhp_shutdown_debugfs(); Is there some reason cpqhp_initialize_debugfs() needs to be called before pci_register_driver()? In other words, could we do this instead? result = pci_register_driver(&cpqhpc_driver); if (result) return result; cpqhp_initialize_debugfs(); return 0; I assume this was found by code inspection? I've never heard of anybody actually using this driver :) > dbg("pci_register_driver = %d\n", result); > return result; > } > -- > 2.17.1 >
在 2022/11/23 4:05, Bjorn Helgaas 写道: > On Tue, Nov 22, 2022 at 10:13:46AM +0000, Yuan Can wrote: >> The cpqhpc_init() returns the result of pci_register_driver() without >> checking it, if pci_register_driver() failed, the debugfs created in >> cpqhp_initialize_debugfs() is not removed, resulting the debugfs of >> cpqhp can never be created later. >> Fix by calling cpqhp_shutdown_debugfs() when pci_register_driver() failed. > Add a blank line between paragraphs. Ok. >> Fixes: 9f3f4681291f ("[PATCH] PCI Hotplug: fix up the sysfs file in the compaq pci hotplug driver") >> Signed-off-by: Yuan Can <yuancan@huawei.com> >> --- >> drivers/pci/hotplug/cpqphp_core.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c >> index c94b40e64baf..c47981ef92ea 100644 >> --- a/drivers/pci/hotplug/cpqphp_core.c >> +++ b/drivers/pci/hotplug/cpqphp_core.c >> @@ -1389,6 +1389,8 @@ static int __init cpqhpc_init(void) >> info(DRIVER_DESC " version: " DRIVER_VERSION "\n"); >> cpqhp_initialize_debugfs(); >> result = pci_register_driver(&cpqhpc_driver); >> + if (result) >> + cpqhp_shutdown_debugfs(); > Is there some reason cpqhp_initialize_debugfs() needs to be called > before pci_register_driver()? > > In other words, could we do this instead? > > result = pci_register_driver(&cpqhpc_driver); > if (result) > return result; > > cpqhp_initialize_debugfs(); > return 0; Thanks for the suggestion! I do not see any reason that the order must be kept, will change to this style in the next version. > I assume this was found by code inspection? I've never heard of > anybody actually using this driver :) Yes, you are right :) >> dbg("pci_register_driver = %d\n", result); >> return result; >> } >> -- >> 2.17.1 >>
diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c index c94b40e64baf..c47981ef92ea 100644 --- a/drivers/pci/hotplug/cpqphp_core.c +++ b/drivers/pci/hotplug/cpqphp_core.c @@ -1389,6 +1389,8 @@ static int __init cpqhpc_init(void) info(DRIVER_DESC " version: " DRIVER_VERSION "\n"); cpqhp_initialize_debugfs(); result = pci_register_driver(&cpqhpc_driver); + if (result) + cpqhp_shutdown_debugfs(); dbg("pci_register_driver = %d\n", result); return result; }
The cpqhpc_init() returns the result of pci_register_driver() without checking it, if pci_register_driver() failed, the debugfs created in cpqhp_initialize_debugfs() is not removed, resulting the debugfs of cpqhp can never be created later. Fix by calling cpqhp_shutdown_debugfs() when pci_register_driver() failed. Fixes: 9f3f4681291f ("[PATCH] PCI Hotplug: fix up the sysfs file in the compaq pci hotplug driver") Signed-off-by: Yuan Can <yuancan@huawei.com> --- drivers/pci/hotplug/cpqphp_core.c | 2 ++ 1 file changed, 2 insertions(+)