From owner-svn-src-head@FreeBSD.ORG Tue Feb 1 13:33:49 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 907A0106566B; Tue, 1 Feb 2011 13:33:49 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 74AFB8FC13; Tue, 1 Feb 2011 13:33:49 +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 p11DXnab073369; Tue, 1 Feb 2011 13:33:49 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id p11DXnJN073367; Tue, 1 Feb 2011 13:33:49 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201102011333.p11DXnJN073367@svn.freebsd.org> From: Konstantin Belousov Date: Tue, 1 Feb 2011 13:33:49 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r218168 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Feb 2011 13:33:49 -0000 Author: kib Date: Tue Feb 1 13:33:49 2011 New Revision: 218168 URL: http://svn.freebsd.org/changeset/base/218168 Log: The unp_gc() function drops and reaquires lock between scan and collect phases. The unp_discard() function executes unp_externalize_fp(), which might make the socket eligible for gc-ing, and then, later, taskqueue will close the socket. Since unp_gc() dropped the list lock to do the malloc, close might happen after the mark step but before the collection step, causing collection to not find the socket and miss one array element. I believe that the race was there before r216158, but the stated revision made the window much wider by postponing the close to taskqueue sometimes. Only process as much array elements as we find the sockets during second phase of gc [1]. Take linkage lock and recheck the eligibility of the socket for gc, as well as call fhold() under the linkage lock. Reported and tested by: jmallett Submitted by: jmallett [1] Reviewed by: rwatson, jeff (possibly) MFC after: 1 week Modified: head/sys/kern/uipc_usrreq.c Modified: head/sys/kern/uipc_usrreq.c ============================================================================== --- head/sys/kern/uipc_usrreq.c Tue Feb 1 13:32:27 2011 (r218167) +++ head/sys/kern/uipc_usrreq.c Tue Feb 1 13:33:49 2011 (r218168) @@ -2153,9 +2153,9 @@ unp_gc(__unused void *arg, int pending) struct unp_head *heads[] = { &unp_dhead, &unp_shead, &unp_sphead, NULL }; struct unp_head **head; - struct file **unref; + struct file *f, **unref; struct unpcb *unp; - int i; + int i, total; unp_taskcount++; UNP_LIST_LOCK(); @@ -2193,33 +2193,37 @@ unp_gc(__unused void *arg, int pending) * Iterate looking for sockets which have been specifically marked * as as unreachable and store them locally. */ + UNP_LINK_RLOCK(); UNP_LIST_LOCK(); - for (i = 0, head = heads; *head != NULL; head++) + for (total = 0, head = heads; *head != NULL; head++) LIST_FOREACH(unp, *head, unp_link) - if (unp->unp_gcflag & UNPGC_DEAD) { - unref[i++] = unp->unp_file; - fhold(unp->unp_file); - KASSERT(unp->unp_file != NULL, - ("unp_gc: Invalid unpcb.")); - KASSERT(i <= unp_unreachable, + if ((unp->unp_gcflag & UNPGC_DEAD) != 0) { + f = unp->unp_file; + if (unp->unp_msgcount == 0 || f == NULL || + f->f_count != unp->unp_msgcount) + continue; + unref[total++] = f; + fhold(f); + KASSERT(total <= unp_unreachable, ("unp_gc: incorrect unreachable count.")); } UNP_LIST_UNLOCK(); + UNP_LINK_RUNLOCK(); /* * Now flush all sockets, free'ing rights. This will free the * struct files associated with these sockets but leave each socket * with one remaining ref. */ - for (i = 0; i < unp_unreachable; i++) + for (i = 0; i < total; i++) sorflush(unref[i]->f_data); /* * And finally release the sockets so they can be reclaimed. */ - for (i = 0; i < unp_unreachable; i++) + for (i = 0; i < total; i++) fdrop(unref[i], NULL); - unp_recycled += unp_unreachable; + unp_recycled += total; free(unref, M_TEMP); }