Regression tests fail with musl libc because libpq.so can't be loaded

Wolfgang Walther <walther@technowledgy.de> writes:

- LD_LIBRARY_PATH is used correctly to look for the direct dependency
loaded in dlopen(...), but is not taken into account anymore when trying
to locate libpq.so. This step only fails with musl, but works fine with
glibc.

Why do you think this is our bug and not musl's? We do not even have
any code that knows anything about indirect library dependencies.

regards, tom lane

Tom Lane:

Why do you think this is our bug and not musl's?

Because I tried to reproduce with musl directly with a very simple
example as mentioned in:

To rule out a musl bug, I also put together a very simple test-case of an executable loading liba with dlopen(), which depends on libb and then constructing the same scenario with LD_LIBRARY_PATH. This works fine when compiled with glibc and musl, too. Thus, I believe the problem to be somewhere in how postgres loads those libraries.

My test case looked like the attached. To compile it with musl via
Dockerfile:

docker build . -t musl-dlopen && docker run --rm musl-dlopen

a.c/a.h is equivalent to libpqwalreceiver and b.c/b.h to libpq.

This works fine with both musl and glibc.

(Note: I also tried putting liba.so and libb.so in different folders,
adding both to LD_LIBRARY_PATH - but that worked fine as well.)

Now my very simple example probably does something different than
postgres, so that the problem doesn't appear there. But since it seems
possible to do this with musl in principle, it should be possible to do
it differently in postgres to make it work, too.

Any ideas?

Best,
Wolfgang

On Sat, Mar 16, 2024 at 11:55 AM Wolfgang Walther <walther@technowledgy.de>
wrote:

Tom Lane:

Why do you think this is our bug and not musl's?

Because I tried to reproduce with musl directly with a very simple
example as mentioned in:

To rule out a musl bug, I also put together a very simple test-case of

an executable loading liba with dlopen(), which depends on libb and then
constructing the same scenario with LD_LIBRARY_PATH. This works fine when
compiled with glibc and musl, too. Thus, I believe the problem to be
somewhere in how postgres loads those libraries.

My test case looked like the attached. To compile it with musl via
Dockerfile:

docker build . -t musl-dlopen && docker run --rm musl-dlopen

a.c/a.h is equivalent to libpqwalreceiver and b.c/b.h to libpq.

This works fine with both musl and glibc.

(Note: I also tried putting liba.so and libb.so in different folders,
adding both to LD_LIBRARY_PATH - but that worked fine as well.)

Now my very simple example probably does something different than
postgres, so that the problem doesn't appear there. But since it seems
possible to do this with musl in principle, it should be possible to do
it differently in postgres to make it work, too.

Any ideas?

On Alpine Linux, which uses musl libc, you have to run `make install`
before you can run `make check`. Have you tried
that?

(Note to self: need a new Alpine buildfarm member)

cheers

andrew

Andrew Dunstan:

On Alpine Linux, which uses musl libc, you have to run `make install`
before you can run `make check`. Have you tried that?

I can see how that could work around the problem, because the library
would already be available in the default library path / rpath and
LD_LIBRARY_PATH would not be needed.

However, this would only be a workaround for the specific case of
running the regression tests, not a solution. Using LD_LIBRARY_PATH, as
documented, would still not be possible.

In my case, I am just using docker images with Alpine to easily
reproduce the problem. I am building with NixOS / nixpkgs' pkgsMusl. The
order of check and install phases can't be changed there, AFAICT. The
workaround I use right now is to temporarily patch rpath of the postgres
binary - this will be reset during the install phase anyway. This works,
but again is not a real solution.

Best,

Wolfgang

Andrew Dunstan <andrew@dunslane.net> writes:

On Alpine Linux, which uses musl libc, you have to run `make install`
before you can run `make check`. Have you tried that?

