Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.