From patchwork Wed Sep 9 20:03:01 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Ricardo Leitner X-Patchwork-Id: 515996 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id C5FB714028F for ; Thu, 10 Sep 2015 06:04:27 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753141AbbIIUEX (ORCPT ); Wed, 9 Sep 2015 16:04:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42839 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751390AbbIIUEW (ORCPT ); Wed, 9 Sep 2015 16:04:22 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 3C54A461ED; Wed, 9 Sep 2015 20:04:22 +0000 (UTC) Received: from localhost.localdomain.com (vpn1-5-131.gru2.redhat.com [10.97.5.131]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t89K4INw016785; Wed, 9 Sep 2015 16:04:19 -0400 From: Marcelo Ricardo Leitner To: netdev@vger.kernel.org Cc: Vlad Yasevich , Neil Horman , linux-sctp@vger.kernel.org Subject: [PATCH net] sctp: fix race on protocol/netns initialization Date: Wed, 9 Sep 2015 17:03:01 -0300 Message-Id: <27f20cc781cac28bc2ce7229719748a2f6824bd8.1441827818.git.marcelo.leitner@gmail.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 | 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 the fix then is to invert the initialization order inside register_pernet_subsys() so that the control socket is created by last and also block socket creation if netns initialization wasn't yet performed. Fixes: 4db67e808640 ("sctp: Make the address lists per network namespace") Signed-off-by: Marcelo Ricardo Leitner --- net/sctp/protocol.c | 18 +++++++++++------- net/sctp/socket.c | 4 ++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 4345790ad3266c353eeac5398593c2a9ce4effda..d8f78165768a75f93f4ce4120dd5475b6a623aaf 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -1271,12 +1271,6 @@ static int __net_init sctp_net_init(struct net *net) 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); @@ -1284,11 +1278,21 @@ static int __net_init sctp_net_init(struct net *net) /* Initialize the address event list */ INIT_LIST_HEAD(&net->sctp.addr_waitq); - INIT_LIST_HEAD(&net->sctp.auto_asconf_splist); spin_lock_init(&net->sctp.addr_wq_lock); net->sctp.addr_wq_timer.expires = 0; setup_timer(&net->sctp.addr_wq_timer, sctp_addr_wq_timeout_handler, (unsigned long)net); + /* sctp_init_sock() will use this to know that netns is + * nearly all initialized but already good to go. + */ + INIT_LIST_HEAD(&net->sctp.auto_asconf_splist); + + /* 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"); + goto err_ctl_sock_init; + } return 0; diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 17bef01b9aa3e7f75328d39fc976f9e80d641e92..45b94deec93d0c7c1612a16922348cf2a7e65ec5 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -3993,6 +3993,10 @@ static int sctp_init_sock(struct sock *sk) pr_debug("%s: sk:%p\n", __func__, sk); + /* Validate if netns is already initialized. */ + if (!net->sctp.auto_asconf_splist.prev) + return -ENOPROTOOPT; + sp = sctp_sk(sk); /* Initialize the SCTP per socket area. */