From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/4112 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] getcwd: Set errno to EINVAL when size == 0 Date: Mon, 7 Oct 2013 12:21:57 -0400 Message-ID: <20131007162157.GC20515@brightrain.aerifal.cx> References: <1381126104-24579-1-git-send-email-mforney@mforney.org> <1381127894.6107.59.camel@eris.loria.fr> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1381162926 32416 80.91.229.3 (7 Oct 2013 16:22:06 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 7 Oct 2013 16:22:06 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-4116-gllmg-musl=m.gmane.org@lists.openwall.com Mon Oct 07 18:22:10 2013 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1VTDZS-0002Gv-9n for gllmg-musl@plane.gmane.org; Mon, 07 Oct 2013 18:22:10 +0200 Original-Received: (qmail 7353 invoked by uid 550); 7 Oct 2013 16:22:09 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 7345 invoked from network); 7 Oct 2013 16:22:09 -0000 Content-Disposition: inline In-Reply-To: <1381127894.6107.59.camel@eris.loria.fr> User-Agent: Mutt/1.5.21 (2010-09-15) Xref: news.gmane.org gmane.linux.lib.musl.general:4112 Archived-At: On Mon, Oct 07, 2013 at 08:38:14AM +0200, Jens Gustedt wrote: > Hello, > > Am Sonntag, den 06.10.2013, 23:08 -0700 schrieb Michael Forney: > > According to POSIX, > > > > The getcwd() function shall fail if: > > > > [EINVAL] > > The size argument is 0. > > [ERANGE] > > The size argument is greater than 0, but is smaller than the length > > of the string +1. > > --- > > src/unistd/getcwd.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/src/unistd/getcwd.c b/src/unistd/getcwd.c > > index 2e540cd..0238fa7 100644 > > --- a/src/unistd/getcwd.c > > +++ b/src/unistd/getcwd.c > > @@ -8,6 +8,10 @@ char *getcwd(char *buf, size_t size) > > { > > char tmp[PATH_MAX]; > > if (!buf) buf = tmp, size = PATH_MAX; > > + else if (size == 0) { > > + errno = EINVAL; > > + return 0; > > + } > > if (syscall(SYS_getcwd, buf, size) < 0) return 0; > > Is the new error check really necessary? I would have expected the > error path to have triggered before when buf is !0 and size is 0 on > entry. In principle the kernel should be generating the EINVAL if size is 0, but maybe it does the wrong thing...? > > return buf == tmp ? strdup(buf) : buf; > > This in turn doesn't seem to be consistent with the extension that > glibc offers. It says > > > In this case, the allocated buffer has the length size You omitted the rest of that sentence: "unless size is zero, when buf is allocated as big as necessary." > So I would think that strdup(buf) should be replaced by something like > > strcpy(malloc(size), buf) This is definitely unsafe if size is less than strnel(buf)+1. I'm not convinced this aspect of the glibc behavior (using the size argument) is beneficial; the only possible case in which it would be benficial is when the caller wants the returned buffer to have space for appending a filename, which could be achieved by passing PATH_MAX. However, I thought the whole point of having getcwd accept a NULL argument was for the GNU HURD "no PATH_MAX limit" model, in which case you wouldn't even know the right length to pass in order to have space left over to append a filename. If it is deemed important to support this weird GNU behavior, I think it would be beneficial to always allocate MAX(strlen(buf)+1,size) rather than just size, to avoid spurious failure. Opinions from anyone else? Rich