Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Mar 2010 13:23:26 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r204950 - head/lib/libc/rpc
Message-ID:  <201003101323.o2ADNQgF048667@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Wed Mar 10 13:23:25 2010
New Revision: 204950
URL: http://svn.freebsd.org/changeset/base/204950

Log:
  Use thr_once() with once_t controls to initialize various thread_key_t
  objects used to provide per-thread storage in the RPC code.  Almost all
  of these used double-checking with a dedicated mutex (tsd_lock) to do this
  before.  However, that is not always safe with more relaxed memory orders.
  There were also other bugs, such as one in __rpc_createrr() that caused a
  new key to be allocated each time __rpc_createrr() was invoked.
  
  PR:		threads/144558
  Reported by:	Sam Robb  samrobb of averesystems com (key leak)
  MFC after:	1 week

Modified:
  head/lib/libc/rpc/Symbol.map
  head/lib/libc/rpc/clnt_simple.c
  head/lib/libc/rpc/getnetconfig.c
  head/lib/libc/rpc/key_call.c
  head/lib/libc/rpc/mt_misc.c
  head/lib/libc/rpc/mt_misc.h
  head/lib/libc/rpc/rpc_generic.c
  head/lib/libc/rpc/rpc_soc.c

Modified: head/lib/libc/rpc/Symbol.map
==============================================================================
--- head/lib/libc/rpc/Symbol.map	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/Symbol.map	Wed Mar 10 13:23:25 2010	(r204950)
@@ -239,10 +239,6 @@ FBSDprivate_1.0 {
 	__key_encryptsession_pk_LOCAL;
 	__key_decryptsession_pk_LOCAL;
 	__key_gendes_LOCAL;
-	__tsd_lock;	/*
-			 * Why does usr.bin/rpcinfo/Makefile need rpc_generic.c?
-			 * Remove this hack if rpcinfo stops building with it.
-			 */
 	__svc_clean_idle;
 	__rpc_gss_unwrap;
 	__rpc_gss_unwrap_stub;

Modified: head/lib/libc/rpc/clnt_simple.c
==============================================================================
--- head/lib/libc/rpc/clnt_simple.c	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/clnt_simple.c	Wed Mar 10 13:23:25 2010	(r204950)
@@ -76,7 +76,11 @@ struct rpc_call_private {
 	char	nettype[NETIDLEN];	/* Network type */
 };
 static struct rpc_call_private *rpc_call_private_main;
+static thread_key_t rpc_call_key;
+static once_t rpc_call_once = ONCE_INITIALIZER;
+static int rpc_call_key_error;
 
+static void rpc_call_key_init(void);
 static void rpc_call_destroy(void *);
 
 static void
@@ -91,6 +95,13 @@ rpc_call_destroy(void *vp)
 	}
 }
 
+static void
+rpc_call_key_init(void)
+{
+
+	rpc_call_key_error = thr_keycreate(&rpc_call_key, rpc_call_destroy);
+}
+
 /*
  * This is the simplified interface to the client rpc layer.
  * The client handle is not destroyed here and is reused for
@@ -112,17 +123,16 @@ rpc_call(host, prognum, versnum, procnum
 	struct rpc_call_private *rcp = (struct rpc_call_private *) 0;
 	enum clnt_stat clnt_stat;
 	struct timeval timeout, tottimeout;
-	static thread_key_t rpc_call_key;
 	int main_thread = 1;
 
 	if ((main_thread = thr_main())) {
 		rcp = rpc_call_private_main;
 	} else {
-		if (rpc_call_key == 0) {
-			mutex_lock(&tsd_lock);
-			if (rpc_call_key == 0)
-				thr_keycreate(&rpc_call_key, rpc_call_destroy);
-			mutex_unlock(&tsd_lock);
+		if (thr_once(&rpc_call_once, rpc_call_key_init) != 0 ||
+		    rpc_call_key_error != 0) {
+			rpc_createerr.cf_stat = RPC_SYSTEMERROR;
+			rpc_createerr.cf_error.re_errno = rpc_call_key_error;
+			return (rpc_createerr.cf_stat);
 		}
 		rcp = (struct rpc_call_private *)thr_getspecific(rpc_call_key);
 	}

Modified: head/lib/libc/rpc/getnetconfig.c
==============================================================================
--- head/lib/libc/rpc/getnetconfig.c	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/getnetconfig.c	Wed Mar 10 13:23:25 2010	(r204950)
@@ -130,21 +130,29 @@ static struct netconfig *dup_ncp(struct 
 
 
 static FILE *nc_file;		/* for netconfig db */
-static pthread_mutex_t nc_file_lock = PTHREAD_MUTEX_INITIALIZER;
+static mutex_t nc_file_lock = MUTEX_INITIALIZER;
 
 static struct netconfig_info	ni = { 0, 0, NULL, NULL};
-static pthread_mutex_t ni_lock = PTHREAD_MUTEX_INITIALIZER;
+static mutex_t ni_lock = MUTEX_INITIALIZER;
 
