Simon McVittie authored 7 months ago and Vivek Das Mohapatra committed 7 months ago

Signed-off-by: Simon McVittie <smcv@collabora.com>

Simon McVittie authored 7 months ago and Vivek Das Mohapatra committed 7 months ago

This makes it easier to reason about their valid lifetimes.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Simon McVittie authored 7 months ago and Vivek Das Mohapatra committed 7 months ago

Was: `i386-linux-gnu-capsule-capture-libs: capture_one:We already have a symlink for libm.so.6`

Now: `i386-linux-gnu-capsule-capture-libs: capture_one: We already have a symlink for libm.so.6`

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Conventionally, for each version definition in a shared library, there
is a symbol named after the verdef with absolute value 0. For example,
libdbus has a symbol LIBDBUS_1_3@LIBDBUS_1_3, and similarly libxkbcommon
has symbols V_0.8.0@V_0.8.0 and so on.

These will typically not match any of the glob patterns that we use to
mark symbols as being public or private, leading to spurious warnings
like this one:

    i386-linux-gnu-capsule-capture-libs: warning: we are assuming "V_0.5.0" to be private, but it's just a guess

So far, we have avoided this because we have not used the "symbols"
comparison order for any library that has verdefs, but I'm intending
to use "versions,name,symbols" for libxkbcommon.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Previously, if we were using --remap-link-prefix but not --link-target
(as in pressure-vessel), and if the path at which we found the
library in the provider (`needed_path_in_provider`) was such that
after resolving symlinks, the canonicalized result (`path`) did not
match any of the --remap-link-prefix options, then we would reset
to using the `needed_path_in_provider` as the symlink target.

However, this was wrong, because it did not take into account the
possibility that the original path might have matched one of the
--remap-link-prefix options even though its canonicalized version didn't.

For example, consider the NixOS "FHS environment", a bwrap-based
container that has been populated with symlinks of the form
/lib/libGL.so.1 -> /nix/store/something. Depending on the precise
behaviour of libc and libdl (the exact trigger for the differing
behaviour is unknown), when we open that library, we might find
that the `needed_path_in_provider` is either /lib/libGL.so.1
or a path below /nix/store.

If the `needed_path_in_provider` is /lib/libGL.so.1, we would
canonicalize it to /nix/store/something, then decide that
/nix/store/something didn't match any of the remapping rules such
as `/lib*` → `/run/host/lib*`, and go back to using /lib/libGL.so.1
as the symlink target. But, inside pressure-vessel's container, `/lib`
is from the container runtime and not the provider - which is precisely
why we need to rewrite /lib paths to /run/host/lib in order to be
able to find the library we want.

Instead, we must limit the situation where we keep the
`needed_path_in_provider` as-is to the situation where there is no
symlink target rewriting: no --link-target, and no --remap-link-prefix.
In this situation, we are expecting the meaning of a symlink in the
container environment to be identical to the meaning of a symlink in
the environment from which we are running capsule-capture-libs,
therefore it's slightly preferable to use a non-canonicalized symlink
if we have one: that way, if the target of a symlink changes during
the container's lifetime (perhaps as a result of an OS upgrade),
it will still work.

Conversely, if we are using either --link-target or --remap-link-prefix,
we want to stick to using only the canonicalized paths of libraries,
because that's the only way we can avoid a symlink being interpreted
differently outside and inside the container, for example as a result
of its parent directory being mounted in a different place, or as a
result of something different being mounted at its target.

This means that we have to relax one of our test assertions slightly.

Helps: https://github.com/ValveSoftware/steam-runtime/issues/684


Signed-off-by: Simon McVittie <smcv@collabora.com>

In a test that already emits a warning message, prepending "quiet:"
to the pattern should de-escalate the warning to a debug message.

In a test that emits a fatal error, prepending "quiet:" to the pattern
has no effect: it's still an error.

steamrt/tasks#488

Signed-off-by: Simon McVittie <smcv@collabora.com>

Sometimes we find ourselves capturing a library for which the
dependencies can legitimately be missing, for example
`libnvidia-pkcs11.so.*` on Ubuntu 24.04 (which depends on OpenSSL 1.1,
which Ubuntu no longer provides).

This is really an OS bug (providing a library without its dependencies),
and ideally Ubuntu should stop providing `libnvidia-pkcs11.so.*`
(like Debian did), but it results in confusing messages that could make
a user wrongly believe that it's the root cause of a genuine problem.

