From: deraadt <deraadt@openbsd.org>
Date: Tue, 2 Aug 2022 16:59:29 +0000 (+0000)
Subject: 1) The yp_bind/yp_unbind and internal _yp_dobind/_yp_unbind sequences shared
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=2d62dde8310c0f11b45f14adaba36cf1950be6bf;p=openbsd

1) The yp_bind/yp_unbind and internal _yp_dobind/_yp_unbind sequences shared
dom_binding structs between threads, which is unsafe -- example, dom_vers
signalled retry events, and structs+socket would get deallocated in _yp_unbind.
Change all yp_first (and similar) functions to understand that _yp_dobind now
provides a private dom_binding and socket, which must be released using
_yp_unbind.  Use similar methods in the one-step yp_all function.
2) domainname caching in get* is not neccessary now that the domainname cannot
change relative to ypconnect(2)'s decisions.
Many fields in dom_binding struct become unused, so delete them.
ok jmatthew, also tested by miod
---

diff --git a/lib/libc/yp/yp_all.c b/lib/libc/yp/yp_all.c
index 04a6ebe2b49..204f085ccc8 100644
--- a/lib/libc/yp/yp_all.c
+++ b/lib/libc/yp/yp_all.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_all.c,v 1.15 2022/07/18 02:31:19 deraadt Exp $ */
+/*	$OpenBSD: yp_all.c,v 1.16 2022/08/02 16:59:29 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -101,7 +101,7 @@ int
 yp_all(const char *dom, const char *inmap, struct ypall_callback *incallback)
 {
 	struct ypreq_nokey yprnk;
-	struct dom_binding *ypbinding;
+	struct dom_binding ypbinding;
 	struct timeval  tv;
 	int connected = 1;
 	u_long		status;
@@ -118,24 +118,18 @@ again:
 	s = ypconnect(SOCK_STREAM);
 	if (s == -1)
 		return YPERR_DOMAIN;	/* YP not running */
+	ypbinding.dom_socket = s;
+	ypbinding.dom_server_addr.sin_port = -1; /* don't consult portmap */
 
-	ypbinding = calloc(1, sizeof *ypbinding);
-	if (ypbinding == NULL) {
-		close(s);
-		return YPERR_RESRC;
-	}
-	ypbinding->dom_socket = s;
-	ypbinding->dom_server_addr.sin_port = -1; /* don't consult portmap */
-
-	ypbinding->dom_client = clnttcp_create(&ypbinding->dom_server_addr,
-	    YPPROG, YPVERS, &ypbinding->dom_socket, 0, 0);
-	if (ypbinding->dom_client == NULL) {
-		close(ypbinding->dom_socket);
-		free(ypbinding);
+	ypbinding.dom_client = clnttcp_create(&ypbinding.dom_server_addr,
+	    YPPROG, YPVERS, &ypbinding.dom_socket, 0, 0);
+	if (ypbinding.dom_client == NULL) {
+		close(ypbinding.dom_socket);
+		ypbinding.dom_socket = -1;
 		clnt_pcreateerror("clnttcp_create");
 		goto again;
 	}
-	clnt_control(ypbinding->dom_client, CLSET_CONNECTED, &connected);
+	clnt_control(ypbinding.dom_client, CLSET_CONNECTED, &connected);
 
 	tv.tv_sec = _yplib_timeout;
 	tv.tv_usec = 0;
@@ -143,11 +137,10 @@ again:
 	yprnk.map = (char *)inmap;
 	ypresp_allfn = incallback->foreach;
 	ypresp_data = (void *) incallback->data;
-	(void) clnt_call(ypbinding->dom_client, YPPROC_ALL,
+	(void) clnt_call(ypbinding.dom_client, YPPROC_ALL,
 	    xdr_ypreq_nokey, &yprnk, _xdr_ypresp_all_seq, &status, tv);
-	close(ypbinding->dom_socket);
-	clnt_destroy(ypbinding->dom_client);
-	free(ypbinding);
+	close(ypbinding.dom_socket);
+	clnt_destroy(ypbinding.dom_client);
 
 	if (status != YP_FALSE)
 		r = ypprot_err(status);
diff --git a/lib/libc/yp/yp_bind.c b/lib/libc/yp/yp_bind.c
index 15db9ea7730..af6775e96b8 100644
--- a/lib/libc/yp/yp_bind.c
+++ b/lib/libc/yp/yp_bind.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_bind.c,v 1.31 2022/07/22 05:55:05 jsg Exp $ */
+/*	$OpenBSD: yp_bind.c,v 1.32 2022/08/02 16:59:29 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993, 1996 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -43,13 +43,13 @@
 #include <rpcsvc/ypclnt.h>
 #include "ypinternal.h"
 
-static struct dom_binding *ypbinding;
 char _yp_domain[HOST_NAME_MAX+1];
 int _yplib_timeout = 10;
 
 int
 _yp_dobind(const char *dom, struct dom_binding **ypdb)
 {
+	struct dom_binding *ypbinding;
 	struct timeval tv;
 	int connected = 1;
 	int s;
@@ -57,19 +57,15 @@ _yp_dobind(const char *dom, struct dom_binding **ypdb)
 	if (dom == NULL || strlen(dom) == 0)
 		return YPERR_BADARGS;
 
-again:
-	if (ypbinding && ypbinding->dom_client)
-		_yp_unbind(ypbinding);
+	ypbinding = calloc(1, sizeof (*ypbinding));
+	if (ypbinding == NULL)
+		return YPERR_RESRC;
 
+again:
 	s = ypconnect(SOCK_DGRAM);
-	if (s == -1)
+	if (s == -1) {
+		free(ypbinding);
 		return YPERR_YPBIND;	/* YP not running */
-
-	free(ypbinding);
-	ypbinding = calloc(1, sizeof *ypbinding);
-	if (ypbinding == NULL) {
-		close(s);
-		return YPERR_RESRC;
 	}
 	ypbinding->dom_socket = s;
 	ypbinding->dom_server_addr.sin_port = -1; /* don't consult portmap */
@@ -80,14 +76,12 @@ again:
 	    YPPROG, YPVERS, tv, &ypbinding->dom_socket);
 	if (ypbinding->dom_client == NULL) {
 		close(ypbinding->dom_socket);
-		free(ypbinding);
-		ypbinding = NULL;
+		ypbinding->dom_socket = -1;
 		clnt_pcreateerror("clntudp_create");
 		goto again;
 	}
 	clnt_control(ypbinding->dom_client, CLSET_CONNECTED, &connected);
-	if (ypdb)
-		*ypdb = ypbinding;
+	*ypdb = ypbinding;
 	return 0;
 }
 
