Re: Bad return value for getpwnam_r et al

[Ingo Schwarze]

Hi,

having just committed the first of my patches for getpwent.c
and getgrent.c which fixes the return values without touching
errno, i'd like to move on regarding some of the remaining
issues, so i'm sending another two independent patches:

 1. Prevent close() and syslog() from stomping on errno
      (identical to parts of a patch by guenther@)
    and set ERANGE when appropriate
      (idential to a patch by jca@)
    This patch is in this mail, see below.

 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.

The patch shown below fixes all bugs i'm aware of for the non-YP
case.  As suggested by millert@, i suggest committing it right now,
doing anything YP-related in a later step.

This one seems relatively safe to me, it just fixes bugs.
Of course, doing so might uncover bugs in ports, but that doesn't
seem very likely, and if it happens, it should be dealt with on
a case-by-case basis.  Proactively building the whole ports tree
and testing the functionality of all affected ports is not feasible,
too many ports use these functions.

So, OK to commit?
  Ingo


--- getpwent.c.retval   Wed Feb 19 19:25:33 2014
+++ getpwent.c  Wed Feb 19 20:13:34 2014
@@ -709,7 +709,7 @@ getpwnam_r(const char *name, struct passwd *pw, char *
        struct passwd *pwret = NULL;
        int flags = 0, *flagsp;
        int my_errno = 0;
-       int saved_errno;
+       int saved_errno, tmp_errno;
 
        _THREAD_PRIVATE_MUTEX_LOCK(pw);
        saved_errno = errno;
@@ -731,8 +731,10 @@ getpwnam_r(const char *name, struct passwd *pw, char *
                pwret = _pwhashbyname(name, buf, buflen, pw, flagsp);
 
        if (!_pw_stayopen) {
+               tmp_errno = errno;
                (void)(_pw_db->close)(_pw_db);
                _pw_db = NULL;
+               errno = tmp_errno;
        }
 fail:
        if (pwretp)
@@ -762,7 +764,7 @@ getpwuid_r(uid_t uid, struct passwd *pw, char *buf, si
        struct passwd *pwret = NULL;
        int flags = 0, *flagsp;
        int my_errno = 0;
-       int saved_errno;
+       int saved_errno, tmp_errno;
 
        _THREAD_PRIVATE_MUTEX_LOCK(pw);
        saved_errno = errno;
@@ -784,8 +786,10 @@ getpwuid_r(uid_t uid, struct passwd *pw, char *buf, si
                pwret = _pwhashbyuid(uid, buf, buflen, pw, flagsp);
 
        if (!_pw_stayopen) {
+               tmp_errno = errno;
                (void)(_pw_db->close)(_pw_db);
                _pw_db = NULL;
+               errno = tmp_errno;
        }
 fail:
        if (pwretp)
@@ -867,9 +871,12 @@ __initdb(void)
                errno = saved_errno;
                return (1);
        }
-       if (!warned)
+       if (!warned) {
+               saved_errno = errno;
                syslog(LOG_ERR, "%s: %m", _PATH_MP_DB);
-       warned = 1;
+               errno = saved_errno;
+               warned = 1;
+       }
        return (0);
 }
 
@@ -883,8 +890,10 @@ __hashpw(DBT *key, char *buf, size_t buflen, struct pa
        if ((_pw_db->get)(_pw_db, key, &data, 0))
                return (0);
        p = (char *)data.data;
-       if (data.size > buflen)
+       if (data.size > buflen) {
+               errno = ERANGE;
                return (0);
+       }
 
        t = buf;
 #define        EXPAND(e)       e = t; while ((*t++ = *p++));