This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] nss: Do not set errno to 0 in get* functions [BZ #21898]
- From: fweimer at redhat dot com (Florian Weimer)
- To: libc-alpha at sourceware dot org
- Date: Wed, 09 Aug 2017 17:27:21 +0200
- Subject: [PATCH] nss: Do not set errno to 0 in get* functions [BZ #21898]
- Authentication-results: sourceware.org; auth=none
- 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 at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CA22D4EFB3
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 <fweimer@redhat.com>
[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,