From owner-svn-src-all@FreeBSD.ORG Tue Nov 27 22:41:17 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id CFE1ABFE for ; Tue, 27 Nov 2012 22:41:17 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.freebsd.org (Postfix) with ESMTP id 20EE88FC15 for ; Tue, 27 Nov 2012 22:41:16 +0000 (UTC) Received: (qmail 37223 invoked from network); 28 Nov 2012 00:12:51 -0000 Received: from c00l3r.networx.ch (HELO [127.0.0.1]) ([62.48.2.2]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 28 Nov 2012 00:12:51 -0000 Message-ID: <50B54180.5020608@freebsd.org> Date: Tue, 27 Nov 2012 23:41:04 +0100 From: Andre Oppermann User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20121026 Thunderbird/16.0.2 MIME-Version: 1.0 To: Peter Wemm Subject: Re: svn commit: r243627 - head/sys/kern References: <201211272004.qARK4qS8047209@svn.freebsd.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Nov 2012 22:41:18 -0000 On 27.11.2012 23:35, Peter Wemm wrote: > Andre.. this breaks incoming connections. TCP is immediately reset > and never even gets to the listener process. You need to back out of > fix this urgently please. I just found out and fixed it. Sorry for the breakage. -- Andre > On Tue, Nov 27, 2012 at 12:04 PM, Andre Oppermann wrote: >> Author: andre >> Date: Tue Nov 27 20:04:52 2012 >> New Revision: 243627 >> URL: http://svnweb.freebsd.org/changeset/base/243627 >> >> Log: >> Fix a race on listen socket teardown where while draining the >> accept queues a new socket/connection may be added to the queue >> due to a race on the ACCEPT_LOCK. >> >> The submitted patch is slightly changed in comments, teardown >> and locking order and extended with KASSERT's. >> >> Submitted by: Vijay Singh >> Found by: His team. >> MFC after: 1 week >> >> Modified: >> head/sys/kern/uipc_socket.c >> >> Modified: head/sys/kern/uipc_socket.c >> ============================================================================== >> --- head/sys/kern/uipc_socket.c Tue Nov 27 19:35:21 2012 (r243626) >> +++ head/sys/kern/uipc_socket.c Tue Nov 27 20:04:52 2012 (r243627) >> @@ -555,6 +555,16 @@ sonewconn(struct socket *head, int conns >> so->so_snd.sb_flags |= head->so_snd.sb_flags & SB_AUTOSIZE; >> so->so_state |= connstatus; >> ACCEPT_LOCK(); >> + /* >> + * The accept socket may be tearing down but we just >> + * won a race on the ACCEPT_LOCK. >> + */ >> + if (!(so->so_options & SO_ACCEPTCONN)) { >> + SOCK_LOCK(so); >> + so->so_head = NULL; >> + sofree(so); /* NB: returns ACCEPT_UNLOCK'ed. */ >> + return (NULL); >> + } >> if (connstatus) { >> TAILQ_INSERT_TAIL(&head->so_comp, so, so_list); >> so->so_qstate |= SQ_COMP; >> @@ -780,9 +790,14 @@ soclose(struct socket *so) >> drop: >> if (so->so_proto->pr_usrreqs->pru_close != NULL) >> (*so->so_proto->pr_usrreqs->pru_close)(so); >> + ACCEPT_LOCK(); >> if (so->so_options & SO_ACCEPTCONN) { >> struct socket *sp; >> - ACCEPT_LOCK(); >> + /* >> + * Prevent new additions to the accept queues due >> + * to ACCEPT_LOCK races while we are draining them. >> + */ >> + so->so_options &= ~SO_ACCEPTCONN; >> while ((sp = TAILQ_FIRST(&so->so_incomp)) != NULL) { >> TAILQ_REMOVE(&so->so_incomp, sp, so_list); >> so->so_incqlen--; >> @@ -801,13 +816,15 @@ drop: >> soabort(sp); >> ACCEPT_LOCK(); >> } >> - ACCEPT_UNLOCK(); >> + KASSERT((TAILQ_EMPTY(&so->so_comp)), >> + ("%s: so_comp populated", __func__)); >> + KASSERT((TAILQ_EMPTY(&so->so_incomp)), >> + ("%s: so_incomp populated", __func__)); >> } >> - ACCEPT_LOCK(); >> SOCK_LOCK(so); >> KASSERT((so->so_state & SS_NOFDREF) == 0, ("soclose: NOFDREF")); >> so->so_state |= SS_NOFDREF; >> - sorele(so); >> + sorele(so); /* NB: Returns with ACCEPT_UNLOCK(). */ >> CURVNET_RESTORE(); >> return (error); >> } > > >