+static thread_key_t nc_key;
+static once_t nc_once = ONCE_INITIALIZER;
+static int nc_key_error;
+
+static void
+nc_key_init(void)
+{
+
+	nc_key_error = thr_keycreate(&nc_key, free);
+}
 
 #define MAXNETCONFIGLINE    1000
 
 static int *
 __nc_error()
 {
-	static pthread_mutex_t nc_lock = PTHREAD_MUTEX_INITIALIZER;
-	static thread_key_t nc_key = 0;
 	static int nc_error = 0;
-	int error, *nc_addr;
+	int *nc_addr;
 
 	/*
 	 * Use the static `nc_error' if we are the main thread
@@ -153,15 +161,8 @@ __nc_error()
 	 */
 	if (thr_main())
 		return (&nc_error);
-	if (nc_key == 0) {
-		error = 0;
-		mutex_lock(&nc_lock);
-		if (nc_key == 0)
-			error = thr_keycreate(&nc_key, free);
-		mutex_unlock(&nc_lock);
-		if (error)
-			return (&nc_error);
-	}
+	if (thr_once(&nc_once, nc_key_init) != 0 || nc_key_error != 0)
+		return (&nc_error);
 	if ((nc_addr = (int *)thr_getspecific(nc_key)) == NULL) {
 		nc_addr = (int *)malloc(sizeof (int));
 		if (thr_setspecific(nc_key, (void *) nc_addr) != 0) {

Modified: head/lib/libc/rpc/key_call.c
==============================================================================
--- head/lib/libc/rpc/key_call.c	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/key_call.c	Wed Mar 10 13:23:25 2010	(r204950)
@@ -279,6 +279,9 @@ struct  key_call_private {
 	uid_t	uid;		/* user-id at last authorization */
 };
 static struct key_call_private *key_call_private_main = NULL;
+static thread_key_t key_call_key;
+static once_t key_call_once = ONCE_INITIALIZER;
+static int key_call_key_error;
 
 static void
 key_call_destroy(void *vp)
@@ -292,6 +295,13 @@ key_call_destroy(void *vp)
 	}
 }
 
+static void
+key_call_init(void)
+{
+
+	key_call_key_error = thr_keycreate(&key_call_key, key_call_destroy);
+}
+
 /*
  * Keep the handle cached.  This call may be made quite often.
  */
@@ -307,7 +317,6 @@ int	vers;
 	struct utsname u;
 	int main_thread;
 	int fd;
-	static thread_key_t key_call_key;
 
 #define	TOTAL_TIMEOUT	30	/* total timeout talking to keyserver */
 #define	TOTAL_TRIES	5	/* Number of tries */
@@ -315,12 +324,9 @@ int	vers;
 	if ((main_thread = thr_main())) {
 		kcp = key_call_private_main;
 	} else {
-		if (key_call_key == 0) {
-			mutex_lock(&tsd_lock);
-			if (key_call_key == 0)
-				thr_keycreate(&key_call_key, key_call_destroy);
-			mutex_unlock(&tsd_lock);
-		}
+		if (thr_once(&key_call_once, key_call_init) != 0 ||
+		    key_call_key_error != 0)
+			return ((CLIENT *) NULL);
 		kcp = (struct key_call_private *)thr_getspecific(key_call_key);
 	}	
 	if (kcp == (struct key_call_private *)NULL) {

Modified: head/lib/libc/rpc/mt_misc.c
==============================================================================
--- head/lib/libc/rpc/mt_misc.c	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/mt_misc.c	Wed Mar 10 13:23:25 2010	(r204950)
@@ -28,7 +28,6 @@ __FBSDID("$FreeBSD$");
 #define	proglst_lock		__proglst_lock
 #define	rpcsoc_lock		__rpcsoc_lock
 #define	svcraw_lock		__svcraw_lock
-#define	tsd_lock		__tsd_lock
 #define	xprtlist_lock		__xprtlist_lock
 
 /* protects the services list (svc.c) */
@@ -76,33 +75,33 @@ pthread_mutex_t	rpcsoc_lock = PTHREAD_MU
 /* svc_raw.c serialization */
 pthread_mutex_t	svcraw_lock = PTHREAD_MUTEX_INITIALIZER;
 
-/* protects TSD key creation */
-pthread_mutex_t	tsd_lock = PTHREAD_MUTEX_INITIALIZER;
-
 /* xprtlist (svc_generic.c) */
 pthread_mutex_t	xprtlist_lock = PTHREAD_MUTEX_INITIALIZER;
 
 #undef	rpc_createerr
 
 struct rpc_createerr rpc_createerr;
+static thread_key_t rce_key;
+static once_t rce_once = ONCE_INITIALIZER;
+static int rce_key_error;
+
+static void
+rce_key_init(void)
+{
+
+	rce_key_error = thr_keycreate(&rce_key, free);
+}
 
 struct rpc_createerr *
 __rpc_createerr()
 {
-	static thread_key_t rce_key = 0;
 	struct rpc_createerr *rce_addr = 0;
 
 	if (thr_main())
 		return (&rpc_createerr);
-	if ((rce_addr =
-	    (struct rpc_createerr *)thr_getspecific(rce_key)) != 0) {
-		mutex_lock(&tsd_lock);
-		if (thr_keycreate(&rce_key, free) != 0) {
-			mutex_unlock(&tsd_lock);
-			return (&rpc_createerr);
-		}
-		mutex_unlock(&tsd_lock);
-	}
+	if (thr_once(&rce_once, rce_key_init) != 0 || rce_key_error != 0)
+		return (&rpc_createerr);
+	rce_addr = (struct rpc_createerr *)thr_getspecific(rce_key);
 	if (!rce_addr) {
 		rce_addr = (struct rpc_createerr *)
 			malloc(sizeof (struct rpc_createerr));

Modified: head/lib/libc/rpc/mt_misc.h
==============================================================================
--- head/lib/libc/rpc/mt_misc.h	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/mt_misc.h	Wed Mar 10 13:23:25 2010	(r204950)
@@ -42,7 +42,6 @@
 #define	proglst_lock		__proglst_lock
 #define	rpcsoc_lock		__rpcsoc_lock
 #define	svcraw_lock		__svcraw_lock
-#define	tsd_lock		__tsd_lock
 #define	xprtlist_lock		__xprtlist_lock
 
 extern pthread_rwlock_t	svc_lock;

Modified: head/lib/libc/rpc/rpc_generic.c
==============================================================================
--- head/lib/libc/rpc/rpc_generic.c	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/rpc_generic.c	Wed Mar 10 13:23:25 2010	(r204950)
@@ -221,6 +221,18 @@ getnettype(nettype)
 	return (_rpctypelist[i].type);
 }
 
+static thread_key_t tcp_key, udp_key;
+static once_t keys_once = ONCE_INITIALIZER;
+static int tcp_key_error, udp_key_error;
+
+static void
+keys_init(void)
+{
+
+	tcp_key_error = thr_keycreate(&tcp_key, free);
+	udp_key_error = thr_keycreate(&udp_key, free);
+}
+
 /*
  * For the given nettype (tcp or udp only), return the first structure found.
  * This should be freed by calling freenetconfigent()
@@ -242,19 +254,10 @@ __rpc_getconfip(nettype)
 		netid_udp = netid_udp_main;
 		netid_tcp = netid_tcp_main;
 	} else {
-		if (tcp_key == 0) {
-			mutex_lock(&tsd_lock);
-			if (tcp_key == 0)
-				thr_keycreate(&tcp_key, free);
-			mutex_unlock(&tsd_lock);
-		}
+		if (thr_once(&keys_once, keys_init) != 0 ||
+		    tcp_key_error != 0 || udp_key_error != 0)
+			return (NULL);
 		netid_tcp = (char *)thr_getspecific(tcp_key);
-		if (udp_key == 0) {
-			mutex_lock(&tsd_lock);
-			if (udp_key == 0)
-				thr_keycreate(&udp_key, free);
-			mutex_unlock(&tsd_lock);
-		}
 		netid_udp = (char *)thr_getspecific(udp_key);
 	}
 	if (!netid_udp && !netid_tcp) {

Modified: head/lib/libc/rpc/rpc_soc.c
==============================================================================
--- head/lib/libc/rpc/rpc_soc.c	Wed Mar 10 11:33:15 2010	(r204949)
+++ head/lib/libc/rpc/rpc_soc.c	Wed Mar 10 13:23:25 2010	(r204950)
@@ -360,6 +360,14 @@ registerrpc(prognum, versnum, procnum, p
  */
 static thread_key_t	clnt_broadcast_key;
 static resultproc_t	clnt_broadcast_result_main;
+static once_t		clnt_broadcast_once = ONCE_INITIALIZER;
+
+static void
+clnt_broadcast_key_init(void)
+{
+
+	thr_keycreate(&clnt_broadcast_key, free);
+}
 
 /*
  * Need to translate the netbuf address into sockaddr_in address.
@@ -402,12 +410,7 @@ clnt_broadcast(prog, vers, proc, xargs, 
 	if (thr_main())
 		clnt_broadcast_result_main = eachresult;
 	else {
-		if (clnt_broadcast_key == 0) {
-			mutex_lock(&tsd_lock);
-			if (clnt_broadcast_key == 0)
-				thr_keycreate(&clnt_broadcast_key, free);
-			mutex_unlock(&tsd_lock);
-		}
+		thr_once(&clnt_broadcast_once, clnt_broadcast_key_init);
 		thr_setspecific(clnt_broadcast_key, (void *) eachresult);
 	}
 	return rpc_broadcast((rpcprog_t)prog, (rpcvers_t)vers,



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