nss: Do not set errno to 0 in get* functions [BZ #21898]

Message ID 20170809152721.C5A8A4029813A@oldenburg.str.redhat.com
State New, archived
Headers

Commit Message

Florian Weimer Aug. 9, 2017, 3:27 p.m. UTC
  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.
  

Patch

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,