[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMyp-nLW+nm_02LmvkN5SQBNv-YiKdag3M=yfmAgQwVz7HSJjw@mail.gmail.com>
Date: Sun, 11 Jun 2017 12:59:51 -0400
From: Rudolph Pereira <rudolph.pereira@...amsec.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix errno not being set to ERANGE by getgr, getpw,
and getspnam
Hi Rich,
thanks for the feedback. I've attached a patch that implements errno
setting as you suggested, other than a couple of cases where the code
immediately returns. This also brings it in line with existing code
(in __getpw_a/__getgr_a) so makes things more consistent. Please see
attached - note this is against HEAD.
On 8 June 2017 at 20:00, Rich Felker <dalias@...c.org> wrote:
> On Tue, Jun 06, 2017 at 12:21:39PM -0400, Rudolph Pereira wrote:
> > Hi,
> >
> > currently, the getgr, getpw, and getspnam functions in musl return ERANGE
> > when the allocated buffer is not large enough, but do not set errno to
> the
> > same value. This causes issues with utilities, for example the "shadow"
> > utilities (useradd/mod, groupmod, etc.) which assume this behaviour
> (which
> > at least gnu libc exhibits) and leads to groups having a small limit on
> the
> > number of members.
> >
> > The attached patch, against 1.1.16, corrects this.
> >
> > Cheers,
> > Rudolph
>
> > --- musl-1.1.16.orig/src/passwd/getgr_r.c
> > +++ musl-1.1.16/src/passwd/getgr_r.c
> > @@ -1,5 +1,6 @@
> > #include "pwf.h"
> > #include <pthread.h>
> > +#include <errno.h>
> >
> > #define FIX(x) (gr->gr_##x = gr->gr_##x-line+buf)
> >
> > @@ -19,6 +20,7 @@
> > if (*res && size < len + (nmem+1)*sizeof(char *) + 32) {
> > *res = 0;
> > rv = ERANGE;
> > + errno = ERANGE;
> > }
> > if (*res) {
> > buf += (16-(uintptr_t)buf)%16;
> > --- musl-1.1.16.orig/src/passwd/getpw_r.c
> > +++ musl-1.1.16/src/passwd/getpw_r.c
> > @@ -1,5 +1,6 @@
> > #include "pwf.h"
> > #include <pthread.h>
> > +#include <errno.h>
> >
> > #define FIX(x) (pw->pw_##x = pw->pw_##x-line+buf)
> >
> > @@ -16,6 +17,7 @@
> > if (*res && size < len) {
> > *res = 0;
> > rv = ERANGE;
> > + errno = ERANGE;
> > }
> > if (*res) {
> > memcpy(buf, line, len);
> > --- musl-1.1.16.orig/src/passwd/getspnam_r.c
> > +++ musl-1.1.16/src/passwd/getspnam_r.c
> > @@ -3,6 +3,7 @@
> > #include <sys/stat.h>
> > #include <ctype.h>
> > #include <pthread.h>
> > +#include <errno.h>
> > #include "pwf.h"
> >
> > /* This implementation support Openwall-style TCB passwords in place of
> > @@ -104,6 +105,7 @@
> > }
> > if (buf[k-1] != '\n') {
> > rv = ERANGE;
> > + errno = ERANGE;
> > break;
> > }
>
> I don't think this patch is complete. A nonzero value of rv can also
> come from __getpw_a/__getgr_a. Insted, just before return there should
> probably be:
>
> if (rv) errno = rv;
>
> Does that look correct? I'm not sure about getspnam_r; it might
> actually be missing some error cases right now.
>
> Rich
>
Content of type "text/html" skipped
View attachment "getX-errno-v2.patch" of type "text/x-patch" (1317 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
