stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Vlad Yasevich <vyasevich@gmail.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.1 14/29] sctp: fix race on protocol/netns initialization
Date: Thu,  1 Oct 2015 11:31:50 +0200	[thread overview]
Message-ID: <20151001093146.332386910@linuxfoundation.org> (raw)
In-Reply-To: <20151001093145.730759857@linuxfoundation.org>

4.1-stable review patch.  If anyone has any objections, please let me know.

------------------

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();



  parent reply	other threads:[~2015-10-01  9:32 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-01  9:31 [PATCH 4.1 00/29] 4.1.10-stable review Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 01/29] ip6_gre: release cached dst on tunnel removal Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 02/29] vxlan: re-ignore EADDRINUSE from igmp_join Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 03/29] cls_u32: complete the check for non-forced case in u32_destroy() Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 04/29] usbnet: Get EVENT_NO_RUNTIME_PM bit before it is cleared Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 05/29] sock, diag: fix panic in sock_diag_put_filterinfo Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 06/29] ipv6: fix exthdrs offload registration in out_rt path Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 07/29] net: fec: clear receive interrupts before processing a packet Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 08/29] net: eth: altera: fix napi poll_list corruption Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 09/29] net/ipv6: Correct PIM6 mrt_lock handling Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 10/29] net: dsa: bcm_sf2: Fix ageing conditions and operation Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 11/29] ipv6: fix multipath route replace error recovery Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 12/29] net: dsa: bcm_sf2: Fix 64-bits register writes Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 13/29] netlink, mmap: transform mmap skb into full skb on taps Greg Kroah-Hartman
2015-10-01  9:31 ` Greg Kroah-Hartman [this message]
2015-10-01  9:31 ` [PATCH 4.1 16/29] net/mlx4_en: really allow to change RSS key Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 17/29] macvtap: fix TUNSETSNDBUF values > 64k Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 18/29] openvswitch: Zero flows on allocation Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 19/29] tcp: add proper TS val into RST packets Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 20/29] net: revert "net_sched: move tp->root allocation into fw_init()" Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 21/29] fib_rules: fix fib rule dumps across multiple skbs Greg Kroah-Hartman
2015-10-01  9:31 ` [PATCH 4.1 22/29] net: dsa: bcm_sf2: Do not override speed settings Greg Kroah-Hartman
2015-10-02  0:47   ` Ben Hutchings
2015-10-02  1:22     ` David Miller
2015-10-03 11:38       ` Greg KH
2015-10-01  9:31 ` [PATCH 4.1 23/29] net: phy: fixed_phy: handle link-down case Greg Kroah-Hartman
2015-10-01  9:32 ` [PATCH 4.1 24/29] of_mdio: add new DT property managed to specify the PHY management type Greg Kroah-Hartman
2015-10-01  9:32 ` [PATCH 4.1 25/29] mvneta: use inband status only when explicitly enabled Greg Kroah-Hartman
2015-10-01  9:32 ` [PATCH 4.1 26/29] netlink: Fix autobind race condition that leads to zero port ID Greg Kroah-Hartman
2015-10-01  9:32 ` [PATCH 4.1 27/29] netlink: Replace rhash_portid with bound Greg Kroah-Hartman
2015-10-01  9:32 ` [PATCH 4.1 28/29] zram: fix possible use after free in zcomp_create() Greg Kroah-Hartman
2015-10-01  9:32 ` [PATCH 4.1 29/29] hp-wmi: limit hotkey enable Greg Kroah-Hartman
     [not found] ` <20151001093146.372268230@linuxfoundation.org>
2015-10-01 23:13   ` [PATCH 4.1 15/29] bridge: fix igmpv3 / mldv2 report parsing Linus Lüssing
2015-10-02  1:29 ` [PATCH 4.1 00/29] 4.1.10-stable review Guenter Roeck
2015-10-02  6:23 ` Sudip Mukherjee
     [not found] ` <560e9a58.e127b40a.bc060.fffffb97@mx.google.com>
2015-10-02 15:04   ` Kevin Hilman
2015-10-02 15:41 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151001093146.332386910@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).