CVE-2015-5283: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The sctp_init function in net/sctp/protocol.c in the Linux kernel before 4.2.3 has an incorrect sequence of protocol-initialization steps, which allows local users to cause a denial of service (panic or memory corruption) by creating SCTP sockets before all of the steps have finished.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2015-10-19T10:59Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: net/sctp/protocol.c
Commit date: 2015-09-11 15:00:02 -0700
Commit message: sctp: fix race on protocol/netns initialization

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>

Diff: diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index b7143337e4fa02..3d9ea9a48289af 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1184,11 +1184,11 @@ static void sctp_v4_del_protocol(void)
 {
 	inet_del_protocol(&sctp_protocol, IPPROTO_SCTP);
 	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;
 
 	/*
 	 * 14. Suggested SCTP Protocol Parameter Values
@@ -1277,16 +1277,10 @@ static int __net_init sctp_net_init(struct net *net)
 	if (status)
 		goto err_init_proc;
 
 	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);
 	sctp_get_local_addr_list(net);
 
@@ -1298,40 +1292,57 @@ static int __net_init sctp_net_init(struct net *net)
 	setup_timer(&net->sctp.addr_wq_timer, sctp_addr_wq_timeout_handler,
 		    (unsigned long)net);
 
 	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:
 	sctp_sysctl_net_unregister(net);
 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);
 	cleanup_sctp_mibs(net);
 	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.  */
 static __init int sctp_init(void)
 {
@@ -1460,22 +1471,25 @@ static __init int sctp_init(void)
 
 	INIT_LIST_HEAD(&sctp_address_families);
 	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;
 
 	status = sctp_v6_protosw_init();
 	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)
 		goto err_add_protocol;
 
@@ -1487,16 +1501,18 @@ static __init int sctp_init(void)
 out:
 	return status;
 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();
 	free_pages((unsigned long)sctp_port_hashtable,
 		   get_order(sctp_port_hashsize *
@@ -1525,16 +1541,18 @@ static __exit void sctp_exit(void)
 
 	/* Unregister with inet6/inet layers. */
 	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();
 
 	sctp_sysctl_unregister();

Vulnerable code: static int __net_init sctp_net_init(struct net *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;
	}

err_ctl_sock_init:
	sctp_dbg_objcnt_exit(net);
	sctp_proc_exit(net);
static void __net_exit sctp_net_exit(struct net *net)
	/* Free the control endpoint.  */
	inet_ctl_sock_destroy(net->sctp.ctl_sock);

static struct pernet_operations sctp_net_ops = {
	.init = sctp_net_init,
	.exit = sctp_net_exit,
	status = sctp_v4_protosw_init();
	status = register_pernet_subsys(&sctp_net_ops);
		goto err_register_pernet_subsys;
	unregister_pernet_subsys(&sctp_net_ops);
err_register_pernet_subsys:
	unregister_pernet_subsys(&sctp_net_ops);
Fixed code: static int __net_init sctp_defaults_init(struct net *net)
static void __net_exit sctp_defaults_exit(struct net *net)
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,
	status = register_pernet_subsys(&sctp_defaults_ops);
	if (status)
		goto err_register_defaults;
	status = sctp_v4_protosw_init();
	status = register_pernet_subsys(&sctp_ctrlsock_ops);
		goto err_register_ctrlsock;
	unregister_pernet_subsys(&sctp_ctrlsock_ops);
err_register_ctrlsock:
	unregister_pernet_subsys(&sctp_defaults_ops);
err_register_defaults:
	unregister_pernet_subsys(&sctp_ctrlsock_ops);
	unregister_pernet_subsys(&sctp_defaults_ops);