From owner-svn-src-all@FreeBSD.ORG Wed Mar 10 13:23:26 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4B823106566B; Wed, 10 Mar 2010 13:23:26 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 3A80A8FC1E; Wed, 10 Mar 2010 13:23:26 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o2ADNQDr048674; Wed, 10 Mar 2010 13:23:26 GMT (envelope-from jhb@svn.freebsd.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o2ADNQgF048667; Wed, 10 Mar 2010 13:23:26 GMT (envelope-from jhb@svn.freebsd.org) Message-Id: <201003101323.o2ADNQgF048667@svn.freebsd.org> From: John Baldwin Date: Wed, 10 Mar 2010 13:23:26 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r204950 - head/lib/libc/rpc X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Mar 2010 13:23:26 -0000 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,