[PATCH] Fix error handling in getlogin_r

[PATCH] Fix error handling in getlogin_r

[Andreas Schwab]

2010-06-02  Andreas Schwab  <schwab@redhat.com>

	* sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
	Fix error handling.  Properly resize buffer.  Don't set errno.
	(getlogin_r): Only use fallback if __getlogin_r_loginuid returns -1.
---
 sysdeps/unix/sysv/linux/getlogin_r.c |   63 ++++++++++++++++++++--------------
 1 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c b/sysdeps/unix/sysv/linux/getlogin_r.c
index dad2671..404c84d 100644
--- a/sysdeps/unix/sysv/linux/getlogin_r.c
+++ b/sysdeps/unix/sysv/linux/getlogin_r.c
@@ -27,6 +27,10 @@ static int getlogin_r_fd0 (char *name, size_t namesize);
 #undef getlogin_r
 
 
+/* Try to determine login name from /proc/self/loginuid and return 0
+   if successful.  If /proc/self/loginuid cannot be read return -1.
+   Otherwise return the error number.  */
+
 int
 attribute_hidden
 __getlogin_r_loginuid (name, namesize)
@@ -35,7 +39,7 @@ __getlogin_r_loginuid (name, namesize)
 {
   int fd = open_not_cancel_2 ("/proc/self/loginuid", O_RDONLY);
   if (fd == -1)
-    return 1;
+    return -1;
 
   /* We are reading a 32-bit number.  12 bytes are enough for the text
      representation.  If not, something is wrong.  */
@@ -51,46 +55,50 @@ __getlogin_r_loginuid (name, namesize)
       || (uidbuf[n] = '\0',
 	  uid = strtoul (uidbuf, &endp, 10),
 	  endp == uidbuf || *endp != '\0'))
-    return 1;
+    return -1;
 
   size_t buflen = 1024;
   char *buf = alloca (buflen);
   bool use_malloc = false;
   struct passwd pwd;
   struct passwd *tpwd;
-  int res;
-
-  while ((res = __getpwuid_r (uid, &pwd, buf, buflen, &tpwd)) != 0)
-    if (__libc_use_alloca (2 * buflen))
-      extend_alloca (buf, buflen, 2 * buflen);
-    else
-      {
-	buflen *= 2;
-	char *newp = realloc (use_malloc ? buf : NULL, buflen);
-	if (newp == NULL)
-	  {
-	  fail:
-	    if (use_malloc)
-	      free (buf);
-	    return 1;
-	  }
-	buf = newp;
-	use_malloc = true;
-      }
+  int result;
+
+  while ((result = __getpwuid_r (uid, &pwd, buf, buflen, &tpwd)) != 0)
+    {
+      if (result != ERANGE)
+	goto out;
+      if (__libc_use_alloca (2 * buflen))
+	buf = extend_alloca (buf, buflen, 2 * buflen);
+      else
+	{
+	  buflen *= 2;
+	  char *newp = realloc (use_malloc ? buf : NULL, buflen);
+	  if (newp == NULL)
+	    {
+	      result = ENOMEM;
+	      goto out;
+	    }
+	  buf = newp;
+	  use_malloc = true;
+	}
+    }
 
   if (tpwd == NULL)
-    goto fail;
+    {
+      result = ENOENT;
+      goto out;
+    }
 
-  int result = 0;
   size_t needed = strlen (pwd.pw_name) + 1;
   if (needed > namesize)
     {
-      __set_errno (ERANGE);
       result = ERANGE;
       goto out;
     }
 
   memcpy (name, pwd.pw_name, needed);
+  result = 0;
 
  out:
   if (use_malloc)
@@ -109,8 +117,11 @@ getlogin_r (name, namesize)
      char *name;
      size_t namesize;
 {
-  if (__getlogin_r_loginuid (name, namesize) == 0)
-    return 0;
+  int result;
+
+  result = __getlogin_r_loginuid (name, namesize);
+  if (result >= 0)
+    return result;
 
   return getlogin_r_fd0 (name, namesize);
 }
-- 
1.7.1


-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [PATCH] Fix error handling in getlogin_r

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/02/2010 04:42 AM, Andreas Schwab wrote:
> 2010-06-02  Andreas Schwab  <schwab@redhat.com>
> 
> 	* sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
> 	Fix error handling.  Properly resize buffer.  Don't set errno.
> 	(getlogin_r): Only use fallback if __getlogin_r_loginuid returns -1.

This patch is changing the way the code works.

The current tries to use the old method whenever the new one fails.
Particularly if the name lookup fails.  Imagine the user account is
removed.  After your change a call always returns an error while the
utmp database can still contain a valid record.

What are you trying to fix?  Perhaps ENOMEM errors could be handled
differently but the patch as-is isn't what should be used.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkwWzuAACgkQ2ijCOnn/RHSgQQCeKc8C+kFACp/J2TFG/GepytmF
CIoAoK3Rbi414e4Fid/NbqJ8d1OyiJah
=JyOF
-----END PGP SIGNATURE-----

Re: [PATCH] Fix error handling in getlogin_r

[Andreas Schwab]

Ulrich Drepper <drepper@redhat.com> writes:

> What are you trying to fix?

Error checking is completely bogus.  There is no handling of ERANGE at
all.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [PATCH] Fix error handling in getlogin_r

[Andreas Schwab]

2010-06-21  Andreas Schwab <schwab@redhat.com>

	* sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
	Restore proper fallback handling.
---
 sysdeps/unix/sysv/linux/getlogin_r.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c b/sysdeps/unix/sysv/linux/getlogin_r.c
index 5c3de69..7d4d6c0 100644
--- a/sysdeps/unix/sysv/linux/getlogin_r.c
+++ b/sysdeps/unix/sysv/linux/getlogin_r.c
@@ -27,6 +27,10 @@ static int getlogin_r_fd0 (char *name, size_t namesize);
 #undef getlogin_r
 
 
+/* Try to determine login name from /proc/self/loginuid and return 0
+   if successful.  If /proc/self/loginuid cannot be read return -1.
+   Otherwise return the error number.  */
+
 int
 attribute_hidden
 __getlogin_r_loginuid (name, namesize)
@@ -35,7 +39,7 @@ __getlogin_r_loginuid (name, namesize)
 {
   int fd = open_not_cancel_2 ("/proc/self/loginuid", O_RDONLY);
   if (fd == -1)
-    return 1;
+    return -1;
 
   /* We are reading a 32-bit number.  12 bytes are enough for the text
      representation.  If not, something is wrong.  */
@@ -51,7 +55,7 @@ __getlogin_r_loginuid (name, namesize)
       || (uidbuf[n] = '\0',
 	  uid = strtoul (uidbuf, &endp, 10),
 	  endp == uidbuf || *endp != '\0'))
-    return 1;
+    return -1;
 
   size_t buflen = 1024;
   char *buf = alloca (buflen);
-- 
1.7.1


-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."