From owner-svn-src-stable-8@FreeBSD.ORG Tue Jun 1 09:32:22 2010 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6A21D106564A; Tue, 1 Jun 2010 09:32:22 +0000 (UTC) (envelope-from attilio@FreeBSD.org) Received: from svn.freebsd.org (unknown [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 58C938FC23; Tue, 1 Jun 2010 09:32:22 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o519WMuh086975; Tue, 1 Jun 2010 09:32:22 GMT (envelope-from attilio@svn.freebsd.org) Received: (from attilio@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o519WMfn086973; Tue, 1 Jun 2010 09:32:22 GMT (envelope-from attilio@svn.freebsd.org) Message-Id: <201006010932.o519WMfn086973@svn.freebsd.org> From: Attilio Rao Date: Tue, 1 Jun 2010 09:32:22 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r208690 - stable/8/sys/netgraph X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jun 2010 09:32:22 -0000 Author: attilio Date: Tue Jun 1 09:32:22 2010 New Revision: 208690 URL: http://svn.freebsd.org/changeset/base/208690 Log: MFC r208300: Fix a race between ngs_rcvmsg() and soclose() which closes the control socket while it is still in use as ngs_rcvmsg() runs without any lock held. Sponsored by: Sandvine Incorporated Approved by: re (bz) Modified: stable/8/sys/netgraph/ng_socket.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) stable/8/sys/geom/sched/ (props changed) Modified: stable/8/sys/netgraph/ng_socket.c ============================================================================== --- stable/8/sys/netgraph/ng_socket.c Tue Jun 1 08:43:46 2010 (r208689) +++ stable/8/sys/netgraph/ng_socket.c Tue Jun 1 09:32:22 2010 (r208690) @@ -855,7 +855,7 @@ static int ngs_rcvmsg(node_p node, item_p item, hook_p lasthook) { struct ngsock *const priv = NG_NODE_PRIVATE(node); - struct ngpcb *const pcbp = priv->ctlsock; + struct ngpcb *pcbp; struct socket *so; struct sockaddr_ng addr; struct ng_mesg *msg; @@ -868,15 +868,27 @@ ngs_rcvmsg(node_p node, item_p item, hoo NG_FREE_ITEM(item); /* + * Grab priv->mtx here to prevent destroying of control socket + * after checking that priv->ctlsock is not NULL. + */ + mtx_lock(&priv->mtx); + pcbp = priv->ctlsock; + + /* * Only allow mesgs to be passed if we have the control socket. * Data sockets can only support the generic messages. */ if (pcbp == NULL) { + mtx_unlock(&priv->mtx); TRAP_ERROR; NG_FREE_MSG(msg); return (EINVAL); } so = pcbp->ng_socket; + SOCKBUF_LOCK(&so->so_rcv); + + /* As long as the race is handled, priv->mtx may be unlocked now. */ + mtx_unlock(&priv->mtx); #ifdef TRACE_MESSAGES printf("[%x]:---------->[socket]: c=<%d>cmd=%x(%s) f=%x #%d\n", @@ -899,6 +911,8 @@ ngs_rcvmsg(node_p node, item_p item, hoo default: error = EINVAL; /* unknown command */ } + SOCKBUF_UNLOCK(&so->so_rcv); + /* Free the message and return. */ NG_FREE_MSG(msg); return (error); @@ -911,6 +925,7 @@ ngs_rcvmsg(node_p node, item_p item, hoo addrlen = snprintf((char *)&addr.sg_data, sizeof(addr.sg_data), "[%x]:", retaddr); if (addrlen < 0 || addrlen > sizeof(addr.sg_data)) { + SOCKBUF_UNLOCK(&so->so_rcv); printf("%s: snprintf([%x]) failed - %d\n", __func__, retaddr, addrlen); NG_FREE_MSG(msg); @@ -928,17 +943,20 @@ ngs_rcvmsg(node_p node, item_p item, hoo NG_FREE_MSG(msg); if (m == NULL) { + SOCKBUF_UNLOCK(&so->so_rcv); TRAP_ERROR; return (ENOBUFS); } /* Send it up to the socket. */ - if (sbappendaddr(&so->so_rcv, (struct sockaddr *)&addr, m, NULL) == 0) { + if (sbappendaddr_locked(&so->so_rcv, (struct sockaddr *)&addr, m, + NULL) == 0) { + SOCKBUF_UNLOCK(&so->so_rcv); TRAP_ERROR; m_freem(m); return (ENOBUFS); } - sorwakeup(so); + sorwakeup_locked(so); return (error); } @@ -1020,8 +1038,11 @@ static int ngs_shutdown(node_p node) { struct ngsock *const priv = NG_NODE_PRIVATE(node); - struct ngpcb *const dpcbp = priv->datasock; - struct ngpcb *const pcbp = priv->ctlsock; + struct ngpcb *dpcbp, *pcbp; + + mtx_lock(&priv->mtx); + dpcbp = priv->datasock; + pcbp = priv->ctlsock; if (dpcbp != NULL) soisdisconnected(dpcbp->ng_socket); @@ -1029,7 +1050,6 @@ ngs_shutdown(node_p node) if (pcbp != NULL) soisdisconnected(pcbp->ng_socket); - mtx_lock(&priv->mtx); priv->node = NULL; NG_NODE_SET_PRIVATE(node, NULL); ng_socket_free_priv(priv);