Thread: Segmentation fault with postgres -C external_pid_file
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
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
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
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,