steamrt/tasks#488

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Previously we said "warning:" for about half the warnings, and didn't
annotate fatal errors at all.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Repeat the test-case where we produced an error message about a library
having the wrong ABI, but this time, use --level-prefix and assert that
we add the <3> prefix for a fatal error.

steamrt/tasks#444

Signed-off-by: Simon McVittie <smcv@collabora.com>

This will allow the output of capsule-capture-libs (and in principle
other tools) to be piped to a log handler such as `logger --prio-prefix`,
`systemd-cat --level-prefix=true` or `srt-logger --parse-level-prefix`
which will filter or format the messages according to their severity.

As currently implemented, fatal errors are emitted as LOG_ERR, warnings
are LOG_WARNING, and all debug messages are LOG_DEBUG. A facility such
as LOG_USER is not included: this is optional for logger(1), and not
allowed for systemd-cat(1) and srt-logger.

steamrt/tasks#444

Signed-off-by: Simon McVittie <smcv@collabora.com>

In a couple of places this requires explicitly emitting the stringified
value of errno, because we have equivalents of warnx() and errx(), but
not warn() and err().

Signed-off-by: Simon McVittie <smcv@collabora.com>

These mimic the behaviour of errx() and warnx(), but will benefit from
future enhancements to capsule_log().

Signed-off-by: Simon McVittie <smcv@collabora.com>

This allows them to be distinguished from messages logged by some
other executable invoked around the same time, and makes
`capsule_log( LOG_WARNING, ... )` a drop-in substitute for `warnx()`.

Signed-off-by: Simon McVittie <smcv@collabora.com>

This will let us change the behaviour of all diagnostic messages at
once, by changing only capsule_logv().

For now, the syslog-style severity is accepted but ignored.
Subsequent commits will make use of this.

Signed-off-by: Simon McVittie <smcv@collabora.com>

We don't enforce a specific error message, but we do assert that there's
no output on stdout, and an error message prefixed with the program name
on one of the lines of stderr.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

This makes the information from doc/Building-libcapsule.txt more
discoverable, and adds more.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Because libcapsule uses non-recursive make, G_TEST_SRCDIR and
G_TEST_BUILDDIR are set to the top-level directory of the source/build
tree. When running the test suite "as-installed", the equivalent for
both is `${libexecdir}/installed-tests/libcapsule`.

This matters because we locate the "version 1" and "version 2" libraries
during testing as `${builddir}/tests/version{1,2}`, which is incorrect
(a duplicate `tests/`) if the `${builddir}` is set to the `tests`
subdirectory rather than the top-level directory.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Steam Runtime 1 'scout' only has 1.11 as its default version, and that's
too old for AM_TESTS_ENVIRONMENT to be respected. It does have 1.15
available, so make sure we have at least that version one way or another.

At the moment this is mitigated by the fact that scout only has GLib 2.32,
which is too old, and we use --with-glib=auto to skip building and running
the one test that uses GLib (utils.t) when building on scout - which
happens to be the only test where correct handling of AM_TESTS_ENVIRONMENT
was strictly required. However, if we backport a newer GLib into scout,
we'll start running that test, and therefore we'll need it to work.

steamrt/tasks#112

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

The ld_libs data structure is pre-allocated at maximum size instead of
using dynamic memory allocation, in an effort to avoid use of malloc()
during critical-path interactions with basic shared library
infrastructure. This makes it about 1 MiB, so if users have reduced
RLIMIT_STACK from its post-1995 default of 8 MiB for whatever reason,
it doesn't take many levels of recursion to exceed that limit.

The symptom for a stack overflow is particularly un-diagnosable (instead
of exiting unsuccessfully with an error message, the program will just
segfault), so allocate this data structure on the heap instead, which
gives us the opportunity to log "libcapsule: out of memory".

The equivalent data structures within libcapsule itself are still
allocated on the stack, to avoid introducing new memory allocation
inside a critical section.

Resolves: https://github.com/ValveSoftware/steam-runtime/issues/653


Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

In zlib 1.3 (2023), the version is exactly libz.so.1.3, with no third
version component.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Some parts of libcapsule are sufficiently low-level to want to avoid
calling malloc(), instead using PATH_MAX-sized buffers and an assumption
that paths are, in practice, smaller than that.

Signed-off-by: Simon McVittie <smcv@collabora.com>

libcrypt.so.1 is build-time optional since glibc 2.28, and is not built
by default since 2.38. In newer distributions like Debian >= 11 and
Ubuntu >= 20.04, it's usually replaced by libxcrypt, either compiled
to be a drop-in replacement for glibc's libcrypt.so.1 (as in Debian,
and therefore the Steam Runtime), or with its own SONAME libcrypt.so.2
and optionally a secondary build as libcrypt.so.1 (as in Arch).

