Date: Tue, 27 Nov 2012 14:35:42 -0800 From: Peter Wemm <peter@wemm.org> To: Andre Oppermann <andre@freebsd.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: <CAGE5yCpxOdsjefe6quR_gjs82pk9a2e_H_WUNUWhUGA3WZPJaw@mail.gmail.com> In-Reply-To: <201211272004.qARK4qS8047209@svn.freebsd.org> References: <201211272004.qARK4qS8047209@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. 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); > } -- Peter Wemm - peter@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com; KI6FJV "All of this is for nothing if we don't go to the stars" - JMS/B5 "If Java had true garbage collection, most programs would delete themselves upon execution." -- Robert Sewell
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGE5yCpxOdsjefe6quR_gjs82pk9a2e_H_WUNUWhUGA3WZPJaw>