| Message ID | 20221114081936.35804-1-yuancan@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: thunderbolt: Fix error handling in tbnet_init() | expand |
Hi, On Mon, Nov 14, 2022 at 08:19:36AM +0000, Yuan Can wrote: > A problem about insmod thunderbolt-net failed is triggered with following > log given while lsmod does not show thunderbolt_net: > > insmod: ERROR: could not insert module thunderbolt-net.ko: File exists > > The reason is that tbnet_init() returns tb_register_service_driver() > directly without checking its return value, if tb_register_service_driver() > failed, it returns without removing property directory, resulting the > property directory can never be created later. > > tbnet_init() > tb_register_property_dir() # register property directory > tb_register_service_driver() > driver_register() > bus_add_driver() > priv = kzalloc(...) # OOM happened > # return without remove property directory > > Fix by remove property directory when tb_register_service_driver() returns > error. > > Fixes: e69b6c02b4c3 ("net: Add support for networking over Thunderbolt cable") > Signed-off-by: Yuan Can <yuancan@huawei.com> > --- > drivers/net/thunderbolt.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c > index 83fcaeb2ac5e..fe6a9881cc75 100644 > --- a/drivers/net/thunderbolt.c > +++ b/drivers/net/thunderbolt.c > @@ -1396,7 +1396,14 @@ static int __init tbnet_init(void) > return ret; > } > > - return tb_register_service_driver(&tbnet_driver); > + ret = tb_register_service_driver(&tbnet_driver); > + if (ret) { > + tb_unregister_property_dir("network", tbnet_dir); > + tb_property_free_dir(tbnet_dir); > + return ret; > + } > + > + return 0; Okay but I suggest that you make it like: ret = tb_register_property_dir("network", tbnet_dir); if (ret) goto err_free_dir; ret = tb_register_service_driver(&tbnet_driver); if (ret) goto err_unregister; return 0; err_unregister: tb_unregister_property_dir("network", tbnet_dir); err_free_dir: tb_property_free_dir(tbnet_dir); return ret; }
在 2022/11/14 18:09, Mika Westerberg 写道: > Hi, > > On Mon, Nov 14, 2022 at 08:19:36AM +0000, Yuan Can wrote: >> A problem about insmod thunderbolt-net failed is triggered with following >> log given while lsmod does not show thunderbolt_net: >> >> insmod: ERROR: could not insert module thunderbolt-net.ko: File exists >> >> The reason is that tbnet_init() returns tb_register_service_driver() >> directly without checking its return value, if tb_register_service_driver() >> failed, it returns without removing property directory, resulting the >> property directory can never be created later. >> >> tbnet_init() >> tb_register_property_dir() # register property directory >> tb_register_service_driver() >> driver_register() >> bus_add_driver() >> priv = kzalloc(...) # OOM happened >> # return without remove property directory >> >> Fix by remove property directory when tb_register_service_driver() returns >> error. >> >> Fixes: e69b6c02b4c3 ("net: Add support for networking over Thunderbolt cable") >> Signed-off-by: Yuan Can <yuancan@huawei.com> >> --- >> drivers/net/thunderbolt.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c >> index 83fcaeb2ac5e..fe6a9881cc75 100644 >> --- a/drivers/net/thunderbolt.c >> +++ b/drivers/net/thunderbolt.c >> @@ -1396,7 +1396,14 @@ static int __init tbnet_init(void) >> return ret; >> } >> >> - return tb_register_service_driver(&tbnet_driver); >> + ret = tb_register_service_driver(&tbnet_driver); >> + if (ret) { >> + tb_unregister_property_dir("network", tbnet_dir); >> + tb_property_free_dir(tbnet_dir); >> + return ret; >> + } >> + >> + return 0; > Okay but I suggest that you make it like: > > ret = tb_register_property_dir("network", tbnet_dir); > if (ret) > goto err_free_dir; > > ret = tb_register_service_driver(&tbnet_driver); > if (ret) > goto err_unregister; > > return 0; > > err_unregister: > tb_unregister_property_dir("network", tbnet_dir); > err_free_dir: > tb_property_free_dir(tbnet_dir); > > return ret; > > } Ok, thanks for the suggestion! I will switch to this style in the v2 patch.
diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c index 83fcaeb2ac5e..fe6a9881cc75 100644 --- a/drivers/net/thunderbolt.c +++ b/drivers/net/thunderbolt.c @@ -1396,7 +1396,14 @@ static int __init tbnet_init(void) return ret; } - return tb_register_service_driver(&tbnet_driver); + ret = tb_register_service_driver(&tbnet_driver); + if (ret) { + tb_unregister_property_dir("network", tbnet_dir); + tb_property_free_dir(tbnet_dir); + return ret; + } + + return 0; } module_init(tbnet_init);
A problem about insmod thunderbolt-net failed is triggered with following log given while lsmod does not show thunderbolt_net: insmod: ERROR: could not insert module thunderbolt-net.ko: File exists The reason is that tbnet_init() returns tb_register_service_driver() directly without checking its return value, if tb_register_service_driver() failed, it returns without removing property directory, resulting the property directory can never be created later. tbnet_init() tb_register_property_dir() # register property directory tb_register_service_driver() driver_register() bus_add_driver() priv = kzalloc(...) # OOM happened # return without remove property directory Fix by remove property directory when tb_register_service_driver() returns error. Fixes: e69b6c02b4c3 ("net: Add support for networking over Thunderbolt cable") Signed-off-by: Yuan Can <yuancan@huawei.com> --- drivers/net/thunderbolt.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)