From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16793 invoked by alias); 25 May 2004 12:49:46 -0000 Mailing-List: contact libc-hacker-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sources.redhat.com Received: (qmail 16624 invoked from network); 25 May 2004 12:49:44 -0000 Received: from unknown (HELO sunsite.ms.mff.cuni.cz) (195.113.15.26) by sourceware.org with SMTP; 25 May 2004 12:49:44 -0000 Received: from sunsite.ms.mff.cuni.cz (sunsite.mff.cuni.cz [127.0.0.1]) by sunsite.ms.mff.cuni.cz (8.12.8/8.12.8) with ESMTP id i4PAaQ3j019421; Tue, 25 May 2004 12:36:26 +0200 Received: (from jakub@localhost) by sunsite.ms.mff.cuni.cz (8.12.8/8.12.8/Submit) id i4PAaPBw019401; Tue, 25 May 2004 12:36:25 +0200 Date: Tue, 25 May 2004 20:31:00 -0000 From: Jakub Jelinek To: Ulrich Drepper , Thorsten Kukuk Cc: Glibc hackers Subject: [PATCH] Fix ypclnt.c Message-ID: <20040525103624.GR5191@sunsite.ms.mff.cuni.cz> Reply-To: Jakub Jelinek Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4i X-SW-Source: 2004-05/txt/msg00039.txt.bz2 Hi! Calling getservbyname_r in different threads at the same time results in crashes. The culprit seems to be unguarded static ypall_foreach and ypall_data variables. I wonder why they are needed, is there something in sunrpc that would preclude this from working (I don't see any problems in clnttcp_call and generally, unless the data pointer passed to the callback is ever different from the data pointer passed to clnt_call, there cannot be problems (the first field in the structure is still u_long, so even if some function assumes it is an u_long, it should work))? The following patch certainly fixes the problem for me and in current (limited) testing did not show up any issues. 2004-05-25 Jakub Jelinek * nis/ypclnt.c (ypall_data, ypall_foreach): Remove. (struct ypresp_all_data): New type. (__xdr_ypresp_all): Change second argument to struct ypresp_all_data *. Replace ypall_foreach and ypall_data with objp->foreach and objp->data. (yp_all): Remove status variable, add data. Replace all uses of status with data.status. Initialize data.foreach and data.data instead of ypall_foreach and ypall_data. --- libc/nis/ypclnt.c.jj 2004-02-12 11:52:31.000000000 +0100 +++ libc/nis/ypclnt.c 2004-05-25 14:34:53.657850959 +0200 @@ -618,12 +618,16 @@ yp_order (const char *indomain, const ch return YPERR_SUCCESS; } -static void *ypall_data; -static int (*ypall_foreach) (int status, char *key, int keylen, - char *val, int vallen, char *data); +struct ypresp_all_data +{ + unsigned long status; + void *data; + int (*foreach) (int status, char *key, int keylen, + char *val, int vallen, char *data); +}; static bool_t -__xdr_ypresp_all (XDR *xdrs, u_long *objp) +__xdr_ypresp_all (XDR *xdrs, struct ypresp_all_data *objp) { while (1) { @@ -633,13 +637,13 @@ __xdr_ypresp_all (XDR *xdrs, u_long *obj if (!xdr_ypresp_all (xdrs, &resp)) { xdr_free ((xdrproc_t) xdr_ypresp_all, (char *) &resp); - *objp = YP_YPERR; + objp->status = YP_YPERR; return FALSE; } if (resp.more == 0) { xdr_free ((xdrproc_t) xdr_ypresp_all, (char *) &resp); - *objp = YP_NOMORE; + objp->status = YP_NOMORE; return TRUE; } @@ -656,24 +660,24 @@ __xdr_ypresp_all (XDR *xdrs, u_long *obj But we are allowed to add data behind the buffer, if we don't modify the length. So add an extra NUL character to avoid trouble with broken code. */ - *objp = YP_TRUE; + objp->status = YP_TRUE; memcpy (key, resp.ypresp_all_u.val.key.keydat_val, keylen); key[keylen] = '\0'; memcpy (val, resp.ypresp_all_u.val.val.valdat_val, vallen); val[vallen] = '\0'; xdr_free ((xdrproc_t) xdr_ypresp_all, (char *) &resp); - if ((*ypall_foreach) (*objp, key, keylen, - val, vallen, ypall_data)) + if ((*objp->foreach) (objp->status, key, keylen, + val, vallen, objp->data)) return TRUE; } break; default: - *objp = resp.ypresp_all_u.val.stat; + objp->status = resp.ypresp_all_u.val.stat; xdr_free ((xdrproc_t) xdr_ypresp_all, (char *) &resp); /* Sun says we don't need to make this call, but must return immediatly. Since Solaris makes this call, we will call the callback function, too. */ - (*ypall_foreach) (*objp, NULL, 0, NULL, 0, ypall_data); + (*objp->foreach) (objp->status, NULL, 0, NULL, 0, objp->data); return TRUE; } } @@ -689,7 +693,7 @@ yp_all (const char *indomain, const char enum clnt_stat result; struct sockaddr_in clnt_sin; CLIENT *clnt; - unsigned long status; + struct ypresp_all_data data; int clnt_sock; int saved_errno = errno; @@ -725,12 +729,12 @@ yp_all (const char *indomain, const char req.domain = (char *) indomain; req.map = (char *) inmap; - ypall_foreach = incallback->foreach; - ypall_data = (void *) incallback->data; + data.foreach = incallback->foreach; + data.data = (void *) incallback->data; result = clnt_call (clnt, YPPROC_ALL, (xdrproc_t) xdr_ypreq_nokey, (caddr_t) &req, (xdrproc_t) __xdr_ypresp_all, - (caddr_t) &status, RPCTIMEOUT); + (caddr_t) &data, RPCTIMEOUT); if (result != RPC_SUCCESS) { @@ -744,10 +748,10 @@ yp_all (const char *indomain, const char clnt_destroy (clnt); - if (res == YPERR_SUCCESS && status != YP_NOMORE) + if (res == YPERR_SUCCESS && data.status != YP_NOMORE) { __set_errno (saved_errno); - return ypprot_err (status); + return ypprot_err (data.status); } ++try; } Jakub