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
PR:		156168
Reviewed by:	dfr
MFC after:	2 weeks

1 files changed, 2 insertions, 1 deletions

diff --git a/sys/rpc/clnt_dg.c b/sys/rpc/clnt_dg.c
index 8a69bf4..c86b18a 100644
--- a/sys/rpc/clnt_dg.c
+++ b/sys/rpc/clnt_dg.c
@@ -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;
 	}