We have the same situation on macOS. There, it seems to be the result
of a "security feature" that strips DYLD_LIBRARY_PATH from the process
environment when make executes a shell. There's not much we can do
about that, and I suspect there is not much we can do about musl's
behavior either. (I am not a fan of proposals to modify the binaries
under test, because then you are not testing what you intend to
install.)

regards, tom lane

On Sun, Mar 17, 2024 at 4:56 AM Wolfgang Walther
<walther@technowledgy.de> wrote:

Any ideas?

I'd look into whether there is a difference in the rules it uses for
deciding not to trust LD_LIBRARY_PATH, which seems to be around here
somewhere:

https://github.com/bminor/musl/blob/7ada6dde6f9dc6a2836c3d92c2f762d35fd229e0/ldso/dynlink.c#L1812

I wonder if you can break into an affected program and check out the
magic there. FWIW on MacOS something equivalent happens at the moment
we execute a shell, because the system shell is 'code signed' and that
OS treats signed stuff similar to setuid binaries for this purpose
(IIRC setting SHELL to point to a suitable unsigned shell could work
around the problem there?)

Another interesting thing that came up when I googled musl/glibc
differences -- old but looks plausibly still true (not that I expect
our code to be modifying that stuff in place, just something to
check):

https://www.openwall.com/lists/musl/2014/08/31/14

Tom Lane:

We have the same situation on macOS. There, it seems to be the result
of a "security feature" that strips DYLD_LIBRARY_PATH from the process
environment when make executes a shell.

I'm not sure whether this explanation is sufficient for the musl case,
because LD_LIBRARY_PATH does make a difference: The direct dependency
(libpqwalreceiver.so) can still be found if it's moved elsewhere and
LD_LIBRARY_PATH points at it. So clearly the LD_LIBRARY_PATH variable is
still set after make executed the shell - it's just not in effect on the
*indirect* dependency (libpq.so) anymore.

Best,

Wolfgang

Thomas Munro:

I'd look into whether there is a difference in the rules it uses for
deciding not to trust LD_LIBRARY_PATH, which seems to be around here
somewhere:

https://github.com/bminor/musl/blob/7ada6dde6f9dc6a2836c3d92c2f762d35fd229e0/ldso/dynlink.c#L1812

Yeah, I have been looking at that, too. I had also experimented a bit
with setuid/setgid for that matter, but that didn't lead anywhere, yet.
I'm not 100% sure, but I think this would also not match my other
observation, that LD_LIBRARY_PATH does work for libpqwalreceiver (direct
dep), but not libpq (indirect dep).

Another interesting thing that came up when I googled musl/glibc
differences -- old but looks plausibly still true (not that I expect
our code to be modifying that stuff in place, just something to
check):

https://www.openwall.com/lists/musl/2014/08/31/14

To me, this seems very much like what could happen - it matches all my
observations, so far. But I can't tell how likely that is, not knowing
much of the postgres code.

Best,

Wolfgang

On Sun, Mar 17, 2024 at 9:19 AM Thomas Munro <thomas.munro@gmail.com> wrote:

Another interesting thing that came up when I googled musl/glibc
differences -- old but looks plausibly still true (not that I expect
our code to be modifying that stuff in place, just something to
check):

https://www.openwall.com/lists/musl/2014/08/31/14

Hmm, that does mention setproctitle, and our ps_status.c does indeed
clobber some stuff in that region (in fact our ps_status.c is likely
derived from the setproctitle() function from sendmail AFAICT). But
that's in our "backend" server processes, unlike the problems we have
on Macs... oh but you're failing to load libpqwalreceiver.so which
makes some sense for the backend hypothesis. What happens if you hack
ps_status.c to use PS_USE_NONE?

Thomas Munro:

Hmm, that does mention setproctitle, and our ps_status.c does indeed
clobber some stuff in that region (in fact our ps_status.c is likely
derived from the setproctitle() function from sendmail AFAICT). But
that's in our "backend" server processes, unlike the problems we have
on Macs... oh but you're failing to load libpqwalreceiver.so which
makes some sense for the backend hypothesis. What happens if you hack
ps_status.c to use PS_USE_NONE?

