Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20170615170403.GP1627@brightrain.aerifal.cx>
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.