1) The yp_bind/yp_unbind and internal _yp_dobind/_yp_unbind sequences shared
authorderaadt <deraadt@openbsd.org>
Tue, 2 Aug 2022 16:59:29 +0000 (16:59 +0000)
committerderaadt <deraadt@openbsd.org>
Tue, 2 Aug 2022 16:59:29 +0000 (16:59 +0000)
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

lib/libc/yp/yp_all.c
lib/libc/yp/yp_bind.c
lib/libc/yp/yp_first.c
lib/libc/yp/yp_get_default_domain.c
lib/libc/yp/yp_maplist.c
lib/libc/yp/yp_master.c
lib/libc/yp/yp_order.c
lib/libc/yp/ypinternal.h
lib/libc/yp/ypmatch_cache.c

index 04a6ebe..204f085 100644 (file)
@@ -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);
index 15db9ea..af6775e 100644 (file)
@@ -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.
 #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 */
 }
index 2f24999..d4978ba 100644 (file)
@@ -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))) {
index 8cf0dc9..7eba82d 100644 (file)
@@ -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.
 #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);
index 17bdefe..8a7afbb 100644 (file)
@@ -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;
index 0d8f678..2200d72 100644 (file)
@@ -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))) {
index 696137a..522d5d7 100644 (file)
@@ -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;
index 7be608d..5b7ed41 100644 (file)
@@ -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>
  * 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"
index 718a1bd..1c2af4c 100644 (file)
@@ -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))) {