Re: [BUGS] BUG #1270: stack overflow in thread in fe_getauthname

What do people think about using (sizeof(struct passwd) + BUFLEN/2) rather
than BUFLEN for the getpwuid_r size, or (sizeof(struct passwd) + MAXPGPATH*2)?
That would reduce the stack requirements and still be safe, I think.

---------------------------------------------------------------------------

Peter Davie wrote:
> Hi Bruce,
>
> (As I explained to Tom, the situation for me is difficult:
>
> How can I "fix the app" when the application is a commercial binary which
> provides no means of configuring the stack size for the thread? I have
> been successfully using PostgreSQL with this application since v7.1 and
> have a happy customer with it. Now, upgrading the database to v7.4.5
> which is strongly recommended by the development group (due to potentially
> serious bugs) means that the application no longer works!
> )
>
> Since then I have done some background reading
> <http://www.opengroup.org/onlinepubs/009695399/functions/getpwuid.html>:
>
> [TSF] The getpwuid_r() function shall update the passwd structure pointed
> to by pwd and store a pointer to that structure at the location pointed to
> by result. The structure shall contain an entry from the user database
> with a matching uid. Storage referenced by the structure is allocated from
> the memory provided with the buffer parameter, which is bufsize bytes in
> size. The maximum size needed for this buffer can be determined with the
> {_SC_GETPW_R_SIZE_MAX} sysconf() parameter. A NULL pointer shall be
> returned at the location pointed to by result on error or if the requested
> entry is not found.
>
> On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.
>
> Thanks
> Peter
>
> > Tom Lane wrote:
> >> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> > Oops. Yep, that is sloppy programming on our part, perhaps my part if
> >> I
> >> > added those. Anyway, patch attached and applied. I used the proper
> >> > struct sizes instead of BUFSIZ.
> >>
> >> You just broke it.
> >>
> >> Those buffers are not used to hold struct passwd's, but to hold
> >> multiple character strings to which the struct passwd will point;
> >> any one of which could be long, but particularly the home directory
> >> path.
> >>
> >> My man page for getpwuid_r says that the minimum recommended buffer size
> >> is 1024.
> >>
> >> > This will be in 8.0.
> >>
> >> I think we should revert it entirely. A small buffer size risks
> >> breaking things unnecessarily, and as I replied earlier, the request
> >> to make libpq run in a less-than-8K stack is not reasonable anyway.
> >
> > Reverted. I forgot about the requirement to store pointers used by the
> > structure. I knew that when doing the thread patches but forgot about
> > it.
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania
> > 19073
> >
>
>
> --
> Relevance Phone: +61 2 6241 6400
> A.B.N. 86 063 403 690 Fax: +61 2 6241 6422
> Unit 11, Mitchell Commercial Ctr, E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
> cnr Brookes and Heffernan Sts., Web: http://www.relevance.com.au
> Mitchell ACT 2911
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> What do people think about using (sizeof(struct passwd) + BUFLEN/2) rather
> than BUFLEN for the getpwuid_r size, or (sizeof(struct passwd) + MAXPGPATH*2)?
> That would reduce the stack requirements and still be safe, I think.

Why bother?

Peter did not say what his closed-source app could tolerate. Without
that knowledge you're just flying blind about fixing his problem.
I see no reason to risk creating buffer-overflow issues for other people
in order to make a maybe-or-maybe-not improvement for one rather broken
closed-source app...

regards, tom lane

Hi Guys,

Please refer to <http://www.opengroup.org/onlinepubs/009695399/functions/getpwuid.html>:

"[TSF] The getpwuid_r() function shall update the passwd structure pointed
to by pwd and store a pointer to that structure at the location pointed to
by result. The structure shall contain an entry from the user database
with a matching uid. Storage referenced by the structure is allocated from
the memory provided with the buffer parameter, which is bufsize bytes in
size. The maximum size needed for this buffer can be determined with the
{_SC_GETPW_R_SIZE_MAX} sysconf() parameter. A NULL pointer shall be
returned at the location pointed to by result on error or if the requested
entry is not found."

On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.

When I modified the source, I punted a value of 1024 and this has worked flawlessly in an intensive environment (numerous inserts per minute, sustained) for a few weeks now.

Thanks
Peter

Tom Lane wrote:

>Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
>
>>What do people think about using (sizeof(struct passwd) + BUFLEN/2) rather
>>than BUFLEN for the getpwuid_r size, or (sizeof(struct passwd) + MAXPGPATH*2)?
>>That would reduce the stack requirements and still be safe, I think.
>>
>>
>
>Why bother?
>
>Peter did not say what his closed-source app could tolerate. Without
>that knowledge you're just flying blind about fixing his problem.
>I see no reason to risk creating buffer-overflow issues for other people
>in order to make a maybe-or-maybe-not improvement for one rather broken
>closed-source app...
>
> regards, tom lane
>
>

--
Relevance... because results count

Relevance Phone: +61 (0)2 6241 6400
A.B.N. 86 063 403 690 Fax: +61 (0)2 6241 6422
Unit 11, Mitchell Commercial Centre, Mobile: +61 (0)417 265 175
cnr Brookes and Heffernan Sts., E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
Mitchell ACT 2911 Web: http://www.relevance.com.au

OK, we got a report. I just thinkg 8192 is excessive for that
structure, and if someone is having a problem, others might as well.

---------------------------------------------------------------------------

Peter Davie wrote:
> Hi Guys,
>
> Please refer to <http://www.opengroup.org/onlinepubs/009695399/functions/getpwuid.html>:
>
> "[TSF] The getpwuid_r() function shall update the passwd structure pointed
> to by pwd and store a pointer to that structure at the location pointed to
> by result. The structure shall contain an entry from the user database
> with a matching uid. Storage referenced by the structure is allocated from
> the memory provided with the buffer parameter, which is bufsize bytes in
> size. The maximum size needed for this buffer can be determined with the
> {_SC_GETPW_R_SIZE_MAX} sysconf() parameter. A NULL pointer shall be
> returned at the location pointed to by result on error or if the requested
> entry is not found."
>
>
>
> On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.
>
> When I modified the source, I punted a value of 1024 and this has worked flawlessly in an intensive environment (numerous inserts per minute, sustained) for a few weeks now.
>
> Thanks
> Peter
>
>
> Tom Lane wrote:
>
> >Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >
> >
> >>What do people think about using (sizeof(struct passwd) + BUFLEN/2) rather
> >>than BUFLEN for the getpwuid_r size, or (sizeof(struct passwd) + MAXPGPATH*2)?
> >>That would reduce the stack requirements and still be safe, I think.
> >>
> >>
> >
> >Why bother?
> >
> >Peter did not say what his closed-source app could tolerate. Without
> >that knowledge you're just flying blind about fixing his problem.
> >I see no reason to risk creating buffer-overflow issues for other people
> >in order to make a maybe-or-maybe-not improvement for one rather broken
> >closed-source app...
> >
> > regards, tom lane
> >
> >
>
>
> --
> Relevance... because results count
>
> Relevance Phone: +61 (0)2 6241 6400
> A.B.N. 86 063 403 690 Fax: +61 (0)2 6241 6422
> Unit 11, Mitchell Commercial Centre, Mobile: +61 (0)417 265 175
> cnr Brookes and Heffernan Sts., E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
> Mitchell ACT 2911 Web: http://www.relevance.com.au
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> OK, we got a report. I just thinkg 8192 is excessive for that
> structure, and if someone is having a problem, others might as well.

>> On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.

I'd be more impressed by this line of reasoning if _SC_GETPW_R_SIZE_MAX
were defined on more than one of the platforms I use...

regards, tom lane

Hi Tom,

How many of these platforms you use are POSIX-compliant? This
information came from the POSIX web site (NOT THE DIGITAL/COMPAQ/HP/...
WEBSITE). Sysconf(_SC_GETPW_R_SIZE_MAX) is supported by Solaris 2.5,
SCO UNIX (circa 1999!), Digital UNIX/Compaq Tru64 UNIX, FreeBSD, AIX,
HP-UX, and probably many more.

Maybe the non-compliant platforms should be catered for as "legacy" with
support code in the "ports" area, and those that do adhere to open
standards can be accommodated without breaking existing production
applications.

Thanks
Peter

Tom Lane wrote:

>Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
>
>>OK, we got a report. I just thinkg 8192 is excessive for that
>>structure, and if someone is having a problem, others might as well.
>>
>>
>
>
>
>>>On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.
>>>
>>>
>
>I'd be more impressed by this line of reasoning if _SC_GETPW_R_SIZE_MAX
>were defined on more than one of the platforms I use...
>
> regards, tom lane
>
>

--
Relevance... because results count

