Skip to content

Commit 0ac3a3f

Browse files
committed
Fix default value for DefaultLimitCORE
1 parent 3b62d09 commit 0ac3a3f

5 files changed

Lines changed: 130 additions & 25 deletions

File tree

‎ansible/files/coredump/process-orioledb-coredumps.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ process_one() {
169169
local tmpdir
170170
tmpdir=$(mktemp -d /tmp/coredump-XXXXXX)
171171
chmod 700 "$tmpdir"
172-
# suppress shellcheck warning about quoting $tmpdir in the trap command -
172+
# suppress shellcheck warning about quoting $tmpdir in the trap command -
173173
# it's correct to quote it, and the trap is evaluated at runtime,
174174
# not parse time.
175175
# shellcheck disable=SC2064
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[Manager]
2+
DefaultLimitCORE=0

‎ansible/tasks/setup-postgres.yml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,33 @@
331331
src: 'files/postgresql_config/coredump.conf'
332332
become: true
333333

334+
# This AMI's base image ships systemd's DefaultLimitCORE=infinity - without
335+
# an explicit override, *every* service that doesn't set its own LimitCORE=
336+
# already gets unlimited-size core dumps, not just postgres (confirmed via
337+
# `systemctl show -p DefaultLimitCORE` on a real instance). This is the
338+
# other half of "only PostgreSQL permitted to generate cores" - postgres
339+
# gets LimitCORE=infinity above, everyone else gets 0 here. [Manager]
340+
# section changes only take effect on the next full boot (or a manual
341+
# `systemctl daemon-reexec`), which is fine for a freshly-imaged AMI but
342+
# means re-testing this on an already-running instance needs a reexec.
343+
- name: Create a systemd system.conf.d dir for the machine-wide coredump default
344+
ansible.builtin.file:
345+
group: 'root'
346+
mode: '0755'
347+
owner: 'root'
348+
path: '/etc/systemd/system.conf.d'
349+
state: 'directory'
350+
become: true
351+
352+
- name: copy machine-wide default core limit (postgres is the only exception)
353+
ansible.builtin.copy:
354+
dest: '/etc/systemd/system.conf.d/disable-coredumps-by-default.conf'
355+
group: 'root'
356+
mode: '0644'
357+
owner: 'root'
358+
src: 'files/postgresql_config/disable-coredumps-by-default.conf'
359+
become: true
360+
334361
- name: Create a systemd-coredump conf.d dir for PostgreSQL storage limits
335362
ansible.builtin.file:
336363
group: 'root'

‎nix/packages/postgres-env.nix‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,13 @@
2020
self'.packages."postgresql_${version}_src"
2121
]
2222
++ lib.optionals pkgs.stdenv.isLinux [ self'.packages."postgresql_${version}_debug" ]
23-
++ lib.optionals (pkgs.stdenv.isLinux && version != "15") [ self'.packages.gatekeeper ];
23+
++ lib.optionals (pkgs.stdenv.isLinux && version != "15") [ self'.packages.gatekeeper ]
24+
# orioledb.so ships as a separate extension package (nix/ext/orioledb.nix), not
25+
# part of the postgresql derivation itself, so its debug output isn't covered by
26+
# postgresql_${version}_debug above and has to be pulled in explicitly.
27+
++ lib.optionals (pkgs.stdenv.isLinux && version == "orioledb-17") [
28+
self'.legacyPackages."psql_${version}".exts.orioledb.debug
29+
];
2430
};
2531
in
2632
{

‎testinfra/test_ami_nix.py‎

Lines changed: 93 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,20 +1473,46 @@ def test_postgres_coredump_filter_excludes_shared_buffers(host):
14731473
)
14741474

14751475

1476+
def test_default_core_limit_is_disabled_machine_wide(host):
1477+
"""Verify DefaultLimitCORE=0 is configured, so postgresql.service's own
1478+
LimitCORE=infinity is the *only* exception, not one of many.
1479+
1480+
This AMI's base image ships systemd's own DefaultLimitCORE=infinity -
1481+
without an explicit override here, every other service that doesn't set
1482+
its own LimitCORE= would also get unlimited-size core dumps, not just
1483+
postgres. This is the other half of "only PostgreSQL permitted to
1484+
generate cores" (postgres gets LimitCORE=infinity, everyone else gets 0
1485+
via this machine-wide default).
1486+
"""
1487+
result = run_ssh_command(host["ssh"], "systemctl show -p DefaultLimitCORE")
1488+
assert result["succeeded"], f"systemctl show failed: {result['stderr']}"
1489+
assert "DefaultLimitCORE=0" in result["stdout"], (
1490+
f"Expected DefaultLimitCORE=0 machine-wide, got:\n{result['stdout']}"
1491+
)
1492+
1493+
14761494
def test_coredump_storage_directory_root_only(host):
1477-
"""Verify /var/lib/systemd/coredump is root-only, since it can hold
1478-
core files containing customer data."""
1495+
"""Verify /var/lib/systemd/coredump is owned by root and not writable by
1496+
anyone else. Systemd's own default for this directory is 0755 (world
1497+
*readable*, confirmed on a real AMI - only 700/750 were checked here
1498+
originally, which was an unverified guess, not systemd's actual
1499+
behavior) - a world-readable directory only leaks core *filenames*, not
1500+
content, since the actual core files get their own restrictive
1501+
permissions from systemd-coredump. What actually matters is that no one
1502+
but root can create/replace/delete files in it."""
14791503
result = run_ssh_command(
14801504
host["ssh"], "stat -c '%a %U:%G' /var/lib/systemd/coredump"
14811505
)
14821506
assert result["succeeded"], f"stat failed: {result['stderr']}"
14831507
mode, owner = result["stdout"].strip().split()
1484-
assert mode in ("700", "750"), (
1485-
f"Expected /var/lib/systemd/coredump to be root-only, got mode {mode}"
1486-
)
14871508
assert owner.startswith("root:"), (
14881509
f"Expected /var/lib/systemd/coredump to be owned by root, got {owner}"
14891510
)
1511+
group_other_bits = mode[-2:]
1512+
assert all(int(bit) & 0o2 == 0 for bit in group_other_bits), (
1513+
f"Expected /var/lib/systemd/coredump to not be group/other-writable, "
1514+
f"got mode {mode}"
1515+
)
14901516

14911517

14921518
def test_postgres_prestart_does_not_reset_core_limit(host):
@@ -1507,11 +1533,28 @@ def _crash_a_backend_and_wait_for_recovery(host):
15071533
"""Grab a real Postgres backend pid, SIGSEGV it, and wait for PostgreSQL's
15081534
normal crash-recovery to bring the instance back on its own
15091535
(Restart=always / auto-reinit, same as production). Returns the crashed
1510-
backend's pid. Used by tests that need a real, fresh core to appear."""
1536+
backend's pid. Used by tests that need a real, fresh core to appear.
1537+
1538+
A single one-shot `select pg_backend_pid()` query is not safe here: the
1539+
psql client disconnects the instant it gets its answer, which makes the
1540+
server-side backend exit normally right after - so by the time `kill`
1541+
runs, that pid is usually already gone (confirmed in practice: this used
1542+
to silently signal a dead/nonexistent pid, producing no crash and no
1543+
core at all). Instead, start a backend that's still busy (via
1544+
pg_sleep), and look its pid up out-of-band via pg_stat_activity so it's
1545+
guaranteed to still be alive when we signal it.
1546+
"""
1547+
run_ssh_command(
1548+
host["ssh"],
1549+
"sudo -u postgres psql -U supabase_admin -h localhost -d postgres "
1550+
"-c 'select pg_sleep(30);' >/dev/null 2>&1 &",
1551+
)
1552+
sleep(1)
15111553
backend_pid = run_ssh_command(
15121554
host["ssh"],
15131555
"sudo -u postgres psql -U supabase_admin -h localhost -d postgres "
1514-
"-tAc 'select pg_backend_pid()'",
1556+
"-tAc \"select pid from pg_stat_activity where query = 'select pg_sleep(30);' "
1557+
"and state = 'active' limit 1;\"",
15151558
)["stdout"].strip()
15161559
assert backend_pid.isdigit(), (
15171560
f"Could not resolve a Postgres backend pid: {backend_pid}"
@@ -1551,21 +1594,20 @@ def test_postgres_backend_crash_produces_core_but_unrelated_process_does_not(hos
15511594
)
15521595
before_lines = set(before["stdout"].splitlines())
15531596

1554-
# Unrelated process: launch a disposable process outside the
1555-
# LimitCORE=infinity-scoped postgresql.service and SIGSEGV it - it must
1556-
# not produce a core, since the default core limit is unchanged for it.
1597+
# run a systemd process that isn't part of postgresql.service and check
1598+
# that it keeps the default core limit (thus not producing a coredump)
15571599
run_ssh_command(
15581600
host["ssh"],
1559-
"setsid bash -c 'sleep 60 & echo $! > /tmp/unrelated_pid; wait' >/dev/null 2>&1 &",
1601+
"sudo systemd-run --unit=testinfra-unrelated-crash --collect /bin/sleep 60",
15601602
)
15611603
sleep(1)
1562-
unrelated_pid = run_ssh_command(host["ssh"], "cat /tmp/unrelated_pid")[
1563-
"stdout"
1564-
].strip()
1565-
assert unrelated_pid.isdigit(), (
1566-
f"Could not resolve the disposable unrelated process pid: {unrelated_pid}"
1604+
unrelated_pid = run_ssh_command(
1605+
host["ssh"], "systemctl show testinfra-unrelated-crash -p MainPID --value"
1606+
)["stdout"].strip()
1607+
assert unrelated_pid.isdigit() and unrelated_pid != "0", (
1608+
f"Could not resolve the disposable unrelated unit's pid: {unrelated_pid}"
15671609
)
1568-
run_ssh_command(host["ssh"], f"kill -SEGV {unrelated_pid}")
1610+
run_ssh_command(host["ssh"], f"sudo kill -SEGV {unrelated_pid}")
15691611
sleep(2)
15701612

15711613
# Postgres backend: grab a real backend pid and crash it the same way.
@@ -1587,6 +1629,32 @@ def test_postgres_backend_crash_produces_core_but_unrelated_process_does_not(hos
15871629
)
15881630

15891631

1632+
def _resolve_postgres_binary(host):
1633+
"""/usr/lib/postgresql/bin/postgres is a Nix wrapper *script* (sets
1634+
NIX_PGLIBDIR, then execs the real ELF elsewhere in the nix store) - not
1635+
an executable itself, so readelf/gdb can't be pointed at it directly.
1636+
Resolve the real binary via the live postmaster's /proc/<pid>/exe,
1637+
which always shows the actual running ELF regardless of the wrapper -
1638+
the same real path a crash would report via coredumpctl."""
1639+
pid = run_ssh_command(
1640+
host["ssh"], "systemctl show postgresql -p MainPID --value"
1641+
)["stdout"].strip()
1642+
return run_ssh_command(host["ssh"], f"sudo readlink -f /proc/{pid}/exe")[
1643+
"stdout"
1644+
].strip()
1645+
1646+
1647+
def _resolve_orioledb_lib(host, postgres_binary):
1648+
"""orioledb.so has no fixed path either - it lives in the same nix store
1649+
derivation as the resolved postgres binary, just under lib/ instead of
1650+
bin/ (see orioledb_lib_path() in process-orioledb-coredumps.sh, which
1651+
this mirrors)."""
1652+
exe_dir = run_ssh_command(
1653+
host["ssh"], f"dirname \"$(dirname '{postgres_binary}')\""
1654+
)["stdout"].strip()
1655+
return f"{exe_dir}/lib/orioledb.so"
1656+
1657+
15901658
def _build_id_of(host, path):
15911659
"""Return the ELF build-id (hex string) of the binary/library at path, or None."""
15921660
import re
@@ -1613,8 +1681,9 @@ def test_postgres_binary_build_id_matches_shipped_debug_symbols(host):
16131681
"""Verify the installed 'postgres' binary's build-id has a matching
16141682
.build-id/xx/yyyy.debug file in the postgres-env debug output, so a
16151683
future coredump-processing GDB session can actually resolve symbols."""
1616-
build_id = _build_id_of(host, "/usr/lib/postgresql/bin/postgres")
1617-
assert build_id, "Could not read a build-id from /usr/lib/postgresql/bin/postgres"
1684+
postgres_binary = _resolve_postgres_binary(host)
1685+
build_id = _build_id_of(host, postgres_binary)
1686+
assert build_id, f"Could not read a build-id from {postgres_binary}"
16181687
assert _debug_file_exists_for_build_id(host, build_id), (
16191688
f"No debug file found under /var/lib/postgresql/.nix-profile/lib/debug/"
16201689
f".build-id/ matching postgres build-id {build_id} - the shipped "
@@ -1626,7 +1695,7 @@ def test_orioledb_library_build_id_matches_shipped_debug_symbols(host):
16261695
"""Verify orioledb.so's build-id has a matching debug file, same as for
16271696
the postgres binary - orioledb.so is built by the same derivation
16281697
(isOrioleDB flavor) so its debug info ships in the same _debug output."""
1629-
orioledb_so = "/usr/lib/postgresql/lib/orioledb.so"
1698+
orioledb_so = _resolve_orioledb_lib(host, _resolve_postgres_binary(host))
16301699
exists = run_ssh_command(host["ssh"], f"test -f {orioledb_so} && echo present")
16311700
if "present" not in exists["stdout"]:
16321701
pytest.skip("orioledb.so not present on this AMI (not an OrioleDB build)")
@@ -1657,8 +1726,9 @@ def test_gdb_resolves_postgres_source_via_shipped_src_package(host):
16571726
"""
16581727
import re
16591728

1660-
build_id = _build_id_of(host, "/usr/lib/postgresql/bin/postgres")
1661-
assert build_id, "Could not read a build-id from /usr/lib/postgresql/bin/postgres"
1729+
postgres_binary = _resolve_postgres_binary(host)
1730+
build_id = _build_id_of(host, postgres_binary)
1731+
assert build_id, f"Could not read a build-id from {postgres_binary}"
16621732
prefix, rest = build_id[:2], build_id[2:]
16631733
debug_file = f"/var/lib/postgresql/.nix-profile/lib/debug/.build-id/{prefix}/{rest}.debug"
16641734

@@ -1674,7 +1744,7 @@ def test_gdb_resolves_postgres_source_via_shipped_src_package(host):
16741744
"sudo -u postgres gdb --batch -quiet "
16751745
"-ex 'set debug-file-directory /var/lib/postgresql/.nix-profile/lib/debug' "
16761746
f"-ex 'set substitute-path {comp_dir} /var/lib/postgresql/.nix-profile' "
1677-
"-ex 'file /usr/lib/postgresql/bin/postgres' "
1747+
f"-ex 'file {postgres_binary}' "
16781748
"-ex 'list main' "
16791749
"2>&1",
16801750
)

0 commit comments

Comments
 (0)