Thread: pg_ctl infinite loop and memory leak

pg_ctl infinite loop and memory leak

From
Jeff Davis
Date:
To reproduce:

1. initdb -D data
2. cat /dev/null > data/postgresql.conf
3. pg_ctl -w -D data start

I attached a quick patch that seems to do the trick. It appears that
fgets() will always return non-NULL if the size passed in is 1 (i.e.
maxlength in the caller is 0).

The patch also changes the same readfile() function in initdb.c. I
assume it's not a practical problem there, but it should be fixed.

Thanks to Corry Haines (chaines at truviso dot com) for reporting the
problem.

Regards,
    Jeff Davis

Attachment

Re: pg_ctl infinite loop and memory leak

From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes:
> I attached a quick patch that seems to do the trick. It appears that
> fgets() will always return non-NULL if the size passed in is 1 (i.e.
> maxlength in the caller is 0).

Huh, interesting corner case.  I'd be inclined to fix by initializing
maxlength to 1 though.

Where's the memory leak?

            regards, tom lane

Re: pg_ctl infinite loop and memory leak

From
Jeff Davis
Date:
On Tue, 2009-09-01 at 22:01 -0400, Tom Lane wrote:
> Huh, interesting corner case.  I'd be inclined to fix by initializing
> maxlength to 1 though.
>
> Where's the memory leak?

The xstrdup() on the zero-length string.

Regards,
    Jeff Davis

Re: pg_ctl infinite loop and memory leak

From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes:
> On Tue, 2009-09-01 at 22:01 -0400, Tom Lane wrote:
>> Where's the memory leak?

> The xstrdup() on the zero-length string.

Oh, I see.  But actually, it's also clobbering memory like crazy (since
we'll run off the result[] array in no time).  Surprising it doesn't
crash from that.

            regards, tom lane

Re: pg_ctl infinite loop and memory leak

From
Bruce Momjian
Date:
Jeff Davis wrote:
> To reproduce:
>
> 1. initdb -D data
> 2. cat /dev/null > data/postgresql.conf
> 3. pg_ctl -w -D data start
>
> I attached a quick patch that seems to do the trick. It appears that
> fgets() will always return non-NULL if the size passed in is 1 (i.e.
> maxlength in the caller is 0).
>
> The patch also changes the same readfile() function in initdb.c. I
> assume it's not a practical problem there, but it should be fixed.
>
> Thanks to Corry Haines (chaines at truviso dot com) for reporting the
> problem.

I thought I knew the libc API pretty well, but I learned something new
here.  :-)

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +