[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131007162157.GC20515@brightrain.aerifal.cx>
Date: Mon, 7 Oct 2013 12:21:57 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] getcwd: Set errno to EINVAL when size == 0
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
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.
