Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential issues with dynamically loaded criu #1080

Closed
jpalus opened this issue Nov 25, 2022 · 5 comments
Closed

Potential issues with dynamically loaded criu #1080

jpalus opened this issue Nov 25, 2022 · 5 comments

Comments

@jpalus
Copy link
Contributor

jpalus commented Nov 25, 2022

Latest release (1.7.1) switches to loading criu dynamically instead of linking against it. That's perfectly fine and welcome change however there is a concerning detail about it: dynamic loading relies on libcriu.so:

wrapper->handle = dlopen ("libcriu.so", RTLD_NOW);

That has two consequences:

  • libcriu.so is part of "devel" packages in many distributions which are only required for building, not during runtime. Therefore in order to use criu end-user would need to install development package
  • since loading mechanism does not rely on SONAME anymore it also ignores any potential ABI changes. Current SONAME for libcriu.so is libcriu.so.2. Suppose ABI changes in criu, new version is released with SONAME libcriu.so.3 since it's incompatible with previous release. Now there's nothing to stop crun from loading ABI incompatible new library and potentially crash in mysterious way

It would be nice if crun could detect and use SONAME from libcriu.so for loading, determined during configure. Also as a fallback and maybe as a first step there could be --with-criu-library-name=<lib> option to override default.

@giuseppe
Copy link
Member

@adrianreber what do you think about it? Should we detect SONAME at configure time?

@adrianreber
Copy link
Contributor

That is a very good point. crun should really load the so versioned file.

@giuseppe
Copy link
Member

could we just hardcode libcriu.so.2 for now?

@adrianreber
Copy link
Contributor

Sure. There are no plans to change the API anytime soon and if the API is changed then crun needs to be adapted anyway.

giuseppe added a commit to giuseppe/crun that referenced this issue Nov 29, 2022
Closes: containers#1080

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
Copy link
Member

fix here: #1084

giuseppe added a commit to giuseppe/crun that referenced this issue Nov 29, 2022
Closes: containers#1080

Signed-off-by: Giuseppe Scrivano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants