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.

Sign up for GitHub

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

Jump to bottom

Allow to dlopen compression libraries #262

Merged
merged 14 commits into from
Dec 6, 2024
Merged

Allow to dlopen compression libraries #262

merged 14 commits into from
Dec 6, 2024

Conversation

lucasdemarchi
Copy link
Contributor

@lucasdemarchi lucasdemarchi commented Nov 30, 2024 •
edited
Loading

Allow to dlopen the compression libraries so distros can choose one to match the MODULE_COMPRESS_* option in the kernel, while still allowing to work with other compressed formats.

I'm not very happy with the meson integration, but I gave up while trying to handle the dependencies between the options.

Still missing here: the ELF note to declare the implicit dependency.

Closes: #48

@lucasdemarchi lucasdemarchi force-pushed the tip-dlopen branch 2 times, most recently from 88ae20b to 4f38062 Compare November 30, 2024 21:09
evelikov
evelikov requested changes Dec 2, 2024
View reviewed changes
Copy link
Collaborator

@evelikov evelikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for beating me to it.

shared/util.c Show resolved Hide resolved
libkmod/libkmod-file-xz.c Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
@lucasdemarchi lucasdemarchi force-pushed the tip-dlopen branch 3 times, most recently from 1087c45 to 3114741 Compare December 4, 2024 17:05
evelikov
evelikov reviewed Dec 5, 2024
View reviewed changes
Copy link
Collaborator

@evelikov evelikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a bunch of minimal - nice to have - comments.

The -D use-dlopen=true (or equiv) would be amazing to have both so a) we don't need to check for all permutations and b) distributions don't shoot themselves in the foot.

shared/util.h Outdated Show resolved Hide resolved
shared/util.c Show resolved Hide resolved
libkmod/libkmod-internal-file.h Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
@lucasdemarchi
util: Add dlfcn helpers
c8ed6f7 
Heavily based on systemd: add similar (but simplified) functions to
dlopen() a library and load specific symbols from it. This will be
useful to allow to load the compression libraries as needed. A few
differences from the systemd implementation:

1) It's allowed to link directly to the library and hence bypass the
   dlopen() + dlsym()
2) The only entrypoint is dlsym_many() which is already declared with
   the sentinel: it's expected callers will use an x-macro that doesn't
   allow forgetting the sentinel
3) No support yet for ELF info annotation yet

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
libkmod/xz: Allow to load liblzma.so on demand
00cf088 
Use dlfcn helpers to load liblzma once it's needed.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
libkmod/zlib: s/err/ret/ for consistency
fd917e6 
Use the libkmod-file-xz.c as reference, keeping similar variable names
so it's easy to share code.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
libkmod/zlib: Allow to load libz.so on demand
5abaf8d 
Use dlfcn helpers to load libz once it's needed.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
libkmod/zstd: Do not re-use size_t for ret
319d487 
size_t is unsigned, while the function returns a negative number.
Instead of using `ret` for the return from ZSTD_decompress(), re-use
dst_size that should be the same as we passed in for that call since
it already checks for ZSTD_CONTENTSIZE_UNKNOWN and ZSTD_CONTENTSIZE_ERROR.

Even if it's not the same, it's more correct to use that value to
assign to file->size to avoid accessing uninitialized memory.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
libkmod/zstd: Allow to load libzstd.so on demand
3469e73 
Use dlfcn helpers to load libzstd once it's needed.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
build: Always define ENABLE_* macros
975d04e 
Define either to 0 or 1 so codebase is forced to used `#if ENABLE_*` or
similar. It's confusing to have some leaving undefined and others
defining as 0 or 1.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
meson: Simplify compression handling
703625c 
Move libs to a table, similarly to how other things are handled,
so this logic can be more easily expanded in future for all compression
types.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
meson: Let openssl option follow same logic as compression
84961ed 
Keep the logic similar for all libraries. While at it, add a comment
divider for the config output.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
meson: Allow to set dlopen option for compression libraries
07faaf6 
Add a dlopen option that allows toggling what libraries libkmod should
attempt to dlopen. If -Ddlopen=foo is passed, it means that library is
required to build, regardless of -Dfoo=*. However that library will
only be linked in if it's not set as dlopen.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
libkmod: Add ELF notes to compression libraries
a848bbf 
Follow the new spec for ELF notes as detailed in
https://systemd.io/ELF_PACKAGE_METADATA/.

We can copy mostly verbatim the macros from systemd codebase.

Example output:

	$ meson setup --native-file build-dev.ini -Dxz=disabled -Ddlopen=zlib build
	...
	    dlopen           : zlib

	    features         : +ZSTD -XZ +ZLIB +OPENSSL

	$ dlopen-notes.py build/libkmod.so.2
	# build/libkmod.so.2
	[
	  {
	    "feature": "xz",
	    "description": "Support for uncompressing xz-compressed modules",
	    "priority": "recommended",
	    "soname": [
	      "liblzma.so.5"
	    ]
	  }
	]

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
ci: Fix fail due to existent build dir
d6610b3 
The second check is a "should fail" check, but it will fail because the
build dir already exists rather than for the true reason. Use a
different dir for the configure tests and move the `rm` inside the
function.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
ci: Add configure tests for dlopen
16dc3d2 
Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
ci: Add 2 more pipelines to the matrix
4d875e9 
Test disabling one of the compression libs while dlopen'ing the rest and
handling 1 linked and 2 dlopen'ed.

Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi lucasdemarchi merged commit 4d875e9 into master Dec 6, 2024
37 checks passed
@lucasdemarchi
Copy link
Contributor Author

lucasdemarchi commented Dec 6, 2024 •
edited
Loading

