Author: rmacklem
Date: Thu Nov 17 16:08:46 2011
New Revision: 227630
URL: http://svn.freebsd.org/changeset/base/227630
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:
stable/9/sys/rpc/clnt_dg.c
Directory Properties:
stable/9/sys/ (props changed)
stable/9/sys/amd64/include/xen/ (props changed)
stable/9/sys/boot/ (props changed)
stable/9/sys/boot/i386/efi/ (props changed)
stable/9/sys/boot/ia64/efi/ (props changed)
stable/9/sys/boot/ia64/ski/ (props changed)
stable/9/sys/boot/powerpc/boot1.chrp/ (props changed)
stable/9/sys/boot/powerpc/ofw/ (props changed)
stable/9/sys/cddl/contrib/opensolaris/ (props changed)
stable/9/sys/conf/ (props changed)
stable/9/sys/contrib/dev/acpica/ (props changed)
stable/9/sys/contrib/octeon-sdk/ (props changed)
stable/9/sys/contrib/pf/ (props changed)
stable/9/sys/contrib/x86emu/ (props changed)
Modified: stable/9/sys/rpc/clnt_dg.c
==============================================================================
--- stable/9/sys/rpc/clnt_dg.c Thu Nov 17 15:51:03 2011 (r227629)
+++ stable/9/sys/rpc/clnt_dg.c Thu Nov 17 16:08:46 2011 (r227630)
@@ -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;
}