Thread: Segmentation fault with postgres -C external_pid_file

Segmentation fault with postgres -C external_pid_file

From
Jehan-Guillaume de Rorthais
Date:
Hello,

I just found that the following command always produce a segmentation fault
when external_pid_file is kept commented:

  $ postgres -C external_pid_file
  Segmentation fault (core dumped)


Here is the full backtrace from the core file using 9.5.3:

  (gdb) bt full
  #0  strlen () at ../sysdeps/x86_64/strlen.S:106
  No locals.
  #1  0x00007fe0f397e99c in _IO_puts (str=0x0) at ioputs.c:36
        result = -1
        len = <optimized out>
  #2  0x0000000000643dc6 in PostmasterMain (argc=argc@entry=3,
  argv=argv@entry=0x297c170) at postmaster.c:827
        opt = <optimized out>
        status = <optimized out>
        userDoption = 0x0
        listen_addr_saved = 0 '\000'
        i = <optimized out>
        output_config_variable = 0x2997c90 "external_pid_file"
        __func__ = "PostmasterMain"
  #3  0x000000000046a377 in main (argc=3, argv=0x297c170) at main.c:228
  No locals.


This has been tested only with 9.5 and 9.4.

The following simple patch seems to fix this bug:

  diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
  index f249b390..0362555 100644
  --- a/src/backend/utils/misc/guc.c
  +++ b/src/backend/utils/misc/guc.c
  @@ -3269,7 +3269,7 @@ static struct config_string ConfigureNamesString[] =
                          GUC_SUPERUSER_ONLY
                  },
                  &external_pid_file,
  -               NULL,
  +               "",
                  check_canonical_path, NULL, NULL
          },


Regards,
--
Jehan-Guillaume de Rorthais
Dalibo

Re: Segmentation fault with postgres -C external_pid_file

From
Michael Paquier
Date:
On Thu, Jun 16, 2016 at 3:40 AM, Jehan-Guillaume de Rorthais
<jgdr@dalibo.com> wrote:
> I just found that the following command always produce a segmentation fault
> when external_pid_file is kept commented:
>
>   $ postgres -C external_pid_file
>   Segmentation fault (core dumped)

Confirmed. puts() would print (null) on OSX even if that's not
mentioned in the standard, and Linux would just segfault on it.
config_file, hba_file or ident_file don't suffer that because they are
set at startup using PGDATA.

> Here is the full backtrace from the core file using 9.5.3:
> This has been tested only with 9.5 and 9.4.
>
> The following simple patch seems to fix this bug:

Another thing that we could do is as well to set external_pid_file to
PGDATA/postmaster.pid at startup like the other *_file GUCs for
consistency's sake, but I'd just use your patch to not change the
default setting on back-branches. Some applications may depend on the
current behavior regarding its default value, that's hard to know..

By the way, I found as well that session_authorization suffers the
same problem. In the latter case the variable where is allocated the
value is completely dummy as only its assign hook is used but we
should definitely not crash on that at postmaster startup. And the
trick is that we also should have a NULL value in this case because
check_session_authorization relies on that.

By the way, your patch is not correct. By setting up a value "" as
default value postmaster.c complains with a "cannot write external PID
file" error.

So instead of your patch, I think that we'd rather just change
PostmasterMain() so as it outputs something useful to the user in case
of a NULL value, like that:
$ postgres -C external_pid_file
(null)
$ postgres -C session_authorization
(null)
This will prevent any other error of this type in the future for new
parameters, and has no risk of impacting existing applications. See
the simple patch attached.
--
Michael

Attachment

Re: Segmentation fault with postgres -C external_pid_file

From
Tom Lane
Date:
Michael Paquier <michael.paquier@gmail.com> writes:
> On Thu, Jun 16, 2016 at 3:40 AM, Jehan-Guillaume de Rorthais
> <jgdr@dalibo.com> wrote:
>> I just found that the following command always produce a segmentation fault
>> when external_pid_file is kept commented:
>> $ postgres -C external_pid_file
>> Segmentation fault (core dumped)

> Confirmed. puts() would print (null) on OSX even if that's not
> mentioned in the standard, and Linux would just segfault on it.

Right, this is going to be platform-specific behavior.

> So instead of your patch, I think that we'd rather just change
> PostmasterMain() so as it outputs something useful to the user in case
> of a NULL value, like that:

Agreed, fixing the -C code seems much less likely to have unexpected
side-effects than changing default values.  Will push this in a bit.

            regards, tom lane

Re: Segmentation fault with postgres -C external_pid_file

From
Jehan-Guillaume de Rorthais
Date:
Le Thu, 16 Jun 2016 11:50:07 -0400,
Tom Lane <tgl@sss.pgh.pa.us> a =C3=A9crit :

> Michael Paquier <michael.paquier@gmail.com> writes:
> > On Thu, Jun 16, 2016 at 3:40 AM, Jehan-Guillaume de Rorthais
> > <jgdr@dalibo.com> wrote:
> >> I just found that the following command always produce a segmentation =
fault
> >> when external_pid_file is kept commented:
> >> $ postgres -C external_pid_file
> >> Segmentation fault (core dumped)
>=20
> > Confirmed. puts() would print (null) on OSX even if that's not
> > mentioned in the standard, and Linux would just segfault on it.
>=20
> Right, this is going to be platform-specific behavior.
>=20
> > So instead of your patch, I think that we'd rather just change
> > PostmasterMain() so as it outputs something useful to the user in case
> > of a NULL value, like that:
>=20
> Agreed, fixing the -C code seems much less likely to have unexpected
> side-effects than changing default values.  Will push this in a bit.

Thank you for the fix guys.

Regards,