Date: Wed, 19 May 2010 15:06:09 +0000 (UTC) From: Attilio Rao <attilio@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r208300 - head/sys/netgraph Message-ID: <201005191506.o4JF69SL007708@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: attilio Date: Wed May 19 15:06:09 2010 New Revision: 208300 URL: http://svn.freebsd.org/changeset/base/208300 Log: Fix a race between ngs_rcvmsg() and soclose() which closes the control socket while it is still in use. priv->ctlsock is checked at the top of the function but without any lock held, which means the control socket state may certainly change. Add a similar protection to ngs_shutdown() even if a race is unlikely to be experienced there. Sponsored by: Sandvine Incorporated Obtained from: Nima Misaghian @ Sandvine Incorporated <nmisaghian at sandvine dot com> MFC after: 10 days Modified: head/sys/netgraph/ng_socket.c Modified: head/sys/netgraph/ng_socket.c ============================================================================== --- head/sys/netgraph/ng_socket.c Wed May 19 14:50:07 2010 (r208299) +++ head/sys/netgraph/ng_socket.c Wed May 19 15:06:09 2010 (r208300) @@ -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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201005191506.o4JF69SL007708>