Well, given the total lack of feeback on my two diffs,
and given that time is running out fast, let's jump the gun:
Theo de Raadt wrote on Thu, Feb 20, 2014 at 05:43:18PM -0700:
> kettenis@ wrote:
>> Indeed. POSIX explicitly says:
>>
>> "No function in this volume of POSIX.1-2008 shall set errno to zero."
>>
>> The standard is slightly ambiguous on what getpwnam_r() should do, but
>> the way I read it, it should not touch errno at all. So it should
>> save errno at the start of the function, and restore it just before
>> return.
> That is the only sane approach.
>
> I would like to see a diff in which at least solves the non-YP cases.
> The YP cases have other issues in other functions as well, that are
> somewhat known..
Here is a third diff, to be applied on top of my previous two,
doing what kettenis@ said: let getpwnam_r() not touch errno at all.
Obviously, that is not a conservative approach and may have
unexpected consequences in some software, even if it's correct.
So, i'm now accepting OKs for either of the following:
* my first diff,
fixing just the return values of get*_r()
without changing errno handling;
i consider that quite safe.
* my second diff,
just avoiding clobbering of errno by close() and syslog()
and setting errno = ERANGE when needed for the non-YP case;
i consider that quite safe,
it can be committed with or without the first one.
* the cumulative diff including all three,
given at the very bottom.
That is NOT CONSERVATIVE, it changes two things in addition
to the above:
- getpw{nam,uid}_r() no longer sets errno even on serious errors.
- If a match is found after any kind of YP error,
getpw{nam,uid}() no longer set errno.
Both make sense to me, but are significant changes.
----- 8< ----- schnipp ----- >8 ----- 8< ----- schnapp ----- >8 -----
### THIS IS MY THIRD DIFF:
--- 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);
}
----- 8< ----- schnipp ----- >8 ----- 8< ----- schnapp ----- >8 -----
### THIS IS THE CUMULATIVE DIFF 1+2+3:
Index: getpwent.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/getpwent.c,v
retrieving revision 1.48
diff -u -p -r1.48 getpwent.c
--- getpwent.c 15 Nov 2013 22:32:55 -0000 1.48
+++ getpwent.c 22 Feb 2014 09:51:15 -0000
@@ -708,8 +708,12 @@ getpwnam_r(const char *name, struct pass
{
struct passwd *pwret = NULL;
int flags = 0, *flagsp;
+ int my_errno = 0;
+ int saved_errno, tmp_errno;
_THREAD_PRIVATE_MUTEX_LOCK(pw);
+ saved_errno = errno;
+ errno = 0;
if (!_pw_db && !__initdb())
goto fail;
@@ -727,23 +731,33 @@ getpwnam_r(const char *name, struct pass
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)
*pwretp = pwret;
+ if (pwret == NULL)
+ my_errno = errno;
+ errno = saved_errno;
_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
- return (pwret ? 0 : 1);
+ return (my_errno);
}
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);
}
@@ -753,8 +767,12 @@ getpwuid_r(uid_t uid, struct passwd *pw,
{
struct passwd *pwret = NULL;
int flags = 0, *flagsp;
+ int my_errno = 0;
+ int saved_errno, tmp_errno;
_THREAD_PRIVATE_MUTEX_LOCK(pw);
+ saved_errno = errno;
+ errno = 0;
if (!_pw_db && !__initdb())
goto fail;
@@ -772,23 +790,33 @@ getpwuid_r(uid_t uid, struct passwd *pw,
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)
*pwretp = pwret;
+ if (pwret == NULL)
+ my_errno = errno;
+ errno = saved_errno;
_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
- return (pwret ? 0 : 1);
+ return (my_errno);
}
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);
}
@@ -851,9 +879,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);
}
@@ -867,8 +898,10 @@ __hashpw(DBT *key, char *buf, size_t buf
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++));
Index: getgrent.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/getgrent.c,v
retrieving revision 1.38
diff -u -p -r1.38 getgrent.c
--- getgrent.c 17 Apr 2013 17:40:35 -0000 1.38
+++ getgrent.c 22 Feb 2014 09:51:15 -0000
@@ -134,6 +134,7 @@ getgrnam_r(const char *name, struct grou
if (bufsize < GETGR_R_SIZE_MAX)
return ERANGE;
errnosave = errno;
+ errno = 0;
*result = getgrnam_gs(name, grp, (struct group_storage *)buffer);
if (*result == NULL)
ret = errno;
@@ -180,6 +181,7 @@ getgrgid_r(gid_t gid, struct group *grp,
if (bufsize < GETGR_R_SIZE_MAX)
return ERANGE;
errnosave = errno;
+ errno = 0;
*result = getgrgid_gs(gid, grp, (struct group_storage *)buffer);
if (*result == NULL)
ret = errno;
