lib/59475: libc/rpc: clnt_vc_create: Do not hold a global mutex during connect

[pfg%freebsd.org@localhost]

>Number:         59475
>Category:       lib
>Synopsis:       libc/rpc: clnt_vc_create: Do not hold a global mutex during connect
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 20 14:40:01 +0000 2025
>Originator:     Pedro Giffuni
>Release:        all releases
>Organization:
FreeBSD
>Environment:
N/A
>Description:
Don't hold a mutex when connecting

A multi-threaded application, connecting to multiple rpc servers,
may dead lock if the connect call stalls on a non responsive server.

That's why the kind linux tirpc guys have these two changes in their system (under a 3 clause BSD license, since the code is derived from FreeBSD):

http://git.linux-nfs.org/?p=steved/libtirpc.git;a=commit;h=644b861b01a0b53a9b920e50f68e4f1252000bbd
http://git.linux-nfs.org/?p=steved/libtirpc.git;a=commit;h=45ceae856f314f7367695e81e835ba4b306cd335

I sort of put them together in a patch but its untested.
>How-To-Repeat:
N/A
>Fix:
This is against FreeBSD but should apply pretty cleanly to NetBSD.
diff --git a/lib/libc/rpc/clnt_vc.c b/lib/libc/rpc/clnt_vc.c
index 60e381168ed3..2a3aa4469ac2 100644
--- a/lib/libc/rpc/clnt_vc.c
+++ b/lib/libc/rpc/clnt_vc.c
@@ -224,24 +224,24 @@ clnt_vc_create(int fd, const struct netbuf *raddr, const rpcprog_t prog,
 	ct->ct_addr.buf = NULL;
 
 	/*
-	 * XXX - fvdl connecting while holding a mutex?
+	 * Do not hold mutex during connect
 	 */
+	mutex_unlock(&clnt_fd_lock);
+
 	slen = sizeof ss;
 	if (_getpeername(fd, (struct sockaddr *)(void *)&ss, &slen) < 0) {
 		if (errno != ENOTCONN) {
 			rpc_createerr.cf_stat = RPC_SYSTEMERROR;
 			rpc_createerr.cf_error.re_errno = errno;
-			mutex_unlock(&clnt_fd_lock);
 			goto err;
 		}
-		if (_connect(fd, (struct sockaddr *)raddr->buf, raddr->len) < 0){
-			rpc_createerr.cf_stat = RPC_SYSTEMERROR;
-			rpc_createerr.cf_error.re_errno = errno;
-			mutex_unlock(&clnt_fd_lock);
-			goto err;
+		if ((_connect(fd, (struct sockaddr *)raddr->buf, raddr->len) < 0) &&
+			(errno != EISCONN) ) {
+				rpc_createerr.cf_stat = RPC_SYSTEMERROR;
+				rpc_createerr.cf_error.re_errno = errno;
+				goto err;
 		}
 	}
-	mutex_unlock(&clnt_fd_lock);
 	if (!__rpc_fd2sockinfo(fd, &si))
 		goto err;