merged this by mistake while merging #264

lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
util: Add dlfcn helpers
1f49475 
Heavily based on systemd: add similar (but simplified) functions to
dlopen() a library and load specific symbols from it. This will be
useful to allow to load the compression libraries as needed. A few
differences from the systemd implementation:

1) It's allowed to link directly to the library and hence bypass the
   dlopen() + dlsym()
2) The only entrypoint is dlsym_many() which is already declared with
   the sentinel: it's expected callers will use an x-macro that doesn't
   allow forgetting the sentinel
3) No support yet for ELF info annotation yet

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod/xz: Allow to load liblzma.so on demand
fe58450 
Use dlfcn helpers to load liblzma once it's needed.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod/zlib: s/err/ret/ for consistency
bb84dcf 
Use the libkmod-file-xz.c as reference, keeping similar variable names
so it's easy to share code.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod/zlib: Allow to load libz.so on demand
b8f00f7 
Use dlfcn helpers to load libz once it's needed.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod/zstd: Do not re-use size_t for ret
9fd66be 
size_t is unsigned, while the function returns a negative number.
Instead of using `ret` for the return from ZSTD_decompress(), re-use
dst_size that should be the same as we passed in for that call since
it already checks for ZSTD_CONTENTSIZE_UNKNOWN and ZSTD_CONTENTSIZE_ERROR.

Even if it's not the same, it's more correct to use that value to
assign to file->size to avoid accessing uninitialized memory.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod/zstd: Allow to load libzstd.so on demand
9a3930e 
Use dlfcn helpers to load libzstd once it's needed.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
build: Always define ENABLE_* macros
01d9c6a 
Define either to 0 or 1 so codebase is forced to used `#if ENABLE_*` or
similar. It's confusing to have some leaving undefined and others
defining as 0 or 1.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
meson: Simplify compression handling
be63021 
Move libs to a table, similarly to how other things are handled,
so this logic can be more easily expanded in future for all compression
types.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
meson: Let openssl option follow same logic as compression
8d39823 
Keep the logic similar for all libraries. While at it, add a comment
divider for the config output.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
meson: Allow to set dlopen option for compression libraries
c22cce1 
Add a dlopen option that allows toggling what libraries libkmod should
attempt to dlopen. If -Ddlopen=foo is passed, it means that library is
required to build, regardless of -Dfoo=*. However that library will
only be linked in if it's not set as dlopen.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod: Add ELF notes to compression libraries
85d7102 
Follow the new spec for ELF notes as detailed in
https://systemd.io/ELF_PACKAGE_METADATA/.

We can copy mostly verbatim the macros from systemd codebase.

Example output:

	$ meson setup --native-file build-dev.ini -Dxz=disabled -Ddlopen=zlib build
	...
	    dlopen           : zlib

	    features         : +ZSTD -XZ +ZLIB +OPENSSL

	$ dlopen-notes.py build/libkmod.so.2
	# build/libkmod.so.2
	[
	  {
	    "feature": "xz",
	    "description": "Support for uncompressing xz-compressed modules",
	    "priority": "recommended",
	    "soname": [
	      "liblzma.so.5"
	    ]
	  }
	]

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
ci: Fix fail due to existent build dir
130463f 
The second check is a "should fail" check, but it will fail because the
build dir already exists rather than for the true reason. Use a
different dir for the configure tests and move the `rm` inside the
function.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
ci: Add configure tests for dlopen
7016adb 
Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
ci: Add 2 more pipelines to the matrix
2d0ae77 
Test disabling one of the compression libs while dlopen'ing the rest and
handling 1 linked and 2 dlopen'ed.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
meson: Allow to set dlopen option for compression libraries
bbab061 
Add a dlopen option that allows toggling what libraries libkmod should
attempt to dlopen. If -Ddlopen=foo is passed, it means that library is
required to build, regardless of -Dfoo=*. However that library will
only be linked in if it's not set as dlopen.

Signed-off-by: Lucas De Marchi <[email protected]>
[ with disagreement on the need to toggle each one individually,
  it'd be better to be all-or-nothing dlopen'ed ]
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
libkmod: Add ELF notes to compression libraries
c9f80c4 
Follow the new spec for ELF notes as detailed in
https://systemd.io/ELF_PACKAGE_METADATA/.

We can copy mostly verbatim the macros from systemd codebase.

Example output:

	$ meson setup --native-file build-dev.ini -Dxz=disabled -Ddlopen=zlib build
	...
	    dlopen           : zlib

	    features         : +ZSTD -XZ +ZLIB +OPENSSL

	$ dlopen-notes.py build/libkmod.so.2
	# build/libkmod.so.2
	[
	  {
	    "feature": "xz",
	    "description": "Support for uncompressing xz-compressed modules",
	    "priority": "recommended",
	    "soname": [
	      "liblzma.so.5"
	    ]
	  }
	]

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
ci: Fix fail due to existent build dir
fa24575 
The second check is a "should fail" check, but it will fail because the
build dir already exists rather than for the true reason. Use a
different dir for the configure tests and move the `rm` inside the
function.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
ci: Add configure tests for dlopen
a3467eb 
Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
lucasdemarchi added a commit that referenced this pull request Dec 6, 2024
@lucasdemarchi
ci: Add 2 more pipelines to the matrix
ccc26d6 
Test disabling one of the compression libs while dlopen'ing the rest and
handling 1 linked and 2 dlopen'ed.

Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Link: #262
@lucasdemarchi
Copy link
Contributor Author

merged this by mistake while merging #264

and now pushed the right version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evelikov evelikov evelikov requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dlopen (de)compression libraries
2 participants
@lucasdemarchi @evelikov