Date: Tue, 27 Nov 2012 23:41:04 +0100 From: Andre Oppermann <andre@freebsd.org> To: Peter Wemm <peter@wemm.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r243627 - head/sys/kern Message-ID: <50B54180.5020608@freebsd.org> In-Reply-To: <CAGE5yCpxOdsjefe6quR_gjs82pk9a2e_H_WUNUWhUGA3WZPJaw@mail.gmail.com> References: <201211272004.qARK4qS8047209@svn.freebsd.org> <CAGE5yCpxOdsjefe6quR_gjs82pk9a2e_H_WUNUWhUGA3WZPJaw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <andre@freebsd.org> 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 <vijju.singh-at-gmail-dot-com> >> 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); >> } > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50B54180.5020608>