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: 799831 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-82949-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="aQQIHs2a"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xSFWr0hLJz9s65 for ; Thu, 10 Aug 2017 01:27:51 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; q=dns; s=default; b= PNDgksfWhjKKNYeSFGLzo++y7xLH/TS8EhbPr8z5A4hWfbD69IzB0lgC48yd4OQH +nJgZe51RhyMZGoRMNZTnPppXxhqtUael93pambodh8hf8FESrecEq1GN8SJmLBN cuB9u2gIj6/BLWpIDiRDLvRHKn5F2M+Atnzd2Ur28zY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; s=default; bh=w5yCe0 cQ2sDmTW2Bjy5ttPjjccY=; b=aQQIHs2avwEAhSWik4tQfj89W+TD6XuWQjJmc+ kg8Tw/u1vCQHRxhpvH0kFwocBYsYQQzjYIefliCmLpyesDTJKQp67dhtRqxoaqwC kTkq+x0vfQX5tY29r1XO8WX22cOiJhyKQJHkG4r1wwjXI7b9jp4k+A+G8PJCBQ4t ojuDA= 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,