@@ -95,20 +89,30 @@ void
 _yp_unbind(struct dom_binding *ypb)
 {
 	close(ypb->dom_socket);
-	ypb->dom_socket = -1;
-	clnt_destroy(ypb->dom_client);
-	ypb->dom_client = NULL;
+	if (ypb->dom_client)
+		clnt_destroy(ypb->dom_client);
+	free(ypb);
 }
 
+/*
+ * Check if YP is running.  But do not leave it active, because we
+ * may not return from libc with a fd active.
+ */
 int
 yp_bind(const char *dom)
 {
-	return _yp_dobind(dom, NULL);
+	struct dom_binding *ysd;
+	int r;
+
+	r = _yp_dobind(dom, &ysd);
+	if (r == 0)
+		_yp_unbind(ysd);
+	return r;
 }
 DEF_WEAK(yp_bind);
 
 void
 yp_unbind(const char *dom)
 {
-	_yp_unbind(ypbinding);
+	/* do nothing */
 }
diff --git a/lib/libc/yp/yp_first.c b/lib/libc/yp/yp_first.c
index 2f24999043f..d4978ba2a57 100644
--- a/lib/libc/yp/yp_first.c
+++ b/lib/libc/yp/yp_first.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_first.c,v 1.11 2015/09/13 20:57:28 guenther Exp $ */
+/*	$OpenBSD: yp_first.c,v 1.12 2022/08/02 16:59:29 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -41,7 +41,7 @@ yp_first(const char *indomain, const char *inmap, char **outkey, int *outkeylen,
 {
 	struct ypresp_key_val yprkv;
 	struct ypreq_nokey yprnk;
-	struct dom_binding *ysd;
+	struct dom_binding *ysd = NULL;
 	struct timeval  tv;
 	int tries = 0, r;
 
@@ -69,7 +69,7 @@ again:
 	if (r != RPC_SUCCESS) {
 		if (tries++)
 			clnt_perror(ysd->dom_client, "yp_first: clnt_call");
-		ysd->dom_vers = -1;
+		_yp_unbind(ysd);
 		goto again;
 	}
 	if (!(r = ypprot_err(yprkv.stat))) {
diff --git a/lib/libc/yp/yp_get_default_domain.c b/lib/libc/yp/yp_get_default_domain.c
index 8cf0dc9fbd9..7eba82d81f4 100644
--- a/lib/libc/yp/yp_get_default_domain.c
+++ b/lib/libc/yp/yp_get_default_domain.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_get_default_domain.c,v 1.9 2015/09/13 20:57:28 guenther Exp $ */
+/*	$OpenBSD: yp_get_default_domain.c,v 1.10 2022/08/02 16:59:30 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -33,17 +33,27 @@
 #include <rpcsvc/yp.h>
 #include <rpcsvc/ypclnt.h>
 #include "ypinternal.h"
+#include "thread_private.h"
+
+_THREAD_PRIVATE_KEY(yp_get_default_domain);
 
 int
 yp_get_default_domain(char **domp)
 {
+	int r = 0;
+
+	_THREAD_PRIVATE_MUTEX_LOCK(yp_get_default_domain);
 	*domp = NULL;
-	if (_yp_domain[0] == '\0')
-		if (getdomainname(_yp_domain, sizeof _yp_domain))
-			return YPERR_NODOM;
+	if (_yp_domain[0] == '\0') {
+		if (getdomainname(_yp_domain, sizeof _yp_domain) ||
+		    _yp_domain[0] == '\0') {
+			r = YPERR_NODOM;
+			goto out;
+		}
+	}
 	*domp = _yp_domain;
-	if (_yp_domain[0] == '\0')
-		return YPERR_NODOM;
-	return 0;
+out:
+	_THREAD_PRIVATE_MUTEX_UNLOCK(yp_get_default_domain);
+	return r;
 }
 DEF_WEAK(yp_get_default_domain);
diff --git a/lib/libc/yp/yp_maplist.c b/lib/libc/yp/yp_maplist.c
index 17bdefe103f..8a7afbbd32d 100644
--- a/lib/libc/yp/yp_maplist.c
+++ b/lib/libc/yp/yp_maplist.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_maplist.c,v 1.9 2015/01/16 16:48:51 deraadt Exp $ */
+/*	$OpenBSD: yp_maplist.c,v 1.10 2022/08/02 16:59:30 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -56,7 +56,7 @@ again:
 	if (r != RPC_SUCCESS) {
 		if (tries++)
 			clnt_perror(ysd->dom_client, "yp_maplist: clnt_call");
-		ysd->dom_vers = -1;
+		_yp_unbind(ysd);
 		goto again;
 	}
 	*outmaplist = ypml.maps;
diff --git a/lib/libc/yp/yp_master.c b/lib/libc/yp/yp_master.c
index 0d8f6787ccc..2200d721f42 100644
--- a/lib/libc/yp/yp_master.c
+++ b/lib/libc/yp/yp_master.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_master.c,v 1.9 2015/01/16 16:48:51 deraadt Exp $ */
+/*	$OpenBSD: yp_master.c,v 1.10 2022/08/02 16:59:30 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -65,7 +65,7 @@ again:
 	if (r != RPC_SUCCESS) {
 		if (tries++)
 			clnt_perror(ysd->dom_client, "yp_master: clnt_call");
-		ysd->dom_vers = -1;
+		_yp_unbind(ysd);
 		goto again;
 	}
 	if (!(r = ypprot_err(yprm.stat))) {
diff --git a/lib/libc/yp/yp_order.c b/lib/libc/yp/yp_order.c
index 696137ae0f4..522d5d7d264 100644
--- a/lib/libc/yp/yp_order.c
+++ b/lib/libc/yp/yp_order.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: yp_order.c,v 1.10 2015/01/16 16:48:51 deraadt Exp $ */
+/*	$OpenBSD: yp_order.c,v 1.11 2022/08/02 16:59:30 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -72,7 +72,7 @@ again:
 	}
 	if (r != RPC_SUCCESS) {
 		clnt_perror(ysd->dom_client, "yp_order: clnt_call");
-		ysd->dom_vers = -1;
+		_yp_unbind(ysd);
 		goto again;
 	}
 	*outorder = ypro.ordernum;
diff --git a/lib/libc/yp/ypinternal.h b/lib/libc/yp/ypinternal.h
index 7be608d812c..5b7ed4103db 100644
--- a/lib/libc/yp/ypinternal.h
+++ b/lib/libc/yp/ypinternal.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ypinternal.h,v 1.13 2022/07/17 03:08:58 deraadt Exp $	 */
+/*	$OpenBSD: ypinternal.h,v 1.14 2022/08/02 16:59:30 deraadt Exp $	 */
 
 /*
  * Copyright (c) 1992, 1993, 1996 Theo de Raadt <deraadt@theos.com>
@@ -31,14 +31,9 @@
  * yp_prot.h and yp.h.
  */
 struct dom_binding {
-	struct dom_binding *dom_pnext;
-	char dom_domain[YPMAXDOMAIN + 1];
 	struct sockaddr_in dom_server_addr;
-	u_short dom_server_port;
 	int dom_socket;
 	CLIENT *dom_client;
-	u_short dom_local_port;
-	long dom_vers;
 };
 
 #define BINDINGDIR	"/var/yp/binding"
