Re: Bad return value for getpwnam_r et al

[Ingo Schwarze]

Hi,

Ingo Schwarze wrote on Thu, Mar 06, 2014 at 01:25:12AM +0100:

>  2. Prevent getpw{nam,uid}_r() from touching errno at all.
>       (as suggested by kettenis@ and agreed in principle by deraadt@)
>     That patch will follow in my next mail.

Here it is.

Note that this is not necessarily a bugfix.  While POSIX explicitly
requires getpw{nam,uid}() to set errno, it does not explicitly
forbid getpw{nam,uid}_r() to do the same, but i think we all agree
that having getpw{nam,uid}_r() leave errno alone is more sane.

Now, this *is* a change in behaviour.  Right now, getpw{nam,uid}_r()
does set errno on error, and with this patch, it will stop to do
so.  Programs that call getpw{nam,uid}_r(), sloppily ignore the
return value, and then hope to find a clue in errno will break -
looking at a few examples from the list of ports using these functions
that sthen@ sent here before lock, i didn't find any, but the list
is too long to check them all, proactively.

So i propose to just do the sane thing and watch out for fallout.
Now is a good time to do that, too.

OK?
  Ingo


--- getpwent.c.errno    Wed Feb 19 20:13:34 2014
+++ getpwent.c  Sat Feb 22 10:45:39 2014
@@ -741,8 +741,7 @@ fail:
                *pwretp = pwret;
        if (pwret == NULL)
                my_errno = errno;
-       if (!errno)
-               errno = saved_errno;
+       errno = saved_errno;
        _THREAD_PRIVATE_MUTEX_UNLOCK(pw);
        return (my_errno);
 }
@@ -751,9 +750,14 @@ struct passwd *
 getpwnam(const char *name)
 {
        struct passwd *pw = NULL;
+       int my_errno;
 
-       if (getpwnam_r(name, &_pw_passwd, _pw_string, sizeof _pw_string, &pw))
+       my_errno = getpwnam_r(name, &_pw_passwd, _pw_string,
+           sizeof _pw_string, &pw);
+       if (my_errno) {
                pw = NULL;
+               errno = my_errno;
+       }
        return (pw);
 }
 
@@ -796,8 +800,7 @@ fail:
                *pwretp = pwret;
        if (pwret == NULL)
                my_errno = errno;
-       if (!errno)
-               errno = saved_errno;
+       errno = saved_errno;
        _THREAD_PRIVATE_MUTEX_UNLOCK(pw);
        return (my_errno);
 }
@@ -806,9 +809,14 @@ struct passwd *
 getpwuid(uid_t uid)
 {
        struct passwd *pw = NULL;
+       int my_errno;
 
-       if (getpwuid_r(uid, &_pw_passwd, _pw_string, sizeof _pw_string, &pw))
+       my_errno = getpwuid_r(uid, &_pw_passwd, _pw_string,
+           sizeof _pw_string, &pw);
+       if (my_errno) {
                pw = NULL;
+               errno = my_errno;
+       }
        return (pw);
 }