Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 17 Nov 2011 16:38:22 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org
Subject:   svn commit: r227631 - releng/9.0/sys/rpc
Message-ID:  <201111171638.pAHGcMcK018249@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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;
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201111171638.pAHGcMcK018249>