diff --git a/lib/libc/yp/ypmatch_cache.c b/lib/libc/yp/ypmatch_cache.c
index 718a1bde927..1c2af4c7c69 100644
--- a/lib/libc/yp/ypmatch_cache.c
+++ b/lib/libc/yp/ypmatch_cache.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ypmatch_cache.c,v 1.17 2015/09/13 20:57:28 guenther Exp $ */
+/*	$OpenBSD: ypmatch_cache.c,v 1.18 2022/08/02 16:59:30 deraadt Exp $ */
 /*
  * Copyright (c) 1992, 1993 Theo de Raadt <deraadt@theos.com>
  * All rights reserved.
@@ -187,7 +187,7 @@ again:
 	if (r != RPC_SUCCESS) {
 		if (tries++)
 			clnt_perror(ysd->dom_client, "yp_match: clnt_call");
-		ysd->dom_vers = -1;
+		_yp_unbind(ysd);
 		goto again;
 	}
 	if (!(r = ypprot_err(yprv.stat))) {
@@ -248,7 +248,7 @@ again:
 	if (r != RPC_SUCCESS) {
 		if (tries++)
 			clnt_perror(ysd->dom_client, "yp_next: clnt_call");
-		ysd->dom_vers = -1;
+		_yp_unbind(ysd);
 		goto again;
 	}
 	if (!(r = ypprot_err(yprkv.stat))) {