From owner-svn-src-all@FreeBSD.ORG Thu Nov 17 16:38:22 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F29F61065679; Thu, 17 Nov 2011 16:38:22 +0000 (UTC) (envelope-from rmacklem@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id C813C8FC28; Thu, 17 Nov 2011 16:38:22 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id pAHGcMKe018251; Thu, 17 Nov 2011 16:38:22 GMT (envelope-from rmacklem@svn.freebsd.org) Received: (from rmacklem@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id pAHGcMcK018249; Thu, 17 Nov 2011 16:38:22 GMT (envelope-from rmacklem@svn.freebsd.org) Message-Id: <201111171638.pAHGcMcK018249@svn.freebsd.org> From: Rick Macklem Date: Thu, 17 Nov 2011 16:38:22 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org X-SVN-Group: releng MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r227631 - releng/9.0/sys/rpc X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 17 Nov 2011 16:38:23 -0000 Author: rmacklem Date: Thu Nov 17 16:38:22 2011 New Revision: 227631 URL: http://svn.freebsd.org/changeset/base/227631 Log: MFC: r227059 Both a crash reported on freebsd-current on Oct. 18 under the subject heading "mtx_lock() of destroyed mutex on NFS" and PR# 156168 appear to be caused by clnt_dg_destroy() closing down the socket prematurely. When to close down the socket is controlled by a reference count (cs_refs), but clnt_dg_create() checks for sb_upcall being non-NULL to decide if a new socket is needed. I believe the crashes were caused by the following race: clnt_dg_destroy() finds cs_refs == 0 and decides to delete socket clnt_dg_destroy() then loses race with clnt_dg_create() for acquisition of the SOCKBUF_LOCK() clnt_dg_create() finds sb_upcall != NULL and increments cs_refs to 1 clnt_dg_destroy() then acquires SOCKBUF_LOCK(), sets sb_upcall to NULL and destroys socket This patch fixes the above race by changing clnt_dg_destroy() so that it acquires SOCKBUF_LOCK() before testing cs_refs. Tested by: bz Reviewed by: dfr Approved by: re (kib) Modified: releng/9.0/sys/rpc/clnt_dg.c Directory Properties: releng/9.0/sys/ (props changed) releng/9.0/sys/amd64/include/xen/ (props changed) releng/9.0/sys/boot/ (props changed) releng/9.0/sys/boot/i386/efi/ (props changed) releng/9.0/sys/boot/ia64/efi/ (props changed) releng/9.0/sys/boot/ia64/ski/ (props changed) releng/9.0/sys/boot/powerpc/boot1.chrp/ (props changed) releng/9.0/sys/boot/powerpc/ofw/ (props changed) releng/9.0/sys/cddl/contrib/opensolaris/ (props changed) releng/9.0/sys/conf/ (props changed) releng/9.0/sys/contrib/dev/acpica/ (props changed) releng/9.0/sys/contrib/octeon-sdk/ (props changed) releng/9.0/sys/contrib/pf/ (props changed) releng/9.0/sys/contrib/x86emu/ (props changed) Modified: releng/9.0/sys/rpc/clnt_dg.c ============================================================================== --- releng/9.0/sys/rpc/clnt_dg.c Thu Nov 17 16:08:46 2011 (r227630) +++ releng/9.0/sys/rpc/clnt_dg.c Thu Nov 17 16:38:22 2011 (r227631) @@ -1001,12 +1001,12 @@ clnt_dg_destroy(CLIENT *cl) cs = cu->cu_socket->so_rcv.sb_upcallarg; clnt_dg_close(cl); + SOCKBUF_LOCK(&cu->cu_socket->so_rcv); mtx_lock(&cs->cs_lock); cs->cs_refs--; if (cs->cs_refs == 0) { mtx_unlock(&cs->cs_lock); - SOCKBUF_LOCK(&cu->cu_socket->so_rcv); soupcall_clear(cu->cu_socket, SO_RCV); clnt_dg_upcallsdone(cu->cu_socket, cs); SOCKBUF_UNLOCK(&cu->cu_socket->so_rcv); @@ -1015,6 +1015,7 @@ clnt_dg_destroy(CLIENT *cl) lastsocketref = TRUE; } else { mtx_unlock(&cs->cs_lock); + SOCKBUF_UNLOCK(&cu->cu_socket->so_rcv); lastsocketref = FALSE; }