Relevance Phone: +61 (0)2 6241 6400
A.B.N. 86 063 403 690 Fax: +61 (0)2 6241 6422
Unit 11, Mitchell Commercial Centre, Mobile: +61 (0)417 265 175
cnr Brookes and Heffernan Sts., E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
Mitchell ACT 2911 Web: http://www.relevance.com.au

One idea would be to use malloc() to allocate storage for the
thread-safe buffers when compiled with thread-safety, rather than using
the stack.

---------------------------------------------------------------------------

Peter Davie wrote:
> Hi Tom,
>
> How many of these platforms you use are POSIX-compliant? This
> information came from the POSIX web site (NOT THE DIGITAL/COMPAQ/HP/...
> WEBSITE). Sysconf(_SC_GETPW_R_SIZE_MAX) is supported by Solaris 2.5,
> SCO UNIX (circa 1999!), Digital UNIX/Compaq Tru64 UNIX, FreeBSD, AIX,
> HP-UX, and probably many more.
>
> Maybe the non-compliant platforms should be catered for as "legacy" with
> support code in the "ports" area, and those that do adhere to open
> standards can be accommodated without breaking existing production
> applications.
>
> Thanks
> Peter
>
> Tom Lane wrote:
>
> >Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >
> >
> >>OK, we got a report. I just thinkg 8192 is excessive for that
> >>structure, and if someone is having a problem, others might as well.
> >>
> >>
> >
> >
> >
> >>>On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.
> >>>
> >>>
> >
> >I'd be more impressed by this line of reasoning if _SC_GETPW_R_SIZE_MAX
> >were defined on more than one of the platforms I use...
> >
> > regards, tom lane
> >
> >
>
>
> --
> Relevance... because results count
>
> Relevance Phone: +61 (0)2 6241 6400
> A.B.N. 86 063 403 690 Fax: +61 (0)2 6241 6422
> Unit 11, Mitchell Commercial Centre, Mobile: +61 (0)417 265 175
> cnr Brookes and Heffernan Sts., E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
> Mitchell ACT 2911 Web: http://www.relevance.com.au
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Hi Bruce,

That would work!

Thanks
Peter

Bruce Momjian wrote:

>One idea would be to use malloc() to allocate storage for the
>thread-safe buffers when compiled with thread-safety, rather than using
>the stack.
>
>---------------------------------------------------------------------------
>
>Peter Davie wrote:
>
>
>>Hi Tom,
>>
>>How many of these platforms you use are POSIX-compliant? This
>>information came from the POSIX web site (NOT THE DIGITAL/COMPAQ/HP/...
>>WEBSITE). Sysconf(_SC_GETPW_R_SIZE_MAX) is supported by Solaris 2.5,
>>SCO UNIX (circa 1999!), Digital UNIX/Compaq Tru64 UNIX, FreeBSD, AIX,
>>HP-UX, and probably many more.
>>
>>Maybe the non-compliant platforms should be catered for as "legacy" with
>>support code in the "ports" area, and those that do adhere to open
>>standards can be accommodated without breaking existing production
>>applications.
>>
>>Thanks
>>Peter
>>
>>Tom Lane wrote:
>>
>>
>>
>>>Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>>>
>>>
>>>
>>>
>>>>OK, we got a report. I just thinkg 8192 is excessive for that
>>>>structure, and if someone is having a problem, others might as well.
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>>>On Tru64 UNIX, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024.
>>>>>
>>>>>
>>>>>
>>>>>
>>>I'd be more impressed by this line of reasoning if _SC_GETPW_R_SIZE_MAX
>>>were defined on more than one of the platforms I use...
>>>
>>> regards, tom lane
>>>
>>>
>>>
>>>
>>--
>> Relevance... because results count
>>
>>Relevance Phone: +61 (0)2 6241 6400
>>A.B.N. 86 063 403 690 Fax: +61 (0)2 6241 6422
>>Unit 11, Mitchell Commercial Centre, Mobile: +61 (0)417 265 175
>>cnr Brookes and Heffernan Sts., E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
>>Mitchell ACT 2911 Web: http://www.relevance.com.au
>>
>>
>>
>
>
>

--
Relevance... because results count

Relevance Phone: +61 (0)2 6241 6400
A.B.N. 86 063 403 690 Fax: +61 (0)2 6241 6422
Unit 11, Mitchell Commercial Centre, Mobile: +61 (0)417 265 175
cnr Brookes and Heffernan Sts., E-mail: peter(dot)davie(at)relevance(dot)com(dot)au
Mitchell ACT 2911 Web: http://www.relevance.com.au