9On Wed, 5 Feb 2014, Ted Unangst wrote:
> On Wed, Feb 05, 2014 at 12:41, Brad Smith wrote:
> >> They do (but through errno). Of course there may be other places where
> >> the internal functions clobber errno before returning to the user code,
> >> but some fixes have been applied already (eg. rev 1.43 of getpwent.c).
> >
> > Is anyone actually looking into fixing this? This issue was also noticed
> > when Dovecot was updated to 2.2 and it broke local account authentication.
>
> Test cases? The fix here will fix this test case, but it's probably
> one of the more contrived error cases.
There's more needed. There are actually three cases:
1) found a match:
getpwnam(): return non-NULL (Linux and Solaris set errno to 0)
getpwnam_r(): set *pwretp to non-NULL and return 0
2) no match, but no error:
getpwnam(): set errno to zero and return NULL
getpwnam_r(): set *pwretp to NULL and return 0
3) something failed:
getpwnam(): set errno to non-zero-errno-value and return NULL
getpwnam_r(): set *pwretp to NULL and return non-zero-errno-value
On Solaris, getpwnam_r() doesn't change errno; on Linux, getpwnam_r() sets
errno to its return value.
So: we need to
- clear errno in the success and "no match" cases,
- need to set errno in the ERANGE case,
- need to return errno from getpw{uid,nam}_r() on failure, and
- need to preserve errno around calls whose failure doesn't affect the
success or failure of the lookup (e.g., the DB close)
Diff below does that, though I haven't checked the YP lookup closely to
verify whether it might return the errno of the wrong system call on
failure. The one thing this diff does *not* do is set errno to zero when
a match is found. Both Linux and Solaris do that, so maybe we should
too...
Philip
Index: gen/getpwent.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/getpwent.c,v
retrieving revision 1.48
diff -u -p -r1.48 getpwent.c
--- gen/getpwent.c 15 Nov 2013 22:32:55 -0000 1.48
+++ gen/getpwent.c 6 Feb 2014 00:15:46 -0000
@@ -672,8 +672,11 @@ _pwhashbyname(const char *name, char *bu
int r;
len = strlen(name);
- if (len > _PW_NAME_LEN)
+ if (len > _PW_NAME_LEN) {
+ /* can't be present, so just treat as "no match" */
+ errno = 0;
return (NULL);
+ }
bf[0] = _PW_KEYBYNAME;
bcopy(name, &bf[1], MIN(len, _PW_NAME_LEN));
key.data = (u_char *)bf;
@@ -707,7 +710,7 @@ getpwnam_r(const char *name, struct pass
struct passwd **pwretp)
{
struct passwd *pwret = NULL;
- int flags = 0, *flagsp;
+ int flags = 0, *flagsp, saved_errno;
_THREAD_PRIVATE_MUTEX_LOCK(pw);
if (!_pw_db && !__initdb())
@@ -727,14 +730,16 @@ getpwnam_r(const char *name, struct pass
pwret = _pwhashbyname(name, buf, buflen, pw, flagsp);
if (!_pw_stayopen) {
+ saved_errno = errno;
(void)(_pw_db->close)(_pw_db);
_pw_db = NULL;
+ errno = saved_errno;
}
fail:
if (pwretp)
*pwretp = pwret;
_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
- return (pwret ? 0 : 1);
+ return (pwret ? 0 : errno);
}
struct passwd *
@@ -742,8 +747,11 @@ getpwnam(const char *name)
{
struct passwd *pw = NULL;
+ /* getpwnam_r sets errno on failure */
if (getpwnam_r(name, &_pw_passwd, _pw_string, sizeof _pw_string, &pw))
pw = NULL;
+ else
+ errno = 0;
return (pw);
}
@@ -752,7 +760,7 @@ getpwuid_r(uid_t uid, struct passwd *pw,
struct passwd **pwretp)
{
struct passwd *pwret = NULL;
- int flags = 0, *flagsp;
+ int flags = 0, *flagsp, saved_errno;
_THREAD_PRIVATE_MUTEX_LOCK(pw);
if (!_pw_db && !__initdb())
@@ -772,14 +780,16 @@ getpwuid_r(uid_t uid, struct passwd *pw,
pwret = _pwhashbyuid(uid, buf, buflen, pw, flagsp);
if (!_pw_stayopen) {
+ saved_errno = errno;
(void)(_pw_db->close)(_pw_db);
_pw_db = NULL;
+ errno = saved_errno;
}
fail:
if (pwretp)
*pwretp = pwret;
_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
- return (pwret ? 0 : 1);
+ return (pwret ? 0 : errno);
}
struct passwd *
@@ -787,8 +797,11 @@ getpwuid(uid_t uid)
{
struct passwd *pw = NULL;
+ /* getpwuid_r sets errno on failure */
if (getpwuid_r(uid, &_pw_passwd, _pw_string, sizeof _pw_string, &pw))
pw = NULL;
+ else
+ errno = 0;
return (pw);
}
@@ -851,8 +864,11 @@ __initdb(void)
errno = saved_errno;
return (1);
}
- if (!warned)
+ if (!warned) {
+ saved_errno = errno;
syslog(LOG_ERR, "%s: %m", _PATH_MP_DB);
+ errno = saved_errno;
+ }
warned = 1;
return (0);
}
@@ -863,12 +879,20 @@ __hashpw(DBT *key, char *buf, size_t buf
{
char *p, *t;
DBT data;
+ int ret;
- if ((_pw_db->get)(_pw_db, key, &data, 0))
+ if ((ret = (_pw_db->get)(_pw_db, key, &data, 0))) {
+ if (ret == 1) {
+ /* not found, but not an error */
+ errno = 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++));
