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

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

From
Bruce Momjian
Date:
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@candle.pha.pa.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@candle.pha.pa.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.davie@relevance.com.au
> cnr Brookes and Heffernan Sts.,   Web:        http://www.relevance.com.au
> Mitchell ACT 2911
> 

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


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

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.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


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

From
Peter Davie
Date:
Hi Guys,<br /><br /><pre wrap="">Please refer to <a class="moz-txt-link-rfc2396E"
href="http://www.opengroup.org/onlinepubs/009695399/functions/getpwuid.html"><http://www.opengroup.org/onlinepubs/009695399/functions/getpwuid.html></a>:

"[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
(numerousinserts per minute, sustained) for a few weeks now.
 

Thanks
Peter</pre><br /> Tom Lane wrote:<br /><blockquote cite="mid24145.1097298418@sss.pgh.pa.us" type="cite"><pre
wrap="">BruceMomjian <a class="moz-txt-link-rfc2396E"
href="mailto:pgman@candle.pha.pa.us"><pgman@candle.pha.pa.us></a>writes: </pre><blockquote type="cite"><pre
wrap="">Whatdo 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.   </pre></blockquote><pre wrap="">
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 </pre></blockquote><br /><br /><pre class="moz-signature" cols="72">--
  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: <a class="moz-txt-link-abbreviated"
href="mailto:peter.davie@relevance.com.au">peter.davie@relevance.com.au</a>
Mitchell ACT 2911                       Web:    <a class="moz-txt-link-freetext"
href="http://www.relevance.com.au">http://www.relevance.com.au</a></pre>