| From foo@baz Wed Sep 30 05:25:07 CEST 2015 |
| From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> |
| Date: Thu, 10 Sep 2015 17:31:15 -0300 |
| Subject: sctp: fix race on protocol/netns initialization |
| |
| From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> |
| |
| [ Upstream commit 8e2d61e0aed2b7c4ecb35844fe07e0b2b762dee4 ] |
| |
| Consider sctp module is unloaded and is being requested because an user |
| is creating a sctp socket. |
| |
| During initialization, sctp will add the new protocol type and then |
| initialize pernet subsys: |
| |
| status = sctp_v4_protosw_init(); |
| if (status) |
| goto err_protosw_init; |
| |
| status = sctp_v6_protosw_init(); |
| if (status) |
| goto err_v6_protosw_init; |
| |
| status = register_pernet_subsys(&sctp_net_ops); |
| |
| The problem is that after those calls to sctp_v{4,6}_protosw_init(), it |
| is possible for userspace to create SCTP sockets like if the module is |
| already fully loaded. If that happens, one of the possible effects is |
| that we will have readers for net->sctp.local_addr_list list earlier |
| than expected and sctp_net_init() does not take precautions while |
| dealing with that list, leading to a potential panic but not limited to |
| that, as sctp_sock_init() will copy a bunch of blank/partially |
| initialized values from net->sctp. |
| |
| The race happens like this: |
| |
| CPU 0 | CPU 1 |
| socket() | |
| __sock_create | socket() |
| inet_create | __sock_create |
| list_for_each_entry_rcu( | |
| answer, &inetsw[sock->type], | |
| list) { | inet_create |
| /* no hits */ | |
| if (unlikely(err)) { | |
| ... | |
| request_module() | |
| /* socket creation is blocked | |
| * the module is fully loaded | |
| */ | |
| sctp_init | |
| sctp_v4_protosw_init | |
| inet_register_protosw | |
| list_add_rcu(&p->list, | |
| last_perm); | |
| | list_for_each_entry_rcu( |
| | answer, &inetsw[sock->type], |
| sctp_v6_protosw_init | list) { |
| | /* hit, so assumes protocol |
| | * is already loaded |
| | */ |
| | /* socket creation continues |
| | * before netns is initialized |
| | */ |
| register_pernet_subsys | |
| |
| Simply inverting the initialization order between |
| register_pernet_subsys() and sctp_v4_protosw_init() is not possible |
| because register_pernet_subsys() will create a control sctp socket, so |
| the protocol must be already visible by then. Deferring the socket |
| creation to a work-queue is not good specially because we loose the |
| ability to handle its errors. |
| |
| So, as suggested by Vlad, the fix is to split netns initialization in |
| two moments: defaults and control socket, so that the defaults are |
| already loaded by when we register the protocol, while control socket |
| initialization is kept at the same moment it is today. |
| |
| Fixes: 4db67e808640 ("sctp: Make the address lists per network namespace") |
| Signed-off-by: Vlad Yasevich <vyasevich@gmail.com> |
| Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/sctp/protocol.c | 64 +++++++++++++++++++++++++++++++++------------------- |
| 1 file changed, 41 insertions(+), 23 deletions(-) |
| |
| --- a/net/sctp/protocol.c |
| +++ b/net/sctp/protocol.c |
| @@ -1166,7 +1166,7 @@ static void sctp_v4_del_protocol(void) |
| unregister_inetaddr_notifier(&sctp_inetaddr_notifier); |
| } |
| |
| -static int __net_init sctp_net_init(struct net *net) |
| +static int __net_init sctp_defaults_init(struct net *net) |
| { |
| int status; |
| |
| @@ -1259,12 +1259,6 @@ static int __net_init sctp_net_init(stru |
| |
| sctp_dbg_objcnt_init(net); |
| |
| - /* Initialize the control inode/socket for handling OOTB packets. */ |
| - if ((status = sctp_ctl_sock_init(net))) { |
| - pr_err("Failed to initialize the SCTP control sock\n"); |
| - goto err_ctl_sock_init; |
| - } |
| - |
| /* Initialize the local address list. */ |
| INIT_LIST_HEAD(&net->sctp.local_addr_list); |
| spin_lock_init(&net->sctp.local_addr_lock); |
| @@ -1280,9 +1274,6 @@ static int __net_init sctp_net_init(stru |
| |
| return 0; |
| |
| -err_ctl_sock_init: |
| - sctp_dbg_objcnt_exit(net); |
| - sctp_proc_exit(net); |
| err_init_proc: |
| cleanup_sctp_mibs(net); |
| err_init_mibs: |
| @@ -1291,15 +1282,12 @@ err_sysctl_register: |
| return status; |
| } |
| |
| -static void __net_exit sctp_net_exit(struct net *net) |
| +static void __net_exit sctp_defaults_exit(struct net *net) |
| { |
| /* Free the local address list */ |
| sctp_free_addr_wq(net); |
| sctp_free_local_addr_list(net); |
| |
| - /* Free the control endpoint. */ |
| - inet_ctl_sock_destroy(net->sctp.ctl_sock); |
| - |
| sctp_dbg_objcnt_exit(net); |
| |
| sctp_proc_exit(net); |
| @@ -1307,9 +1295,32 @@ static void __net_exit sctp_net_exit(str |
| sctp_sysctl_net_unregister(net); |
| } |
| |
| -static struct pernet_operations sctp_net_ops = { |
| - .init = sctp_net_init, |
| - .exit = sctp_net_exit, |
| +static struct pernet_operations sctp_defaults_ops = { |
| + .init = sctp_defaults_init, |
| + .exit = sctp_defaults_exit, |
| +}; |
| + |
| +static int __net_init sctp_ctrlsock_init(struct net *net) |
| +{ |
| + int status; |
| + |
| + /* Initialize the control inode/socket for handling OOTB packets. */ |
| + status = sctp_ctl_sock_init(net); |
| + if (status) |
| + pr_err("Failed to initialize the SCTP control sock\n"); |
| + |
| + return status; |
| +} |
| + |
| +static void __net_init sctp_ctrlsock_exit(struct net *net) |
| +{ |
| + /* Free the control endpoint. */ |
| + inet_ctl_sock_destroy(net->sctp.ctl_sock); |
| +} |
| + |
| +static struct pernet_operations sctp_ctrlsock_ops = { |
| + .init = sctp_ctrlsock_init, |
| + .exit = sctp_ctrlsock_exit, |
| }; |
| |
| /* Initialize the universe into something sensible. */ |
| @@ -1442,8 +1453,11 @@ static __init int sctp_init(void) |
| sctp_v4_pf_init(); |
| sctp_v6_pf_init(); |
| |
| - status = sctp_v4_protosw_init(); |
| + status = register_pernet_subsys(&sctp_defaults_ops); |
| + if (status) |
| + goto err_register_defaults; |
| |
| + status = sctp_v4_protosw_init(); |
| if (status) |
| goto err_protosw_init; |
| |
| @@ -1451,9 +1465,9 @@ static __init int sctp_init(void) |
| if (status) |
| goto err_v6_protosw_init; |
| |
| - status = register_pernet_subsys(&sctp_net_ops); |
| + status = register_pernet_subsys(&sctp_ctrlsock_ops); |
| if (status) |
| - goto err_register_pernet_subsys; |
| + goto err_register_ctrlsock; |
| |
| status = sctp_v4_add_protocol(); |
| if (status) |
| @@ -1469,12 +1483,14 @@ out: |
| err_v6_add_protocol: |
| sctp_v4_del_protocol(); |
| err_add_protocol: |
| - unregister_pernet_subsys(&sctp_net_ops); |
| -err_register_pernet_subsys: |
| + unregister_pernet_subsys(&sctp_ctrlsock_ops); |
| +err_register_ctrlsock: |
| sctp_v6_protosw_exit(); |
| err_v6_protosw_init: |
| sctp_v4_protosw_exit(); |
| err_protosw_init: |
| + unregister_pernet_subsys(&sctp_defaults_ops); |
| +err_register_defaults: |
| sctp_v4_pf_exit(); |
| sctp_v6_pf_exit(); |
| sctp_sysctl_unregister(); |
| @@ -1507,12 +1523,14 @@ static __exit void sctp_exit(void) |
| sctp_v6_del_protocol(); |
| sctp_v4_del_protocol(); |
| |
| - unregister_pernet_subsys(&sctp_net_ops); |
| + unregister_pernet_subsys(&sctp_ctrlsock_ops); |
| |
| /* Free protosw registrations */ |
| sctp_v6_protosw_exit(); |
| sctp_v4_protosw_exit(); |
| |
| + unregister_pernet_subsys(&sctp_defaults_ops); |
| + |
| /* Unregister with socket layer. */ |
| sctp_v6_pf_exit(); |
| sctp_v4_pf_exit(); |