| Message ID | 20221101093722.127223-1-chenzhongjin@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 62ff373da2534534c55debe6c724c7fe14adb97f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/smc: Fix possible leaked pernet namespace in smc_init() | expand |
On Tue, Nov 01, 2022 at 05:37:22PM +0800, Chen Zhongjin wrote: > In smc_init(), register_pernet_subsys(&smc_net_stat_ops) is called > without any error handling. > If it fails, registering of &smc_net_ops won't be reverted. > And if smc_nl_init() fails, &smc_net_stat_ops itself won't be reverted. > > This leaves wild ops in subsystem linkedlist and when another module > tries to call register_pernet_operations() it triggers page fault: > > BUG: unable to handle page fault for address: fffffbfff81b964c > RIP: 0010:register_pernet_operations+0x1b9/0x5f0 > Call Trace: > <TASK> > register_pernet_subsys+0x29/0x40 > ebtables_init+0x58/0x1000 [ebtables] > ... > > Fixes: 194730a9beb5 ("net/smc: Make SMC statistics network namespace aware") > Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com> This patch looks good to me. The subject of this patch should be in net, the prefix tag is missed. Reviewed-by: Tony Lu <tonylu@linux.alibaba.com> > --- > net/smc/af_smc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index 3ccbf3c201cd..e12d4fa5aece 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -3380,14 +3380,14 @@ static int __init smc_init(void) > > rc = register_pernet_subsys(&smc_net_stat_ops); > if (rc) > - return rc; > + goto out_pernet_subsys; > > smc_ism_init(); > smc_clc_init(); > > rc = smc_nl_init(); > if (rc) > - goto out_pernet_subsys; > + goto out_pernet_subsys_stat; > > rc = smc_pnet_init(); > if (rc) > @@ -3480,6 +3480,8 @@ static int __init smc_init(void) > smc_pnet_exit(); > out_nl: > smc_nl_exit(); > +out_pernet_subsys_stat: > + unregister_pernet_subsys(&smc_net_stat_ops); > out_pernet_subsys: > unregister_pernet_subsys(&smc_net_ops); > > -- > 2.17.1
Hi, On 2022/11/1 19:00, Tony Lu wrote: > On Tue, Nov 01, 2022 at 05:37:22PM +0800, Chen Zhongjin wrote: >> In smc_init(), register_pernet_subsys(&smc_net_stat_ops) is called >> without any error handling. >> If it fails, registering of &smc_net_ops won't be reverted. >> And if smc_nl_init() fails, &smc_net_stat_ops itself won't be reverted. >> >> This leaves wild ops in subsystem linkedlist and when another module >> tries to call register_pernet_operations() it triggers page fault: >> >> BUG: unable to handle page fault for address: fffffbfff81b964c >> RIP: 0010:register_pernet_operations+0x1b9/0x5f0 >> Call Trace: >> <TASK> >> register_pernet_subsys+0x29/0x40 >> ebtables_init+0x58/0x1000 [ebtables] >> ... >> >> Fixes: 194730a9beb5 ("net/smc: Make SMC statistics network namespace aware") >> Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com> > This patch looks good to me. > > The subject of this patch should be in net, the prefix tag is missed. Thanks for review and remind! I will add net tag for the next time I send patch to netdev. Best, Chen > > Reviewed-by: Tony Lu <tonylu@linux.alibaba.com> > >> --- >> net/smc/af_smc.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c >> index 3ccbf3c201cd..e12d4fa5aece 100644 >> --- a/net/smc/af_smc.c >> +++ b/net/smc/af_smc.c >> @@ -3380,14 +3380,14 @@ static int __init smc_init(void) >> >> rc = register_pernet_subsys(&smc_net_stat_ops); >> if (rc) >> - return rc; >> + goto out_pernet_subsys; >> >> smc_ism_init(); >> smc_clc_init(); >> >> rc = smc_nl_init(); >> if (rc) >> - goto out_pernet_subsys; >> + goto out_pernet_subsys_stat; >> >> rc = smc_pnet_init(); >> if (rc) >> @@ -3480,6 +3480,8 @@ static int __init smc_init(void) >> smc_pnet_exit(); >> out_nl: >> smc_nl_exit(); >> +out_pernet_subsys_stat: >> + unregister_pernet_subsys(&smc_net_stat_ops); >> out_pernet_subsys: >> unregister_pernet_subsys(&smc_net_ops); >> >> -- >> 2.17.1
On 01.11.22 10:37, Chen Zhongjin wrote: > In smc_init(), register_pernet_subsys(&smc_net_stat_ops) is called > without any error handling. > If it fails, registering of &smc_net_ops won't be reverted. > And if smc_nl_init() fails, &smc_net_stat_ops itself won't be reverted. > > This leaves wild ops in subsystem linkedlist and when another module > tries to call register_pernet_operations() it triggers page fault: > > BUG: unable to handle page fault for address: fffffbfff81b964c > RIP: 0010:register_pernet_operations+0x1b9/0x5f0 > Call Trace: > <TASK> > register_pernet_subsys+0x29/0x40 > ebtables_init+0x58/0x1000 [ebtables] > ... > > Fixes: 194730a9beb5 ("net/smc: Make SMC statistics network namespace aware") > Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com> It looks good, thank you for the effort! Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> > --- > net/smc/af_smc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index 3ccbf3c201cd..e12d4fa5aece 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -3380,14 +3380,14 @@ static int __init smc_init(void) > > rc = register_pernet_subsys(&smc_net_stat_ops); > if (rc) > - return rc; > + goto out_pernet_subsys; > > smc_ism_init(); > smc_clc_init(); > > rc = smc_nl_init(); > if (rc) > - goto out_pernet_subsys; > + goto out_pernet_subsys_stat; > > rc = smc_pnet_init(); > if (rc) > @@ -3480,6 +3480,8 @@ static int __init smc_init(void) > smc_pnet_exit(); > out_nl: > smc_nl_exit(); > +out_pernet_subsys_stat: > + unregister_pernet_subsys(&smc_net_stat_ops); > out_pernet_subsys: > unregister_pernet_subsys(&smc_net_ops); >
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 1 Nov 2022 17:37:22 +0800 you wrote: > In smc_init(), register_pernet_subsys(&smc_net_stat_ops) is called > without any error handling. > If it fails, registering of &smc_net_ops won't be reverted. > And if smc_nl_init() fails, &smc_net_stat_ops itself won't be reverted. > > This leaves wild ops in subsystem linkedlist and when another module > tries to call register_pernet_operations() it triggers page fault: > > [...] Here is the summary with links: - net/smc: Fix possible leaked pernet namespace in smc_init() https://git.kernel.org/netdev/net/c/62ff373da253 You are awesome, thank you!
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c index 3ccbf3c201cd..e12d4fa5aece 100644 --- a/net/smc/af_smc.c +++ b/net/smc/af_smc.c @@ -3380,14 +3380,14 @@ static int __init smc_init(void) rc = register_pernet_subsys(&smc_net_stat_ops); if (rc) - return rc; + goto out_pernet_subsys; smc_ism_init(); smc_clc_init(); rc = smc_nl_init(); if (rc) - goto out_pernet_subsys; + goto out_pernet_subsys_stat; rc = smc_pnet_init(); if (rc) @@ -3480,6 +3480,8 @@ static int __init smc_init(void) smc_pnet_exit(); out_nl: smc_nl_exit(); +out_pernet_subsys_stat: + unregister_pernet_subsys(&smc_net_stat_ops); out_pernet_subsys: unregister_pernet_subsys(&smc_net_ops);
In smc_init(), register_pernet_subsys(&smc_net_stat_ops) is called without any error handling. If it fails, registering of &smc_net_ops won't be reverted. And if smc_nl_init() fails, &smc_net_stat_ops itself won't be reverted. This leaves wild ops in subsystem linkedlist and when another module tries to call register_pernet_operations() it triggers page fault: BUG: unable to handle page fault for address: fffffbfff81b964c RIP: 0010:register_pernet_operations+0x1b9/0x5f0 Call Trace: <TASK> register_pernet_subsys+0x29/0x40 ebtables_init+0x58/0x1000 [ebtables] ... Fixes: 194730a9beb5 ("net/smc: Make SMC statistics network namespace aware") Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com> --- net/smc/af_smc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)