[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 15 Jun 2017 13:04:03 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix errno not being set to ERANGE by getgr,
getpw, and getspnam
On Thu, Jun 15, 2017 at 08:18:18AM -0400, Rudolph Pereira wrote:
> Hi,
>
> please see the updated attached patch. In terms of style, I've implemented
> your latter suggestion as it is more compact, but happy either way.
Thanks. Committing it.
Rich
> On 14 June 2017 at 20:05, Rich Felker <dalias@...c.org> wrote:
>
> > On Sun, Jun 11, 2017 at 12:59:51PM -0400, Rudolph Pereira wrote:
> > > 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.
> >
> > It looks like you omitted the change to getgr_r.c corresponding to
> > this one for getpw_r.c:
> >
> > > diff --git a/src/passwd/getpw_r.c b/src/passwd/getpw_r.c
> > > index e8cc811..0c87ab0 100644
> > > --- a/src/passwd/getpw_r.c
> > > +++ b/src/passwd/getpw_r.c
> > > @@ -27,6 +27,7 @@ static int getpw_r(const char *name, uid_t uid, struct
> > passwd *pw, char *buf, si
> > > }
> > > free(line);
> > > pthread_setcancelstate(cs, 0);
> > > + if (rv) errno = rv;
> > > return rv;
> > > }
> >
> > Also:
> >
> > > diff --git a/src/passwd/getspnam_r.c b/src/passwd/getspnam_r.c
> > > index 9233952..47ce3d3 100644
> > > --- a/src/passwd/getspnam_r.c
> > > +++ b/src/passwd/getspnam_r.c
> > > @@ -72,14 +72,24 @@ int getspnam_r(const char *name, struct spwd *sp,
> > char *buf, size_t size, struct
> > >
> > > /* Disallow potentially-malicious user names */
> > > if (*name=='.' || strchr(name, '/') || !l)
> > > + {
> > > + errno = EINVAL;
> > > return EINVAL;
> > > + }
> >
> > Please use consistent style for braces (open brace on if line).
> > Alternatively (if you don't balk at the style; not sure if others will
> > like it), this works:
> >
> > - return EINVAL;
> > + return errno = EINVAL;
> >
> > Rich
> >
> diff --git a/src/passwd/getgr_r.c b/src/passwd/getgr_r.c
> index 7246e8a..f3e8f60 100644
> --- a/src/passwd/getgr_r.c
> +++ b/src/passwd/getgr_r.c
> @@ -34,6 +34,7 @@ static int getgr_r(const char *name, gid_t gid, struct group *gr, char *buf, siz
> free(mem);
> free(line);
> pthread_setcancelstate(cs, 0);
> + if (rv) errno = rv;
> return rv;
> }
>
> diff --git a/src/passwd/getpw_r.c b/src/passwd/getpw_r.c
> index e8cc811..0c87ab0 100644
> --- a/src/passwd/getpw_r.c
> +++ b/src/passwd/getpw_r.c
> @@ -27,6 +27,7 @@ static int getpw_r(const char *name, uid_t uid, struct passwd *pw, char *buf, si
> }
> free(line);
> pthread_setcancelstate(cs, 0);
> + if (rv) errno = rv;
> return rv;
> }
>
> diff --git a/src/passwd/getspnam_r.c b/src/passwd/getspnam_r.c
> index 9233952..e488b67 100644
> --- a/src/passwd/getspnam_r.c
> +++ b/src/passwd/getspnam_r.c
> @@ -72,14 +72,15 @@ int getspnam_r(const char *name, struct spwd *sp, char *buf, size_t size, struct
>
> /* Disallow potentially-malicious user names */
> if (*name=='.' || strchr(name, '/') || !l)
> - return EINVAL;
> + return errno = EINVAL;
>
> /* Buffer size must at least be able to hold name, plus some.. */
> - if (size < l+100) return ERANGE;
> + if (size < l+100)
> + return errno = EINVAL;
>
> /* Protect against truncation */
> if (snprintf(path, sizeof path, "/etc/tcb/%s/shadow", name) >= sizeof path)
> - return EINVAL;
> + return errno = EINVAL;
>
> fd = open(path, O_RDONLY|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC);
> if (fd >= 0) {
> @@ -112,5 +113,6 @@ int getspnam_r(const char *name, struct spwd *sp, char *buf, size_t size, struct
> break;
> }
> pthread_cleanup_pop(1);
> + if (rv) errno = rv;
> return rv;
> }
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.
