Hi,
i investigated some more.
It is even worse than we thought so far.
Philip Guenther wrote on Wed, Feb 05, 2014 at 04:31:16PM -0800:
> 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
There are even more cases.
1) no error occurred, found a match (~ guenther@'s 1):
getpwnam_r(): set *pwretp to non-NULL and return 0
getpwnam(): return non-NULL, leave errno untouched
2) an error occurred, but found a match anyway (NEW):
This can happen if YP access fails with some error
but fallback to the local database returns an entry.
Strictly speaking, POSIX requires failure in this case, i.e.
getpwnam_r(): set *pwretp to NULL and return non-zero-errno-value
getpwnam(): set errno to non-zero-errno-value and return NULL
However, we do NOT want to follow POSIX in this case because
the login_passwd(8) program uses getpwnam(3) and denies login
in case of failure, but we WANT fallback to work in this case.
In order for this to work reliably, i guess the error must NOT
be reported at all, lest the calling program despair on the error
and ignore the result - login_passwd(8) won't, but given that
POSIX does not allow error and result at the same time, other
programs might get really confused -, so we want
getpwnam_r(): set *pwretp to non-NULL and return 0
getpwnam(): return non-NULL, leave errno untouched
3) no error occurred, found no match (~ guenther@'s 2):
getpwnam_r(): set *pwretp to NULL and return 0
getpwnam(): return NULL, leave errno untouched
4) an error occurred, and found no match (= guenther@'s 3):
getpwnam_r(): set *pwretp to NULL and return non-zero-errno-value
getpwnam(): set errno to non-zero-errno-value and return NULL
So, currently, we have the following issues, ordered by decreasing
severity:
A. When there is no error but getpwnam_r() finds no match,
it returns 1 instead of 0.
This affects case 3 only.
This ought to be fixed for release, see below.
B. When an error occurs and getpwnam_r() finds no match,
it returns 1 instead of the errno value.
This affects case 4 only.
This ought to be fixed for release, see below.
C. No matter whether they succeed or fail,
both functions may clobber errno with irrelevant errors.
This affects all cases.
For the file only case, we can fix this for release if we want to.
For the YP case, there is no way to fix this before release.
The YP code calls lots and lots of library functions, and some
of these clobber errno. Fixing all of that looks like a
hackathon-sized task to me, at least if i were to work on it.
D. In case of failure, getpwnam_r() sets errno,
which maybe it shouldn't, as suggested by kettenis@.
This affects cases 1 and 2.
Touching this would be too intrusive before release.
So here is my first patch for release, just fixing the return value
of getpwnam_r(), getpwuid_r(), getgrnam_r(), and getgrgid_r(),
i.e. issues A and B above. It does not yet attempt to prevent
any clobbering, i.e. issues C and D above. So if there is an
irrelevant error, clobbering errno, and then no match is found,
getpwnam_r() will still return that error, but that's no worse
than what we have now.
OK?
Ingo
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 19 Feb 2014 16:36:48 -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;
_THREAD_PRIVATE_MUTEX_LOCK(pw);
+ saved_errno = errno;
+ errno = 0;
if (!_pw_db && !__initdb())
goto fail;
@@ -733,8 +737,12 @@ getpwnam_r(const char *name, struct pass
fail:
if (pwretp)
*pwretp = pwret;
+ if (pwret == NULL)
+ my_errno = errno;
+ if (!errno)
+ errno = saved_errno;
_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
- return (pwret ? 0 : 1);
+ return (my_errno);
}
struct passwd *
@@ -753,8 +761,12 @@ getpwuid_r(uid_t uid, struct passwd *pw,
{
struct passwd *pwret = NULL;
int flags = 0, *flagsp;
+ int my_errno = 0;
+ int saved_errno;
_THREAD_PRIVATE_MUTEX_LOCK(pw);
+ saved_errno = errno;
+ errno = 0;
if (!_pw_db && !__initdb())
goto fail;
@@ -778,8 +790,12 @@ getpwuid_r(uid_t uid, struct passwd *pw,
fail:
if (pwretp)
*pwretp = pwret;
+ if (pwret == NULL)
+ my_errno = errno;
+ if (!errno)
+ errno = saved_errno;
_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
- return (pwret ? 0 : 1);
+ return (my_errno);
}
struct passwd *
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 19 Feb 2014 16:36:49 -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;