Nailed it. PS_USE_NONE fixes it.

Best,

Wolfgang

On Mar 17, 2024, at 02:44, Wolfgang Walther <walther@technowledgy.de> wrote:

Nailed it. PS_USE_NONE fixes it.

Given the musl (still?) does not define a preprocessor macro specific to it, is there a way of improving the test in pg_status.c to catch this case? It seems wrong that the current test passes a case that doesn't actually work.

Christophe Pettus:

Given the musl (still?) does not define a preprocessor macro specific to it, is there a way of improving the test in pg_status.c to catch this case? It seems wrong that the current test passes a case that doesn't actually work.

The missing macro is on purpose and unlikely to change:
https://openwall.com/lists/musl/2013/03/29/13

I also found this thread, which discusses exactly our case:
https://www.openwall.com/lists/musl/2022/08/17/1

Some quotes from that thread:

I understand that what Postgres et al are doing is a nasty hack.

And:

Applications that *really* want setproctitle type functionality can
presumably do something like re-exec themselves with a suitably large
argv[0] to give them safe space to overwrite with their preferred
message, rather than UB trying to relocate the environment (and auxv?
how? they can't tell libc they moved it) to some other location.

Could that be a more portable way of doing this?

Best,

Wolfgang

On Mar 17, 2024, at 06:11, Wolfgang Walther <walther@technowledgy.de> wrote:
The missing macro is on purpose and unlikely to change: https://openwall.com/lists/musl/2013/03/29/13

Indeed.

I also found this thread, which discusses exactly our case: https://www.openwall.com/lists/musl/2022/08/17/1

While getting proper setproctitle functionality on musl would be great, my goal was more modest: Have it correctly set PS_USE_NONE when compiling against musl.

On Sun, Mar 17, 2024 at 11:45 AM Christophe Pettus <xof@thebuild.com> wrote:

On Mar 17, 2024, at 06:11, Wolfgang Walther <walther@technowledgy.de>

wrote:

The missing macro is on purpose and unlikely to change:

https://openwall.com/lists/musl/2013/03/29/13

Indeed.

That seems a little shortsighted. If other libc implementations find it
appropriate to have similar macros why should they be different?

I also found this thread, which discusses exactly our case:

https://www.openwall.com/lists/musl/2022/08/17/1

While getting proper setproctitle functionality on musl would be great, my
goal was more modest: Have it correctly set PS_USE_NONE when compiling
against musl.

One simple thing might be for us to enclose the block in ps_status.c at
lines 49-59 in #ifndef PS_USE_NONE/#endif. Then you could compile with
-DPS_USE_NONE in your CPPFLAGS.

cheers

andrew

On Mar 17, 2024, at 13:33, Andrew Dunstan <andrew@dunslane.net> wrote:

That seems a little shortsighted. If other libc implementations find it appropriate to have similar macros why should they be different?

It's a philosophical argument against checking for particular libc implementations instead of particular features. I'm not unsympathetic to that argument, but AFAICT there's no clean way of checking for this by examining feature #defines.

On Mon, Mar 18, 2024 at 10:06 AM Christophe Pettus <xof@thebuild.com> wrote:

On Mar 17, 2024, at 13:33, Andrew Dunstan <andrew@dunslane.net> wrote:
That seems a little shortsighted. If other libc implementations find it appropriate to have similar macros why should they be different?

It's a philosophical argument against checking for particular libc implementations instead of particular features. I'm not unsympathetic to that argument, but AFAICT there's no clean way of checking for this by examining feature #defines.

I like their philosophy, and I like their FAQ. Down with software
monocultures, up with standards and cooperation. But anyway...

I wondered for a moment if there could be a runtime way to test if
we'd broken stuff, but it seems hard without a way to ask the runtime
linker for its search path to see if it has any pointers into the
environment. We can't, that "env_path" variable in dynlink.c is not
accessible to us by any reasonable means. And yeah, this whole thing
is a nasty invasive hack that harks back to the 80s I assume, before
many systems provided a clean way to do this (and some never did)...

Hmm, I can think of one dirty hack on top of our existing dirty hack
that might work. I feel bad typing this out, but here goes nothing:
In save_ps_display_args(), we compute end_of_area, stepping past
contiguous arguments and environment variables. But what if we
terminated that if we saw an environment entry beginning "LD_"? We'd
still feel free to clobber the memory up to that point (rather than
limiting ourselves to the argv space, another more conservative choice
that might truncate a few PS display messages, or maybe not given the
typical postmaster arguments, maye that'd work out OK), and we'd still
copy the environment to somewhere new, but anything like "LD_XXX" that
the runtime linker might have stashed a pointer to would remain valid.
/me runs away and hides

On Mar 17, 2024, at 16:20, Thomas Munro <thomas.munro@gmail.com> wrote:

We'd
still feel free to clobber the memory up to that point (rather than
limiting ourselves to the argv space, another more conservative choice
that might truncate a few PS display messages, or maybe not given the
typical postmaster arguments, maye that'd work out OK), and we'd still
copy the environment to somewhere new, but anything like "LD_XXX" that
the runtime linker might have stashed a pointer to would remain valid.
/me runs away and hides

It doesn't lack for bravery! (And I have to just comment that the linker storing pointers into that space as a way of finding libraries... well, that doesn't get them the moral high ground for nasty hacks.)

I'm comfortable with "if you are using musl, you don't get the ps messages" as a first solution, if we can find a way of detecting a libc that passes the other tests but doesn't support any of the existing hacks.

On Mon, Mar 18, 2024 at 10:34 PM Christophe Pettus <xof@thebuild.com> wrote:

On Mar 17, 2024, at 16:20, Thomas Munro <thomas.munro@gmail.com> wrote:
We'd
still feel free to clobber the memory up to that point (rather than
limiting ourselves to the argv space, another more conservative choice
that might truncate a few PS display messages, or maybe not given the
typical postmaster arguments, maye that'd work out OK), and we'd still
copy the environment to somewhere new, but anything like "LD_XXX" that
the runtime linker might have stashed a pointer to would remain valid.
/me runs away and hides

It doesn't lack for bravery! (And I have to just comment that the linker storing pointers into that space as a way of finding libraries... well, that doesn't get them the moral high ground for nasty hacks.)

FWIW here is a blind patch if someone wants to try it out... no musl here.

(Hmm, I think it's not that unreasonable on their part to assume the
initial environment is immutable if their implementation doesn't
mutate it, and our doing so is undeniably UB; surprising, maybe, given
that the technique works on that other popular brand of C library on
that kind of kernel, not to mention dozens of old Unixen of yore...
The real solution may turn out to be the prctl() described in that
thread, where you can tell the kernel where you're planning to move
your argv and it can find it to show ps/top, but I checked and you
still can't call that without special privileges, so maybe someone
should get onto complaining to kernel hackers about that? That thread
is wrong about us clobbering auxv BTW, we're not animals!)

Thomas Munro <thomas.munro@gmail.com> writes:

(Hmm, I think it's not that unreasonable on their part to assume the
initial environment is immutable if their implementation doesn't
mutate it, and our doing so is undeniably UB; surprising, maybe, given
that the technique works on that other popular brand of C library on
that kind of kernel, not to mention dozens of old Unixen of yore...

Does their implementation also ignore the effects of putenv() or
setenv() on LD_LIBRARY_PATH? They have no moral high ground
whatsoever if that's the case. But if it doesn't, an alternative
route to a solution could be to scan the original environment, strdup
and putenv each entry to move it to freshly malloc'd space, and
then reclaim the old environment area.

regards, tom lane