Because libxcrypt implements a superset of the glibc libcrypt.so.1 ABI,
adding some functions and symvers of its own, it is backward- but not
forward-compatible: it's OK to use libxcrypt libcrypt.so.1 as a
replacement for glibc libcrypt.so.1, but it is not OK to do the opposite.
This means it would be incorrect for us to use a system copy of glibc
libcrypt.so.1 (perhaps from glibc 2.32 or newer) as a replacement for a
container's libxcrypt libcrypt.so.1, even if the system copy of glibc
is strictly newer than the container's glibc (for example,
Steam Runtime 3 'sniper' is based on Debian 11, so it comes with
glibc 2.31 and libxcrypt).

Unlike libpthread, libdl and librt, symbols from `libcrypt.so.1` were
never absorbed into `libc.so.6`, so we don't have to apply the reasoning
seen in 4c2e3e3b "capture-libs: Add a special case to support glibc 2.34+".

In practice we've always got away with it in the past, but it's something
that could cause a crash or incompatibility. I'm now looking into the
possibility of backporting libxcrypt into Steam Runtime 1 and 2
(steamrt/tasks#332), which makes it more important to get this right.

Accordingly, exclude libcrypt.so.1 from that special treatment, and
instead set it up to be compared separately using the same comparators
we use for glibc: by public symvers, then by public symbols, and as a
last resort by name.

Signed-off-by: Simon McVittie <smcv@collabora.com>

These are only directly used within their translation unit,
therefore don't need to be extern.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Just for completeness, we're not going to support any of these any
time soon.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

Now that the tests pass on merged-/usr systems, we can use a more modern
Debian suite.

Normally I'd use the latest Debian stable (Debian 12), but that seems
to have a false positive for a printf argument to %s being NULL,
causing the asan/ubsan builds to fail; so use Debian 10 and 11 as our
representatives of older systems, together with Debian 13 prereleases
as our representative of a modern system.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Previously, we were assuming that the symlink created by
capsule-capture-libs would point to the `realpath()` of a library,
possibly with a `--link-target` prefix. However, it actually only
resolves the symlink representing the SONAME: it will resolve `libz.so.1`
to `libz.so.1.2.13`, but unlike Perl `abs_path()` or C `realpath()`,
it will not necessarily resolve symlinks in the directory hierarchy
leading up to that point.

For example, on a merged-/usr system like Debian >= 12, /lib is a
symbolic link to usr/lib. The realpath() of `libz.so.1` is something like
`/usr/lib/MULTIARCH/libz.so.1.2.13`, but because both `/lib/MULTIARCH`
and `/usr/lib/MULTIARCH` appear in `/etc/ld.so.conf.d`, it is undefined
whether capsule-capture-libs will output `/lib/MULTIARCH/libz.so.1.2.13`
or `/usr/lib/MULTIARCH/libz.so.1.2.13`.

Relax the expectations of this test so we only say that the symlink
points to some reasonable `$libdir`, followed by the `basename()` of the
`realpath()` of `libc.so.6`.

Signed-off-by: Simon McVittie <smcv@collabora.com>

Signed-off-by: Simon McVittie <smcv@collabora.com>

`warn(fmt, args)` is equivalent to (pseudocode)
`warnx(fmt + ": %s", args, strerror(errno))`. In contexts where we do
not have a useful value for errno, or where we are showing an error
message that should already contain a previous result of strerror, we
should use warnx() instead. This avoids showing a misleading errno
which might have been set for some unrelated reason.

Prompted by jupiter/tasks#887.

Signed-off-by: Simon McVittie <smcv@collabora.com>

glibc searches /lib:/usr/lib as a hard-coded fallback, but we don't
expect to get there when searching for dependencies in practice: on
OSs that have dedicated 32- and 64-bit directories (such as
Red-Hat-style multilib, Arch-style multilib and Debian-style multiarch),
those directories are searched indirectly as part of ld.so.cache.

Mentioning that this is a fallback makes it a little bit more obvious
that it isn't an error that we were not searching /usr/lib32, /usr/lib64
or /usr/lib/MULTIARCH when looking for a library of the appropriate
word size.

While I'm here, deduplicate the end of the message a bit.

Suggested-by: Emil Velikov
Signed-off-by: Simon McVittie <smcv@collabora.com>

If one of these happens, then something odd is going on and we should
diagnose it.

Signed-off-by: Simon McVittie <smcv@collabora.com>