From patchwork Wed Aug 9 15:27:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 22046 Received: (qmail 55002 invoked by alias); 9 Aug 2017 15:27:26 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 54918 invoked by uid 89); 9 Aug 2017 15:27:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CA22D4EFB3 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=fweimer@redhat.com Date: Wed, 09 Aug 2017 17:27:21 +0200 To: libc-alpha@sourceware.org Subject: [PATCH] nss: Do not set errno to 0 in get* functions [BZ #21898] User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20170809152721.C5A8A4029813A@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) Some of the get* functions are in POSIX, and POSIX functions must not set errno to 0. Callers are expected to set errno to 0 or another invalid value themselves in order to detect the “successful lookup, but no data” case. 2017-08-09 Florian Weimer [BZ #21898] * nss/getXXbyYY.c (FUNCTION_NAME): Check return code from the internal function and restore errno if the return code is zero. Set h_errno to NETDB_INTERNAL more consistently. * nss/getXXbyYY_r.c (REENTRANT_NAME): Do not set errno to zero. Restore errno on success. * pwd/getpw.c (__getpw): Set errno to zero if a successful lookup did not find anything. diff --git a/nss/getXXbyYY.c b/nss/getXXbyYY.c index 26ee726..254ec2c 100644 --- a/nss/getXXbyYY.c +++ b/nss/getXXbyYY.c @@ -95,13 +95,19 @@ FUNCTION_NAME (ADD_PARAMS) static LOOKUP_TYPE resbuf; LOOKUP_TYPE *result; + /* This function must save and restore errno for NULL return values. + *Preservation in the _r function is not sufficient because the _r + *function can be called multiple times until it stops returning + *ERANGE, clobbering errno. */ + int saved_errno = errno; + #ifdef HANDLE_DIGITS_DOTS /* Wrap both __nss_hostname_digits_dots and the actual lookup function call in the same context. */ struct resolv_context *res_ctx = __resolv_context_get (); if (res_ctx == NULL) { -# if NEED_H_ERRNO +# ifdef NEED_H_ERRNO __set_h_errno (NETDB_INTERNAL); # endif return NULL; @@ -115,6 +121,12 @@ FUNCTION_NAME (ADD_PARAMS) { buffer_size = BUFLEN; buffer = (char *) malloc (buffer_size); + if (buffer == NULL) + { +#ifdef NEED_H_ERRNO + __set_h_errno (NETDB_INTERNAL); +#endif + } } #ifdef HANDLE_DIGITS_DOTS @@ -127,15 +139,19 @@ FUNCTION_NAME (ADD_PARAMS) } #endif - while (buffer != NULL - && (INTERNAL (REENTRANT_NAME) (ADD_VARIABLES, &resbuf, buffer, - buffer_size, &result H_ERRNO_VAR) - == ERANGE) + int rc = ENOMEM; + while (buffer != NULL) + { + rc = INTERNAL (REENTRANT_NAME) (ADD_VARIABLES, &resbuf, buffer, + buffer_size, &result H_ERRNO_VAR); + if (rc != ERANGE #ifdef NEED_H_ERRNO - && h_errno == NETDB_INTERNAL + || h_errno != NETDB_INTERNAL #endif ) - { + break; + + /* Enlarge the buffer. */ char *new_buf; buffer_size *= 2; new_buf = (char *) realloc (buffer, buffer_size); @@ -144,6 +160,9 @@ FUNCTION_NAME (ADD_PARAMS) /* We are out of memory. Free the current buffer so that the process gets a chance for a normal termination. */ free (buffer); +#ifdef NEED_H_ERRNO + __set_h_errno (NETDB_INTERNAL); +#endif __set_errno (ENOMEM); } buffer = new_buf; @@ -152,6 +171,12 @@ FUNCTION_NAME (ADD_PARAMS) if (buffer == NULL) result = NULL; + /* If the call was successful, result could still be NULL if there + is no record. The caller can detect this by setting errno to + zero before calling this function. */ + if (rc == 0) + __set_errno (saved_errno); + #ifdef HANDLE_DIGITS_DOTS done: #endif diff --git a/nss/getXXbyYY_r.c b/nss/getXXbyYY_r.c index 5e3bead..0e938da 100644 --- a/nss/getXXbyYY_r.c +++ b/nss/getXXbyYY_r.c @@ -211,6 +211,7 @@ INTERNAL (REENTRANT_NAME) (ADD_PARAMS, LOOKUP_TYPE *resbuf, char *buffer, #ifdef NEED_H_ERRNO bool any_service = false; #endif + int saved_errno = errno; #ifdef NEED__RES /* The HANDLE_DIGITS_DOTS case below already needs the resolver @@ -426,7 +427,13 @@ done: else return errno; - __set_errno (res); + if (res != 0) + /* Some callers expect that a non-zero return value leaves the + actual error code in the errno variable. */ + __set_errno (res); + else + /* Preserve errno on success. */ + __set_errno (saved_errno); return res; } diff --git a/pwd/getpw.c b/pwd/getpw.c index 0a73f28..cf14db9 100644 --- a/pwd/getpw.c +++ b/pwd/getpw.c @@ -48,7 +48,12 @@ __getpw (__uid_t uid, char *buf) return -1; if (p == NULL) - return -1; + { + /* No lookup error, but no data either. This historic interface + is expected to set errno to zero. */ + errno = 0; + return -1; + } if (sprintf (buf, "%s:%s:%lu:%lu:%s:%s:%s", p->pw_name, p->pw_passwd, (unsigned long int) p->pw_uid, (unsigned long int) p->pw_gid,