Skip to content

Commit

Permalink
Fix pgrx install causing postgresql coredump (#1263)
Browse files Browse the repository at this point in the history
this is because pgrx will overwrite the .so file in place.

dlopen will use MAP_PRIVATE to open the file and map a read-only memory
using mmap(2). usually this memory is copy-on-write. if others are
modifying the file, the previously mapped memory should not change.

but there is an undefined behavior, from `man mmap(2)`:

> MAP_PRIVATE: It is unspecified whether changes made to the file after the mmap()
> call are visible in the mapped region.

what actually happens is that the read-only memory in postgresql is
modified, and all pointers in the .text segment can get mashed up.
the call stack looks like

```
 0x0000000000731510 in ?? ()
 0x00007f7a94515132 in core::sync::atomic::AtomicUsize::store (self=0x7f7a95110318 <pgrx_pg_sys::submodules::thread_check::ACTIVE_THREAD::h60448dcb81097e92>, val=0, order=core::sync::atomic::Ordering::Relaxed) at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/sync/atomic.rs:2291
 0x00007f7a944f574b in pgrx_pg_sys::submodules::thread_check::init_active_thread::clear_in_child () at src/submodules/thread_check.rs:39
 0x00007f7a962f8a88 in __run_postfork_handlers (who=who@entry=atfork_run_child, do_locking=do_locking@entry=false, lastrun=lastrun@entry=2) at register-atfork.c:187
 0x00007f7a962df773 in __libc_fork () at fork.c:109
 0x00005555e66ad948 in fork_process () at fork_process.c:61
 0x00005555e66a5d48 in StartAutoVacWorker () at autovacuum.c:1543
 0x00005555e66c43f7 in StartAutovacuumWorker () at postmaster.c:6155
 0x00005555e66c3d21 in sigusr1_handler (postgres_signal_arg=10) at postmaster.c:5820
 <signal handler called>
 0x00007f7a9630eb84 in __GI___select (nfds=6, readfds=0x7ffc61c2fa40, writefds=0x0, exceptfds=0x0, timeout=0x7ffc61c2f9b0) at ../sysdeps/unix/sysv/linux/select.c:69
 0x00005555e66be343 in ServerLoop () at postmaster.c:1950
 0x00005555e66bdb0f in PostmasterMain (argc=5, argv=0x5555e8fb5490) at postmaster.c:1631
 0x00005555e6560e41 in main (argc=5, argv=0x5555e8fb5490) at main.c:240
```

this is `pgrx_pg_sys` trying to run the postfork hook, but the variable
`ACTIVE_THREAD` and the code binary will mismatch.
  • Loading branch information
Sasasu authored Aug 23, 2023
1 parent 6bd46c9 commit f659f69
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions cargo-pgrx/src/command/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,19 @@ pub(crate) fn install_extension(
};
dest.push(format!("{}.so", so_name));

if cfg!(target_os = "macos") {
// Remove the existing .so if present. This is a workaround for an
// issue highlighted by the following apple documentation:
// https://developer.apple.com/documentation/security/updating_mac_software
if dest.exists() {
std::fs::remove_file(&dest).wrap_err_with(|| {
format!("unable to remove existing file {}", dest.display())
})?;
}
// Remove the existing .so if present. This is a workaround for an
// issue highlighted by the following apple documentation:
// https://developer.apple.com/documentation/security/updating_mac_software
//
// for Linux, dlopen(2) will use mmap to load the .so.
// if update the file in place, the modification will pass into the running
// process which will mash up all pointers in the .TEXT segment.
// this simulate linux's install(1) behavior
if dest.exists() {
std::fs::remove_file(&dest)
.wrap_err_with(|| format!("unable to remove existing file {}", dest.display()))?;
}

copy_file(&shlibpath, &dest, "shared library", false, &package_manifest_path)?;
}

Expand Down

0 comments on commit f659f69

Please sign in to comment.