[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 7 Oct 2013 18:15:24 +0100
From: Justin Cormack <justin@...cialbusservice.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] getcwd: Set errno to EINVAL when size == 0
On Mon, Oct 7, 2013 at 5:21 PM, Rich Felker <dalias@...ifal.cx> wrote:
> 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?
I can't see any way in which the user could detect (in the malloc
case) that you always allocated PATH_MAX not the provided size, so you
may as well just do that if they insist on using this stupid interface
in the first place.
Justin
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.
