criu: dlopen() optional libraries

[criupatchwork]

Introduce support for dynamically loaded libraries in CRIU for
optional functionality.

Currently we check for optional dependencies build-time which makes
them non-optional for the end user and requires installation on build.

Let's move the optional functionality to run-time.
In the end such libraries could be put in a separate section of
the Specfile. As for example `firefox' on my system:

Name : firefox
Version : 70.0.1-1
Description : Standalone web browser from mozilla.org
Depends On : gtk3 libxt startup-notification mime-types dbus-glib ffmpeg
nss ttf-font libpulse
Optional Deps : networkmanager: Location detection via available WiFi networks
libnotify: Notification integration [installed]
pulseaudio: Audio support [installed]
speech-dispatcher: Text-to-Speech
hunspell-en_US: Spell checking, American English [installed]

This also helps with adding nftables support (#861) as
libnftables.so.0 is ABI-incompatible with libnftables.so.1

So, I wish we wouldn't bind to a particular version.

Though, the introduced cr-libs force you to put supported major version
of the library as per semver.org the new major version means that there
may be incompatible API-breaking changes.

And finally, I'm not very fond of this boilerplate that's result of
helper shared_libs_lookup_once() - but we can introduce some macro
such as DEFINE_SHARED_LIB_FUNCION(lib_id, name, ...) to add some sugar
and reduce the code for every library's function..
Still, I wanted to send this before introducing this macro while it's
more clear how it works "inside".

Dmitry Safonov (6):
log: Include stdbool.h
criu: Use shared libraries for optional functionality
criu: Add lib-bsd
criu: Remove CONFIG_HAS_LIBBSD
net/lsm: Move default socket labeling helpers in lsm.c
lsm: Remove reset_setsockcreatecon()

Makefile.config | 11 +--
criu/Makefile.crtools | 3 +-
criu/cgroup-props.c | 1 -
criu/cgroup.c | 1 -
criu/cr-dump.c | 2 +-
criu/cr-libs.c | 133 +++++++++++++++++++++++++++++++++++
criu/cr-service.c | 2 +-
criu/crtools.c | 20 ++++--
criu/include/cr-libs.h | 39 ++++++++++
criu/include/lib-bsd.h | 11 +++
criu/include/log.h | 3 +-
criu/include/lsm.h | 19 ++---
criu/include/setproctitle.h | 19 -----
criu/include/string.h | 20 ------
criu/{string.c => lib-bsd.c} | 63 +++++++++++++----
criu/log.c | 2 +-
criu/lsm.c | 97 +++++++++++++++++++++----
criu/net.c | 77 +++-----------------
criu/proc_parse.c | 2 +-
criu/sk-inet.c | 2 +-
criu/tun.c | 2 +-
scripts/feature-tests.mak | 50 -------------
22 files changed, 354 insertions(+), 225 deletions(-)
create mode 100644 criu/cr-libs.c
create mode 100644 criu/include/cr-libs.h
create mode 100644 criu/include/lib-bsd.h
delete mode 100644 criu/include/setproctitle.h
delete mode 100644 criu/include/string.h
rename criu/{string.c => lib-bsd.c} (52%)

This PR is autogenerated from: https://patchwork.criu.org/series/3883

[adrianreber]

From a packaging point of view I am not sure this is a good idea. Libraries linked against CRIU are automatically detected by RPM. If libraries are dlopen()ed it requires manual changes. I am not against this change just mentioning that it is not optimal for packaging.

[0x7f454c46]

Hi @adrianreber,
Yeah, I see. Is there such a thing as optional packets in RPM?
Like, CRIU can still work without libnftables, libselinux, libbsd and libtls.
It will miss some features, but those aren't essential.
That's what I've seen in Arch Linux packaging specs, I thought it's a common practice.
If it's going to make distribution maintainers job harder - than it's the opposite of what I've tried to achieve with this and I should reconsider the approach..

[adrianreber]

It probably depends on the distribution and the environment. For Fedora at least all dependencies are available because they have to. That is, however, not really a problem for Fedora at least. Also RHEL and CentOS can live with the current situation as the libraries are not really huge or are anyway there.

Could be that Arch Linux tries to provide the user more flexibility by not linking to non-essential libraries. I have seen situation where dlopen()ing is a good thing if a distribution can not ship certain libraries for legal reasons (media codecs like mp3 some years ago).

Yeah, I see. Is there such a thing as optional packets in RPM?

There are optional packets, but it is not very common to use it for libraries (maybe because it is a newer feature in RPM).

Like, CRIU can still work without libnftables, libselinux, libbsd and libtls.

As the list of libraries is getting longer the idea is probably good, but it will be difficult for the users to understand why some features are not working. From a Fedora point of view all mentioned libraries are anyway on the system (besides libbsd) as they are used by multiple packages.

So on Fedora nothing is really improved using dlopen() besides having to track manually the SO names in the RPM package. The libraries are installed anyway. But if it makes sense for other distributions I do not want to block it.

[0x7f454c46]

@adrianreber Yeah, well I thought it would improve the life for people compiling on one system and using CRIU on another with a different set of libs (for example, I don't have SELinux), but if it makes life harder for packaging, I would say we shouldn't take this way.

Hm, on the other side on Fedora nothing is really improved using dlopen() besides having to track manually the SO names in the RPM package - I guess you don't have to. As those are optional.
So, a user can see warnings in log* like: Can't load libselinux.so - won't C/R SE labels or Can't load libnftables.so - won't C/R nftables rules. Probably if the user doesn't have them on system, he doesn't use those features..
*[I know that the last any user wants is to look where is the criu dump/restore log and what's inside]

FWIW, @adrianreber do you know any way to give a hint to RPM to automatically detect a dependency? (if there is any)

In conclusion, I'm not any insisting on those changes - in my POV, they make sense to let nftables C/R working with the two ABIs of libnftables, but the last I want is to create difficulties for packages maintainers.

[adrianreber]

FWIW, @adrianreber do you know any way to give a hint to RPM to automatically detect a dependency? (if there is any)

RPM basically does a ldd to see which libraries are used by the binaries in the package and creates the following dependency list for CRIU:

$ rpm -q criu --requires
/usr/bin/bash
libbsd.so.0()(64bit)
libbsd.so.0(LIBBSD_0.0)(64bit)
libbsd.so.0(LIBBSD_0.5)(64bit)
libbsd.so.0(LIBBSD_0.6)(64bit)
libc.so.6()(64bit)
libc.so.6(GLIBC_2.10)(64bit)
libc.so.6(GLIBC_2.13)(64bit)
libc.so.6(GLIBC_2.14)(64bit)
libc.so.6(GLIBC_2.2.5)(64bit)
libc.so.6(GLIBC_2.22)(64bit)
libc.so.6(GLIBC_2.28)(64bit)
libc.so.6(GLIBC_2.3)(64bit)
libc.so.6(GLIBC_2.3.2)(64bit)
libc.so.6(GLIBC_2.3.4)(64bit)
libc.so.6(GLIBC_2.4)(64bit)
libc.so.6(GLIBC_2.5)(64bit)
libc.so.6(GLIBC_2.7)(64bit)
libc.so.6(GLIBC_2.8)(64bit)
libc.so.6(GLIBC_2.9)(64bit)
libdl.so.2()(64bit)
libdl.so.2(GLIBC_2.2.5)(64bit)
libnet.so.1()(64bit)
libnl-3.so.200()(64bit)
libnl-3.so.200(libnl_3)(64bit)
libprotobuf-c.so.1()(64bit)
libprotobuf-c.so.1(LIBPROTOBUF_C_1.0.0)(64bit)
libselinux.so.1()(64bit)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsZstd) <= 5.4.18-1
rtld(GNU_HASH)

[mihalicyn]

@0x7f454c46
what did we decide with that patchset?
Do I need to rebase nftables support onto these changes?

[0x7f454c46]

Hi @mihalicyn, so I was talking with @avagin to get his point of view over dlopen() patches.
Related to nft - he suggested that we could look if there is a way to detect if nft rules are present and only than fail the dump if no libnftables is present.
I believe we could look for nft module in /proc/modules. Though it doesn't say that this particular container uses nft, so maybe there is more precise way to tell (some sysfs file?).
About this pull-request, per my perception, he is pro-dlopen idea. But again, I'm not pushing to it - in my opinion, it's better to make CRIU more modular, but I'm trying to get other's points of view as I can be mistaken.

[mihalicyn]

Hi @0x7f454c46

About this pull-request, per my perception, he is pro-dlopen idea.

If I understood correctly, Andrew also thinks that dlopen()-patch is a good idea?
I also thinks that this is good approach with dlopen. But as Adrian noticed we need to think about packages maintainers... Too hard question. We need to make a full list of pros and cons. :)

offtop

Related to nft - he suggested that we could look if there is a way to detect if nft rules are present and only than fail the dump if no libnftables is present.

Currently, we have no way to determine which container use (or not use) nft. But:

• I could try to implement this if needed and prepare the patch for Netfilter
  or
• We could use a raw Netlink-socket interface only to determine is ruleset empty or not. It's not hard.

[adrianreber]

From the point of view of a packager I do not like this change. Usually the libraries are installed once and do not change or go away. So, as a packager, this make my life harder.

[Snorch]

Hi @mihalicyn, so I was talking with @avagin to get his point of view over dlopen() patches.
Related to nft - he suggested that we could look if there is a way to detect if nft rules are present and only than fail the dump if no libnftables is present.
I believe we could look for nft module in /proc/modules. Though it doesn't say that this particular container uses nft, so maybe there is more precise way to tell (some sysfs file?).

I think that detecting if nft is used in CT without nftables library available is a bad idea, because AFAICS there is no info in sysfs about used/unused nft and writing separate bare netlink detection ruins all benefits we gain from library (e.g. if kernel interface will change library will smoothly switch us to a new API without the need to write compatibility code).

So I think for now we can skip nftables c/r if no nftables library available on compilation.

[0x7f454c46]

To sum up: I still believe it's a great idea. But as it makes @adrianreber life harder, it lacks enough justification in doing so ;-)
So, let's drop it for now. Keeping the discussion open if some more pros will appear.
Thanks for the discussion @adrianreber, @mihalicyn, @Snorch.

[0x7f454c46]

Adding here from criu gitter channel:

sunmoon9898 @sunmoon9898 Apr 07 14:18
Hi, we have a low memory system in ARM64 and want to checkpoint and restore a special process on it. In this way, we don’t need a full features of the criu. Is there any direction to trim it?

Pavel Tikhomirov @Snorch Apr 07 14:32
@sunmoon9898 Can you please explain your point a little bit more? Do you mean that you need to decrease the size of criu binary? (Make it less than <6Mb) And can you also please list what exact criu features you don't need?

sunmoon9898 @sunmoon9898 Apr 07 14:49
@Snorch thanks for your quick reply. yeah, I want to decrease the size of ciru. The real size of the running ciru instance may larger than 6M , because some shared libs are not in the ciru img.

sunmoon9898 @sunmoon9898 Apr 07 15:00
are those libs " libselinux/libgnutls.so.3 / libprotobuf-c.so.1/ libdl.so.2/libnl-3.so. /[libnet.so.1 “ nesserary to load before running criu?

Pavel Tikhomirov @Snorch Apr 07 15:06
You can see ./Makefile.config some libraries are not strictly required and they are disabled on compilation if not found, for example libnftables libbsd libgnutls. libprotobuf though would be a 100% required because criu saves it's image files in protobuf format

that user could benifit from dlopen()

[0x7f454c46]

And another one:

phlax @phlax 12:47
hi - i have built a debian buster backport of criu. It seems to work ok - ie builds/installs/checkpoints/restores
i had a couple of questions. Firstly i had to build a backport for gnutls and deps, as criu in debian unstable requires libgnutls30 > 3.6.12
but debian stable version is 3.6.7
im wondering if its possible to build against the gnutls version in stable
the other question was - altho it checkpoints/restores i get a few errors/warnings
which can be seen on travis, but travis seems to be having problems, ill post later
the recipe for backport can be seen here...
https://github.com/phlax/criu-backport

Adrian Reber @adrianreber 14:48
@phlax you could build without gnutls support if you do not need the encrypted page-server

phlax @phlax 14:49
i dont think i do - but i guess the question was more in terms of backporting criu