Worth using personality(ADDR_NO_RANDOMIZE) for EXEC_BACKEND on linux?
Hi,
When testing EXEC_BACKEND on linux I see occasional test failures as long as I
don't disable ASLR. There's a code comment to that effect:
* If testing EXEC_BACKEND on Linux, you should run this as root before
* starting the postmaster:
*
* echo 0 >/proc/sys/kernel/randomize_va_space
but I don't like doing that on a system wide basis.
Linux allows disabling ASLR on a per-process basis using
personality(ADDR_NO_RANDOMIZE). There's a wrapper binary to do that as well,
setarch --addr-no-randomize.
I was wondering if we should have postmaster do personality(ADDR_NO_RANDOMIZE)
for EXEC_BACKEND builds? It seems nicer to make it automatically work than
have people remember that they need to call "setarch --addr-no-randomize make check".
Not that it actually matters for EXEC_BACKEND, but theoretically doing
personality(ADDR_NO_RANDOMIZE) in postmaster is a tad more secure than doing
it via setarch, as in the personality() case postmaster's layout itself is
still randomized...
Or perhaps we should just add a comment mentioning setarch.
Greetings,
Andres Freund
On 8/5/21 11:29 PM, Andres Freund wrote:
Hi,
When testing EXEC_BACKEND on linux I see occasional test failures as long as I
don't disable ASLR. There's a code comment to that effect:* If testing EXEC_BACKEND on Linux, you should run this as root before
* starting the postmaster:
*
* echo 0 >/proc/sys/kernel/randomize_va_spacebut I don't like doing that on a system wide basis.
Linux allows disabling ASLR on a per-process basis using
personality(ADDR_NO_RANDOMIZE). There's a wrapper binary to do that as well,
setarch --addr-no-randomize.I was wondering if we should have postmaster do personality(ADDR_NO_RANDOMIZE)
for EXEC_BACKEND builds? It seems nicer to make it automatically work than
have people remember that they need to call "setarch --addr-no-randomize make check".Not that it actually matters for EXEC_BACKEND, but theoretically doing
personality(ADDR_NO_RANDOMIZE) in postmaster is a tad more secure than doing
it via setarch, as in the personality() case postmaster's layout itself is
still randomized...Or perhaps we should just add a comment mentioning setarch.
If we can set it conveniently then that seems worth doing.
(Thinks: do we have non-Windows buildfarm members doing EXEC_BACKEND?)
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 2021-Aug-06, Andrew Dunstan wrote:
On 8/5/21 11:29 PM, Andres Freund wrote:
I was wondering if we should have postmaster do personality(ADDR_NO_RANDOMIZE)
for EXEC_BACKEND builds? It seems nicer to make it automatically work than
have people remember that they need to call "setarch --addr-no-randomize make check".
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.
Not that it actually matters for EXEC_BACKEND, but theoretically doing
personality(ADDR_NO_RANDOMIZE) in postmaster is a tad more secure than doing
it via setarch, as in the personality() case postmaster's layout itself is
still randomized...
True. I think the security aspect is not critically important, since
hopefully nobody should be using such builds for production.
(Thinks: do we have non-Windows buildfarm members doing EXEC_BACKEND?)
culicidae does that.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Pido que me den el Nobel por razones humanitarias" (Nicanor Parra)
On Mon, Aug 9, 2021 at 1:30 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.
On macOS, failures are extremely common. Sometimes I have to run
simple tests many times to get even one success. The proposal on the
table won't help with that problem since it's Linux-specific, but if
there's any way to do something similar on macOS it would be a _huge_
help.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2021-08-09 13:43:03 -0400, Robert Haas wrote:
On Mon, Aug 9, 2021 at 1:30 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.
I get check-world failures in about 1/2-1/3 of the runs, and a plain check
fails in maybe 1/4 of the cases. It's pretty annoying because it often isn't
trivial to distinguish whether I've broken something or whether it's
randomization related...
The frequency of failures isn't stable over time, making it a bit harder to
know what's going on.
On macOS, failures are extremely common. Sometimes I have to run
simple tests many times to get even one success. The proposal on the
table won't help with that problem since it's Linux-specific, but if
there's any way to do something similar on macOS it would be a _huge_
help.
There does seem to be a way of doing so, because I found blog posts talking
about how to get e.g. lldb to *enable* ASLR, which it disables by default...
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2021-08-09 13:43:03 -0400, Robert Haas wrote:
On Mon, Aug 9, 2021 at 1:30 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.
I get check-world failures in about 1/2-1/3 of the runs, and a plain check
fails in maybe 1/4 of the cases. It's pretty annoying because it often isn't
trivial to distinguish whether I've broken something or whether it's
randomization related...
I don't have numbers, but I do know that on Linux EXEC_BACKEND builds fail
often enough to be annoying if you don't disable ASLR. If we can do
something not-too-invasive about that, it'd be great.
regards, tom lane
Hi,
On 2021-08-09 13:54:25 -0400, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2021-08-09 13:43:03 -0400, Robert Haas wrote:
On Mon, Aug 9, 2021 at 1:30 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.I get check-world failures in about 1/2-1/3 of the runs, and a plain check
fails in maybe 1/4 of the cases. It's pretty annoying because it often isn't
trivial to distinguish whether I've broken something or whether it's
randomization related...I don't have numbers, but I do know that on Linux EXEC_BACKEND builds fail
often enough to be annoying if you don't disable ASLR. If we can do
something not-too-invasive about that, it'd be great.
I now not sure if personality(NO_RANDOMIZE) in postmaster is actually
sufficient. It does seem to drastically reduce the frequency of issues, but
there's still conceivable failures where postmaster's layout would class with
the non-randomized children - obviously personality(NO_RANDOMIZE) cannot
retroactively change the layout of the already running binary.
So maybe we should put it into pg_ctl?
Greetings,
Andres Freund
On Tue, Aug 10, 2021 at 5:43 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Aug 9, 2021 at 1:30 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.On macOS, failures are extremely common. Sometimes I have to run
simple tests many times to get even one success. The proposal on the
table won't help with that problem since it's Linux-specific, but if
there's any way to do something similar on macOS it would be a _huge_
help.
Yeah, make check always fails for me on macOS 11. With the attached
experimental hack, it fails only occasionally (1 in 8 runs or so). I
don't know why.
Attachments:
0001-Try-to-make-EXEC_BACKEND-more-convenient-on-macOS.patchapplication/x-patch; name=0001-Try-to-make-EXEC_BACKEND-more-convenient-on-macOS.patchDownload
From 9bcedede452c1f37dd790f86bc587353cc455e3f Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Tue, 10 Aug 2021 22:05:00 +1200
Subject: [PATCH] Try to make EXEC_BACKEND more convenient on macOS.
Use posix_spawn() instead of fork() + execv(), with a special
undocumented flag that disables ASLR, if you build with -DEXEC_BACKEND
-DUSE_POSIX_SPAWN -DUSE_POSIX_SPAWN_DISABLE_ASLR.
XXX Experiment only...
XXX Still fails make check occasionally :-(
---
src/backend/postmaster/postmaster.c | 33 +++++++++++++++++++
src/bin/pg_ctl/pg_ctl.c | 51 ++++++++++++++++++++++++++++-
2 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index fc0bc8d99e..3a1e9ae8f8 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -93,6 +93,10 @@
#include <pthread.h>
#endif
+#ifdef USE_POSIX_SPAWN
+#include <spawn.h>
+#endif
+
#include "access/transam.h"
#include "access/xlog.h"
#include "catalog/pg_control.h"
@@ -4585,6 +4589,10 @@ internal_forkexec(int argc, char *argv[], Port *port)
char tmpfilename[MAXPGPATH];
BackendParameters param;
FILE *fp;
+#ifdef USE_POSIX_SPAWN
+ posix_spawn_file_actions_t spawn_file_actions;
+ posix_spawnattr_t spawnattrs;
+#endif
if (!save_backend_variables(¶m, port))
return -1; /* log made by save_backend_variables */
@@ -4642,6 +4650,30 @@ internal_forkexec(int argc, char *argv[], Port *port)
/* Insert temp file name after --fork argument */
argv[2] = tmpfilename;
+#ifdef USE_POSIX_SPAWN
+ posix_spawn_file_actions_init(&spawn_file_actions);
+ posix_spawnattr_init(&spawnattrs);
+#ifdef USE_POSIX_SPAWN_DISABLE_ASLR
+ /*
+ * Undocumented magic. See bsd/sys/spawn.h and bsd/kern/kern_exec.c in the
+ * Darwin sources at https://github.com/apple/darwin-xnu.
+ */
+ if ((errno = posix_spawnattr_setflags(&spawnattrs, 0x0100)) != 0)
+ elog(ERROR, "could not set ASLR disable flag when spawning backend: %m");
+#endif
+ errno = posix_spawn(&pid,
+ postgres_exec_path,
+ &spawn_file_actions,
+ &spawnattrs,
+ argv,
+ NULL);
+ if (errno != 0)
+ {
+ ereport(LOG,
+ (errmsg("could not spawn server process \"%s\": %m",
+ postgres_exec_path)));
+ }
+#else
/* Fire off execv in child */
if ((pid = fork_process()) == 0)
{
@@ -4654,6 +4686,7 @@ internal_forkexec(int argc, char *argv[], Port *port)
exit(1);
}
}
+#endif
return pid; /* Parent returns pid, or -1 on fork failure */
}
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..f105e483c4 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -18,6 +18,10 @@
#include <sys/wait.h>
#include <unistd.h>
+#ifdef USE_POSIX_SPAWN
+#include <spawn.h>
+#endif
+
#ifdef HAVE_SYS_RESOURCE_H
#include <sys/time.h>
#include <sys/resource.h>
@@ -442,15 +446,59 @@ free_readfile(char **optlines)
static pgpid_t
start_postmaster(void)
{
+#ifdef USE_POSIX_SPAWN
+ posix_spawn_file_actions_t spawn_file_actions;
+ posix_spawnattr_t spawnattrs;
+#else
char cmd[MAXPGPATH];
+#endif
#ifndef WIN32
- pgpid_t pm_pid;
+ pid_t pm_pid;
/* Flush stdio channels just before fork, to avoid double-output problems */
fflush(stdout);
fflush(stderr);
+#ifdef USE_POSIX_SPAWN
+ posix_spawn_file_actions_init(&spawn_file_actions);
+ posix_spawnattr_init(&spawnattrs);
+#ifdef USE_POSIX_SPAWN_DISABLE_ASLR
+ /*
+ * Undocumented magic. See bsd/sys/spawn.h and bsd/kern/kern_exec.c in the
+ * Darwin sources at https://github.com/apple/darwin-xnu.
+ */
+ if ((errno = posix_spawnattr_setflags(&spawnattrs, 0x0100)) != 0)
+ write_stderr(_("could not set undocumented ASLR disable flag when spawning postmaster: %s"),
+ strerror(errno));
+#endif
+ {
+ /* XXX:HACK this is incomplete, doesn't include the postopts... */
+ /* XXX Theory here was that shell exec style used by the exec code
+ * might drop the flag, hence spawning executable directly, but doing
+ * that properly would involve unpicking the options and their quotes
+ * etc... */
+ char *args[] = {
+ exec_path,
+ "-D",
+ getenv("PGDATA"),
+ NULL
+ };
+ errno = posix_spawn(&pm_pid,
+ exec_path,
+ &spawn_file_actions,
+ &spawnattrs,
+ args,
+ NULL);
+ }
+ if (errno != 0)
+ {
+ write_stderr(_("%s: could not start server: %s\n"),
+ progname, strerror(errno));
+ exit(1);
+ }
+ return pm_pid;
+#else
pm_pid = fork();
if (pm_pid < 0)
{
@@ -502,6 +550,7 @@ start_postmaster(void)
exit(1);
return 0; /* keep dumb compilers quiet */
+#endif
#else /* WIN32 */
--
2.30.1 (Apple Git-130)
Hi,
On Tue, Aug 10, 2021, at 15:19, Thomas Munro wrote:
On Tue, Aug 10, 2021 at 5:43 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Aug 9, 2021 at 1:30 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
How common is to get a failure? I know I've run tests under
EXEC_BACKEND and not seen any failures. Not many runs though.On macOS, failures are extremely common. Sometimes I have to run
simple tests many times to get even one success. The proposal on the
table won't help with that problem since it's Linux-specific, but if
there's any way to do something similar on macOS it would be a _huge_
help.Yeah, make check always fails for me on macOS 11. With the attached
experimental hack, it fails only occasionally (1 in 8 runs or so). I
don't know why.
I suspect you'd need to use the hack in pg_ctl to make it reliable. The layout of normally stayed position independent postmaster can be incompatible with the non ASLR spawned child.
Andres
On Wed, Aug 11, 2021 at 2:12 AM Andres Freund <andres@anarazel.de> wrote:
On Tue, Aug 10, 2021, at 15:19, Thomas Munro wrote:
Yeah, make check always fails for me on macOS 11. With the attached
experimental hack, it fails only occasionally (1 in 8 runs or so). I
don't know why.I suspect you'd need to use the hack in pg_ctl to make it reliable. The layout of normally stayed position independent postmaster can be incompatible with the non ASLR spawned child.
Yeah, but the patch already changes both pg_ctl.c and postmaster.c.
On Wed, Aug 11, 2021 at 7:07 AM Thomas Munro <thomas.munro@gmail.com> wrote:
On Wed, Aug 11, 2021 at 2:12 AM Andres Freund <andres@anarazel.de> wrote:
On Tue, Aug 10, 2021, at 15:19, Thomas Munro wrote:
Yeah, make check always fails for me on macOS 11. With the attached
experimental hack, it fails only occasionally (1 in 8 runs or so). I
don't know why.I suspect you'd need to use the hack in pg_ctl to make it reliable. The layout of normally stayed position independent postmaster can be incompatible with the non ASLR spawned child.
Yeah, but the patch already changes both pg_ctl.c and postmaster.c.
/me stares at vmmap output for a while
Oooh. It's working perfectly (for example if you export
PATH=binarys:$PATH, pg_ctl -D pgdata start, make installcheck), but
pg_regress.c has its own separate fork/exec to launch the temporary
cluster that needs to be similarly hacked. Unfortunately I have to
give this Macintosh back and go and do some real work on a different
computer now. That does seem to be a working solution to the problem,
though, and could be polished into proposable form.
I saw claims that you can also link with -Wl,-no_pie or toggle the PIE
bit on your executable and libraries, but that didn't work for me on
11, Intel (no effect) or ARM (linker option gone).
On 8/10/21 7:59 PM, Thomas Munro wrote:
On Wed, Aug 11, 2021 at 7:07 AM Thomas Munro <thomas.munro@gmail.com> wrote:
On Wed, Aug 11, 2021 at 2:12 AM Andres Freund <andres@anarazel.de> wrote:
On Tue, Aug 10, 2021, at 15:19, Thomas Munro wrote:
Yeah, make check always fails for me on macOS 11. With the attached
experimental hack, it fails only occasionally (1 in 8 runs or so). I
don't know why.I suspect you'd need to use the hack in pg_ctl to make it reliable. The layout of normally stayed position independent postmaster can be incompatible with the non ASLR spawned child.
Yeah, but the patch already changes both pg_ctl.c and postmaster.c.
/me stares at vmmap output for a while
Oooh. It's working perfectly (for example if you export
PATH=binarys:$PATH, pg_ctl -D pgdata start, make installcheck), but
pg_regress.c has its own separate fork/exec to launch the temporary
cluster that needs to be similarly hacked.
initdb also runs postgres a bunch of times via system(), similarly to
pg_regress but without the "exec". Does it also need adjusting?
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On Tue, Aug 10, 2021 at 7:59 PM Thomas Munro <thomas.munro@gmail.com> wrote:
I saw claims that you can also link with -Wl,-no_pie or toggle the PIE
bit on your executable and libraries, but that didn't work for me on
11, Intel (no effect) or ARM (linker option gone).
I think that worked for me on older macOS releases, and then it
stopped working at some point after some update or reinstall or
something. Unfortunately I don't know any more precisely than that,
but it does seem like we have to find some other approach to work on
modern systems.
Hopefully they don't keep whacking this around...
--
Robert Haas
EDB: http://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
initdb also runs postgres a bunch of times via system(), similarly to
pg_regress but without the "exec". Does it also need adjusting?
That shouldn't be an issue, because we're only running the single
"standalone backend" process, not a cluster.
regards, tom lane
On Thu, Aug 12, 2021 at 1:45 AM Robert Haas <robertmhaas@gmail.com> wrote:
I think that worked for me on older macOS releases, and then it
stopped working at some point after some update or reinstall or
something. Unfortunately I don't know any more precisely than that,
but it does seem like we have to find some other approach to work on
modern systems.
I gave up on trying to make that work once I realised that
/usr/lib/dyld doesn't seem to obey the flag, so although other
segments become deterministic and the success rate is fairly high,
there's still a 600kb wrecking ball swinging around. I wondered what
the "slide" range could be... it appears to be fairly small
(vm_map_get_max_aslr_slide_pages() seems to be the place that's
determined and it's a 16MB or 256MB window, depending on architecture,
if I read that right). Given that, the death of 32 bit processes
since Catalina, and the typical layout we see, I think just doing
something like (/me rolls dice) export PG_SHMEM_ADDR=0x80000000000 is
a good candidate for something that works on both architectures, being
many TB away from everything else (above everything on ARM, between
heap etc and libs on Intel but with 8TB of space below it and 120TB
above). That gets the tests passing consistently with unpatched
master, -DEXEC_BACKEND, on both flavours of silicon.
On Wed, Aug 11, 2021 at 6:24 PM Thomas Munro <thomas.munro@gmail.com> wrote:
On Thu, Aug 12, 2021 at 1:45 AM Robert Haas <robertmhaas@gmail.com> wrote:
I think that worked for me on older macOS releases, and then it
stopped working at some point after some update or reinstall or
something. Unfortunately I don't know any more precisely than that,
but it does seem like we have to find some other approach to work on
modern systems.I gave up on trying to make that work once I realised that
/usr/lib/dyld doesn't seem to obey the flag, so although other
segments become deterministic and the success rate is fairly high,
there's still a 600kb wrecking ball swinging around. I wondered what
the "slide" range could be... it appears to be fairly small
(vm_map_get_max_aslr_slide_pages() seems to be the place that's
determined and it's a 16MB or 256MB window, depending on architecture,
if I read that right). Given that, the death of 32 bit processes
since Catalina, and the typical layout we see, I think just doing
something like (/me rolls dice) export PG_SHMEM_ADDR=0x80000000000 is
a good candidate for something that works on both architectures, being
many TB away from everything else (above everything on ARM, between
heap etc and libs on Intel but with 8TB of space below it and 120TB
above). That gets the tests passing consistently with unpatched
master, -DEXEC_BACKEND, on both flavours of silicon.
Ugh, OK. So, is there a way that we can get an "easy button" committed
to the tree?
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Aug 11, 2021 at 6:24 PM Thomas Munro <thomas.munro@gmail.com> wrote:
... I think just doing
something like (/me rolls dice) export PG_SHMEM_ADDR=0x80000000000 is
a good candidate for something that works on both architectures, being
many TB away from everything else (above everything on ARM, between
heap etc and libs on Intel but with 8TB of space below it and 120TB
above). That gets the tests passing consistently with unpatched
master, -DEXEC_BACKEND, on both flavours of silicon.
Ugh, OK. So, is there a way that we can get an "easy button" committed
to the tree?
I don't see why that approach couldn't be incorporated into pg_ctl,
or the postmaster itself. Given Andres' point that Linux ASLR
disable probably has to happen in pg_ctl, it seems like doing it
in pg_ctl in all cases is the way to move forward.
regards, tom lane
On Fri, Aug 13, 2021 at 3:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Ugh, OK. So, is there a way that we can get an "easy button" committed
to the tree?I don't see why that approach couldn't be incorporated into pg_ctl,
or the postmaster itself. Given Andres' point that Linux ASLR
disable probably has to happen in pg_ctl, it seems like doing it
in pg_ctl in all cases is the way to move forward.
I think doing it in the postmaster is best, since otherwise you have
to put code into pg_regress.c and pg_ctl.c. Here's a patch like that.
Attachments:
0001-Make-EXEC_BACKEND-more-convenient-on-macOS.patchapplication/octet-stream; name=0001-Make-EXEC_BACKEND-more-convenient-on-macOS.patchDownload
From a2cfc411be2571cf92facdfbe3cd00f0364e5826 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Fri, 13 Aug 2021 09:27:57 +1200
Subject: [PATCH] Make EXEC_BACKEND more convenient on macOS.
It's hard to disable ASLR on current macOS releases, when testing with
-DEXEC_BACKEND. Previously you could set the environment variable
PG_SHMEM_ADDR to something not likely to collide with other segments
that are mapped early in process startup. Let's also provide a default
value that works on current releases and architectures, for developer
convenience. It became a lot easier to pick a value when Apple killed
off 32 bit support.
As noted in the pre-existing comment, this is a questionable hack, but
-DEXEC_BACKEND is only used by Unix developers for testing some
otherwise Windows-only code paths, so this seems excusable.
Discussion: https://postgr.es/m/20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de
---
src/backend/port/sysv_shmem.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index 0cc83ffc16..8ecf9d2ab5 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -143,6 +143,16 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size)
if (pg_shmem_addr)
requestedAddress = (void *) strtoul(pg_shmem_addr, NULL, 0);
+#if defined(__darwin__) && SIZEOF_VOID_P == 8
+ else
+ {
+ /*
+ * Provide a default value that is known to work on current macOS
+ * releases.
+ */
+ requestedAddress = (void *) 0x80000000000;
+ }
+#endif
}
#endif
--
2.30.1 (Apple Git-130)
Thomas Munro <thomas.munro@gmail.com> writes:
On Fri, Aug 13, 2021 at 3:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't see why that approach couldn't be incorporated into pg_ctl,
or the postmaster itself. Given Andres' point that Linux ASLR
disable probably has to happen in pg_ctl, it seems like doing it
in pg_ctl in all cases is the way to move forward.
I think doing it in the postmaster is best, since otherwise you have
to put code into pg_regress.c and pg_ctl.c. Here's a patch like that.
Hmm, ok. Small thought: it might be better to put the #if inside
the "else { .... }". That way it scales easily to allow other
platform-specific defaults if we find anything useful. As-is,
the obvious extension would end up with multiple else-blocks,
which seems likely to confuse pgindent if nothing else.
regards, tom lane
On Fri, Aug 13, 2021 at 9:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hmm, ok. Small thought: it might be better to put the #if inside
the "else { .... }". That way it scales easily to allow other
platform-specific defaults if we find anything useful. As-is,
the obvious extension would end up with multiple else-blocks,
which seems likely to confuse pgindent if nothing else.
True. Thanks. Pushed to all live branches.
Obviously the address may have to be adjusted if Apple moves stuff
around, or if the initial layout and ASLR slide range turn out to be
less constrained than I inferred by nosing around the source code and
testing on a couple of systems.
Thomas Munro <thomas.munro@gmail.com> writes:
Obviously the address may have to be adjusted if Apple moves stuff
around, or if the initial layout and ASLR slide range turn out to be
less constrained than I inferred by nosing around the source code and
testing on a couple of systems.
Sure. But we're no worse off than before; the workaround of "set
PG_SHMEM_ADDR by hand" is just as applicable as ever.
regards, tom lane
Here's a patch for Linux and also FreeBSD. The latter OS decided to
turn on ASLR by default recently, causing my workstation to fail like
this quite reliably, which reminded me to follow up with this. It
disables ASLR in pg_ctl and pg_regress, which is enough for check and
check-world, but doesn't help you if you run the server directly
(unlike the different hack done for macOS).
For whatever random reason the failures are rarer on Linux (could be
my imagination, but I think they might be clustered, I didn't look
into the recipe for the randomness), but even without reproducing a
failure it's clear to see using pmap that this has the right effect.
I didn't bother with a check for the existence of ADDR_NO_RANDOMIZE
because it's since 2.6.12 which is definitely ancient enough.
Attachments:
0001-Make-EXEC_BACKEND-work-reliably-on-Linux-and-FreeBSD.patchtext/x-patch; charset=US-ASCII; name=0001-Make-EXEC_BACKEND-work-reliably-on-Linux-and-FreeBSD.patchDownload
From e615dc88c89142a1b7ecf7ba6f6f01ea59c61be5 Mon Sep 17 00:00:00 2001
From: Thomas Munro <tmunro@b1f14.hub>
Date: Wed, 17 Nov 2021 20:25:19 +1300
Subject: [PATCH] Make EXEC_BACKEND work reliably on Linux and FreeBSD.
Try to disable ASLR when building in EXEC_BACKEND mode (developer only),
to avoid random memory mapping failures.
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de
---
configure | 2 +-
configure.ac | 1 +
src/bin/pg_ctl/pg_ctl.c | 4 ++++
src/common/exec.c | 32 ++++++++++++++++++++++++++++++++
src/include/pg_config.h.in | 3 +++
src/include/port.h | 11 +++++++++++
src/test/regress/pg_regress.c | 4 ++++
7 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index ded80be880..9b7033d7dd 100755
--- a/configure
+++ b/configure
@@ -13427,7 +13427,7 @@ $as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
fi
-for ac_header in atomic.h copyfile.h execinfo.h getopt.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/event.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/uio.h sys/un.h termios.h ucred.h wctype.h
+for ac_header in atomic.h copyfile.h execinfo.h getopt.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/event.h sys/ipc.h sys/personality.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/uio.h sys/un.h termios.h ucred.h wctype.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
diff --git a/configure.ac b/configure.ac
index 775e5c4436..9602b04e36 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1376,6 +1376,7 @@ AC_CHECK_HEADERS(m4_normalize([
sys/epoll.h
sys/event.h
sys/ipc.h
+ sys/personality.h
sys/prctl.h
sys/procctl.h
sys/pstat.h
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7fbbe7022e..eadb5c2297 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -451,6 +451,10 @@ start_postmaster(void)
fflush(stdout);
fflush(stderr);
+#ifdef EXEC_BACKEND
+ pg_disable_aslr();
+#endif
+
pm_pid = fork();
if (pm_pid < 0)
{
diff --git a/src/common/exec.c b/src/common/exec.c
index 81b810d4cf..e5d9174333 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -25,6 +25,14 @@
#include <sys/wait.h>
#include <unistd.h>
+#ifdef EXEC_BACKEND
+#if defined(HAVE_SYS_PERSONALITY_H)
+#include <sys/personality.h>
+#elif defined(HAVE_SYS_PROCCTL_H)
+#include <sys/procctl.h>
+#endif
+#endif
+
/*
* Hacky solution to allow expressing both frontend and backend error reports
* in one macro call. First argument of log_error is an errcode() call of
@@ -470,6 +478,30 @@ set_pglocale_pgservice(const char *argv0, const char *app)
}
}
+#ifdef EXEC_BACKEND
+/*
+ * For the benefit of PostgreSQL developers testing EXEC_BACKEND code paths on
+ * Unix systems (ie paths normally exercised only on Windows), provide a way to
+ * disable ASLR so that we avoid memory map in developer-only builds, if we
+ * know how on this platform. (See also the macOS-specific hack in
+ * sysv_shmem.c.)
+ */
+int
+pg_disable_aslr(void)
+{
+#if defined(HAVE_SYS_PERSONALITY_H)
+ return personality(ADDR_NO_RANDOMIZE);
+#elif defined(HAVE_SYS_PROCCTL_H) && defined(PROC_ASLR_FORCE_DISABLE)
+ int data = PROC_ASLR_FORCE_DISABLE;
+
+ return procctl(P_PID, 0, PROC_ASLR_CTL, &data);
+#else
+ errno = ENOSYS;
+ return -1;
+#endif
+}
+#endif
+
#ifdef WIN32
/*
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index ca3592465e..a3d658644a 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -617,6 +617,9 @@
/* Define to 1 if you have the <sys/ipc.h> header file. */
#undef HAVE_SYS_IPC_H
+/* Define to 1 if you have the <sys/personality.h> header file. */
+#undef HAVE_SYS_PERSONALITY_H
+
/* Define to 1 if you have the <sys/prctl.h> header file. */
#undef HAVE_SYS_PRCTL_H
diff --git a/src/include/port.h b/src/include/port.h
index 49b4d38131..94b0f23837 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -42,6 +42,12 @@ typedef SOCKET pgsocket;
typedef unsigned int socklen_t;
#endif
+#ifdef EXEC_BACKEND
+#if defined(HAVE_SYS_PROCCTL_H)
+#include <sys/procctl.h>
+#endif
+#endif
+
/* non-blocking */
extern bool pg_set_noblock(pgsocket sock);
extern bool pg_set_block(pgsocket sock);
@@ -140,6 +146,11 @@ extern char *pipe_read_line(char *cmd, char *line, int maxsize);
/* Doesn't belong here, but this is used with find_other_exec(), so... */
#define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n"
+#ifdef EXEC_BACKEND
+/* Disable ASLR before exec, for developer builds only (in exec.c) */
+extern int pg_disable_aslr(void);
+#endif
+
#if defined(WIN32) || defined(__CYGWIN__)
#define EXE ".exe"
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 2c8a600bad..67c78bccf2 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1246,6 +1246,10 @@ spawn_process(const char *cmdline)
if (logfile)
fflush(logfile);
+#ifdef EXEC_BACKEND
+ pg_disable_aslr();
+#endif
+
pid = fork();
if (pid == -1)
{
--
2.33.1
On 11/23/21, 11:29 AM, "Thomas Munro" <thomas.munro@gmail.com> wrote:
Here's a patch for Linux and also FreeBSD. The latter OS decided to
turn on ASLR by default recently, causing my workstation to fail like
this quite reliably, which reminded me to follow up with this. It
disables ASLR in pg_ctl and pg_regress, which is enough for check and
check-world, but doesn't help you if you run the server directly
(unlike the different hack done for macOS).For whatever random reason the failures are rarer on Linux (could be
my imagination, but I think they might be clustered, I didn't look
into the recipe for the randomness), but even without reproducing a
failure it's clear to see using pmap that this has the right effect.
I didn't bother with a check for the existence of ADDR_NO_RANDOMIZE
because it's since 2.6.12 which is definitely ancient enough.
FWIW I just found this patch very useful for testing some EXEC_BACKEND
stuff on Linux.
Nathan
On Sat, Jan 8, 2022 at 9:20 AM Bossart, Nathan <bossartn@amazon.com> wrote:
FWIW I just found this patch very useful for testing some EXEC_BACKEND
stuff on Linux.
Thanks for testing. Tidied and pushed, to master only for now.
On Tue, Jan 11, 2022 at 12:29:44AM +1300, Thomas Munro wrote:
Thanks for testing. Tidied and pushed, to master only for now.
I have noticed the following failure for v11~14 on one of my hosts
that compiles with -DEXEC_BACKEND, and Nathan has redirected me here:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gokiburi&dt=2023-01-31%2012%3A07%3A32
FATAL: could not reattach to shared memory (key=1050468, addr=0xffff97eb2000): Invalid argument
Could it be worth back-patching f3e7806? I don't mind changing this
animal setup by switching the kernel configuration or reducing the
branch scope, but this solution is less invasive because it would not
influence parallel runs.
Thoughts?
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
Could it be worth back-patching f3e7806?
That's aged long enough now that it seems like a pretty safe
thing to do.
regards, tom lane
On Tue, Jan 31, 2023 at 08:37:29PM -0500, Tom Lane wrote:
That's aged long enough now that it seems like a pretty safe
thing to do.
Thanks. I'll wait for a few days before doing something for my
buildfarm stuff, in case somebody thinks this is a bad idea..
--
Michael
On Wed, Feb 1, 2023 at 2:58 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jan 31, 2023 at 08:37:29PM -0500, Tom Lane wrote:
That's aged long enough now that it seems like a pretty safe
thing to do.Thanks. I'll wait for a few days before doing something for my
buildfarm stuff, in case somebody thinks this is a bad idea..
+1, go for it. It shouldn't affect Unix build releases, and on
Windows the function does nothing.
On Wed, Feb 01, 2023 at 03:06:16PM +1300, Thomas Munro wrote:
+1, go for it. It shouldn't affect Unix build releases, and on
Windows the function does nothing.
Thanks for confirming. I am wondering what these animals may complain
about next, but based on some tests on this buildfarm host with the
same configuration, things are looking OK once this stuff is applied
on 11~14.
--
Michael
On Thu, Feb 02, 2023 at 10:06:15AM +0900, Michael Paquier wrote:
Thanks for confirming. I am wondering what these animals may complain
about next, but based on some tests on this buildfarm host with the
same configuration, things are looking OK once this stuff is applied
on 11~14.
Actually, I completely forgot to take into account that there is a
minor release planned for next week:
https://www.postgresql.org/developer/roadmap/
So I'll hold on a bit longer here, until the next versions get their
tags.
--
Michael
On Thu, Feb 02, 2023 at 11:14:39AM +0900, Michael Paquier wrote:
Actually, I completely forgot to take into account that there is a
minor release planned for next week:
https://www.postgresql.org/developer/roadmap/So I'll hold on a bit longer here, until the next versions get their
tags.
So, the backpatch down to 12 is straight-forward, with some conflicts
in ./configure. ~12 handles also differently its CFLAGS with
pg_config.h.win32. 11 is more annoying because it lacks
HAVE_SYS_PROCCTL_H and it would need a partial backport of f98b847. I
am not completely sure if this could have more side effects, though,
even if the part of pmsignal.c is left out. It should not..
At the end, I have just done this stuff down to ~12, 11 does not seem
worth the trouble as the next stable version to go out of support.
I'll reduce gokiburi's script a bit, as a result, until the oldest
version support is v12.
--
Michael
On 2023-02-08 13:12:15 +0900, Michael Paquier wrote:
On Thu, Feb 02, 2023 at 11:14:39AM +0900, Michael Paquier wrote:
Actually, I completely forgot to take into account that there is a
minor release planned for next week:
https://www.postgresql.org/developer/roadmap/So I'll hold on a bit longer here, until the next versions get their
tags.So, the backpatch down to 12 is straight-forward, with some conflicts
in ./configure. ~12 handles also differently its CFLAGS with
pg_config.h.win32. 11 is more annoying because it lacks
HAVE_SYS_PROCCTL_H and it would need a partial backport of f98b847. I
am not completely sure if this could have more side effects, though,
even if the part of pmsignal.c is left out. It should not..At the end, I have just done this stuff down to ~12, 11 does not seem
worth the trouble as the next stable version to go out of support.
I'll reduce gokiburi's script a bit, as a result, until the oldest
version support is v12.
That seems reasonable to me.
On Wed, Feb 8, 2023 at 5:12 PM Michael Paquier <michael@paquier.xyz> wrote:
At the end, I have just done this stuff down to ~12, 11 does not seem
worth the trouble as the next stable version to go out of support.
I'll reduce gokiburi's script a bit, as a result, until the oldest
version support is v12.
For the record, according to [1]/messages/by-id/1347459696.16215.11.camel@vanquo.pezone.net it's not necessary to use
--reset-author when back-patching. (Maybe a little confusingly,
because it's not quite clear whether our policies consider the author
field to be meaningful or not.)
[1]: /messages/by-id/1347459696.16215.11.camel@vanquo.pezone.net
On Fri, Feb 10, 2023 at 08:15:21AM +1300, Thomas Munro wrote:
For the record, according to [1] it's not necessary to use
--reset-author when back-patching. (Maybe a little confusingly,
because it's not quite clear whether our policies consider the author
field to be meaningful or not.)[1] /messages/by-id/1347459696.16215.11.camel@vanquo.pezone.net
Oops, sorry about that. Using --reset-author is a habit when it comes
to backpatch. It looks like my mistake when back-patching something
only to stable branches.
--
Michael