|
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.