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

test: drop mocklibc #461

Merged
merged 3 commits into from
Jun 26, 2024
Merged

test: drop mocklibc #461

merged 3 commits into from
Jun 26, 2024

Conversation

mrc0mmand
Copy link
Member

@mrc0mmand mrc0mmand commented Jun 10, 2024

Let's get rid of mocklibc and replace it with a simple combination of mount & user namespaces + bind mount to replace the host's /etc with our own version.

This means we don't $LD_PRELOAD the mocklibc DSO, but instead run each unit test through a very simple python wrapper that sets up a temporary user & mount namespace through the unshare() syscall, gains "fake" root using uid_map and gid_map, overmounts /etc in this new namespace (with our own custom test files), and then executes the test binary itself. Check user_namespaces(7) for more information about the namespace shenanigans.

@mrc0mmand mrc0mmand force-pushed the drop-mocklibc branch 12 times, most recently from c623ccf to de16e75 Compare June 11, 2024 08:49
@mrc0mmand
Copy link
Member Author

mrc0mmand commented Jun 11, 2024

@jrybar-rh here's the promised mocklibc replacement. It replaces mocklibc with a simple python script that sets up a user and mount namespace, so we can overmount /etc for the unit tests without disturbing the underlying system (and so we can do that at all as an unprivileged user). This also allows us to run all unit tests under sanitizers, which uncovered one memory leak, see the second commit. The last commit also shows that the tests can be now run as part of an RPM build if desired.

While testing this I noticed the CIs (and my local runs) skip the netgroup tests, because the test sources don't include config.h with necessary #defines, so preferably #454 should land first which mitigates this issue without having to scatter more #include "config.h" across the codebase.

env: test_env,
is_parallel: false,
test_wrapper,
args: ['--data-dir', test_data_dir, exe.full_path()],
Copy link
Member

Choose a reason for hiding this comment

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

Is this applicable to still-supported F39, which we use for CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, thanks to the last patch the tests run in all Packit CIs, including the F39 one, and everything seems to pass there.

Let's get rid of mocklibc and replace it with a simple combination of
mount & user namespaces + bind mount to replace the host's /etc with our
own version.

This means we don't $LD_PRELOAD the mocklibc DSO, but instead run each
unit test through a very simple python wrapper that sets up a temporary
user & mount namespace through the unshare() syscall,  gains "fake" root
using uid_map and gid_map, overmounts /etc in this new namespace (with
our own custom test files), and then executes the test binary itself.
Check user_namespaces(7) for more information about the namespace
shenanigans.
duk_error() never returns, so the error string gets leaked every time an error
is thrown. Let's avoid this by creating the error object first without throwing
it, freeing the original error string (we don't need it anymore since it gets
sprintf-ed into the error object), and then throwing the error object from top
of the current context stack.

 =================================================================
==1270==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 231 byte(s) in 2 object(s) allocated from:
    #0 0x7f3a489258b7 in malloc (/lib64/libasan.so.8+0xf68b7) (BuildId: 388cbb99455c2e2eaec79bd8db6d9a78eb39f80d)
    #1 0x7f3a47b27487 in __vasprintf_internal (/lib64/libc.so.6+0x8a487) (BuildId: 4a92fcedbba6d6d2629ce066a2970017faa9995e)
    #2 0x7f3a484b06a2 in g_vasprintf (/lib64/libglib-2.0.so.0+0xb16a2) (BuildId: 795136df3faa85587229ddc59d709f81d6f697df)
    #3 0x7f3a48480a92 in g_strdup_vprintf (/lib64/libglib-2.0.so.0+0x81a92) (BuildId: 795136df3faa85587229ddc59d709f81d6f697df)
    polkit-org#4 0x7f3a48480b50 in g_strdup_printf (/lib64/libglib-2.0.so.0+0x81b50) (BuildId: 795136df3faa85587229ddc59d709f81d6f697df)
    polkit-org#5 0x41fcec in js_polkit_spawn ../src/polkitbackend/polkitbackendduktapeauthority.c:1090
    polkit-org#6 0x7f3a483b31b8 in duk__handle_call_raw.lto_priv.0 (/lib64/libduktape.so.207+0x2a1b8) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#7 0x7f3a483962e1 in duk__js_execute_bytecode_inner.lto_priv.0 (/lib64/libduktape.so.207+0xd2e1) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#8 0x7f3a483b33eb in duk_js_execute_bytecode (/lib64/libduktape.so.207+0x2a3eb) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#9 0x7f3a483b319d in duk__handle_call_raw.lto_priv.0 (/lib64/libduktape.so.207+0x2a19d) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#10 0x7f3a48399ab0 in duk__pcall_prop_raw (/lib64/libduktape.so.207+0x10ab0) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#11 0x7f3a483b23cc in duk_handle_safe_call.lto_priv.0 (/lib64/libduktape.so.207+0x293cc) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#12 0x7f3a483972e8 in duk_pcall_prop (/lib64/libduktape.so.207+0xe2e8) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#13 0x41d048 in runaway_killer_thread_call_js ../src/polkitbackend/polkitbackendduktapeauthority.c:682
    polkit-org#14 0x7f3a4888cb45 in asan_thread_start(void*) (/lib64/libasan.so.8+0x5db45) (BuildId: 388cbb99455c2e2eaec79bd8db6d9a78eb39f80d)

Direct leak of 128 byte(s) in 1 object(s) allocated from:
    #0 0x7f3a489247b8 in realloc.part.0 (/lib64/libasan.so.8+0xf57b8) (BuildId: 388cbb99455c2e2eaec79bd8db6d9a78eb39f80d)
    #1 0x7f3a4846304a in g_realloc (/lib64/libglib-2.0.so.0+0x6404a) (BuildId: 795136df3faa85587229ddc59d709f81d6f697df)
    #2 0x7f3a48481b19 in g_string_expand (/lib64/libglib-2.0.so.0+0x82b19) (BuildId: 795136df3faa85587229ddc59d709f81d6f697df)
    #3 0x7f3a48481b90 in g_string_sized_new (/lib64/libglib-2.0.so.0+0x82b90) (BuildId: 795136df3faa85587229ddc59d709f81d6f697df)
    polkit-org#4 0x41fb0f in js_polkit_spawn ../src/polkitbackend/polkitbackendduktapeauthority.c:1099
    polkit-org#5 0x7f3a483b31b8 in duk__handle_call_raw.lto_priv.0 (/lib64/libduktape.so.207+0x2a1b8) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#6 0x7f3a483962e1 in duk__js_execute_bytecode_inner.lto_priv.0 (/lib64/libduktape.so.207+0xd2e1) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#7 0x7f3a483b33eb in duk_js_execute_bytecode (/lib64/libduktape.so.207+0x2a3eb) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#8 0x7f3a483b319d in duk__handle_call_raw.lto_priv.0 (/lib64/libduktape.so.207+0x2a19d) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#9 0x7f3a48399ab0 in duk__pcall_prop_raw (/lib64/libduktape.so.207+0x10ab0) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#10 0x7f3a483b23cc in duk_handle_safe_call.lto_priv.0 (/lib64/libduktape.so.207+0x293cc) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#11 0x7f3a483972e8 in duk_pcall_prop (/lib64/libduktape.so.207+0xe2e8) (BuildId: a9f661ee1766489794e9ece7cfd0d6a7fb420ccb)
    polkit-org#12 0x41d048 in runaway_killer_thread_call_js ../src/polkitbackend/polkitbackendduktapeauthority.c:682
    polkit-org#13 0x7f3a4888cb45 in asan_thread_start(void*) (/lib64/libasan.so.8+0x5db45) (BuildId: 388cbb99455c2e2eaec79bd8db6d9a78eb39f80d)

SUMMARY: AddressSanitizer: 359 byte(s) leaked in 3 allocation(s).
@jrybar-rh jrybar-rh marked this pull request as ready for review June 26, 2024 15:04
@jrybar-rh jrybar-rh merged commit 8e17f09 into polkit-org:main Jun 26, 2024
32 checks passed
@mrc0mmand mrc0mmand deleted the drop-mocklibc branch June 26, 2024 18:42
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

Successfully merging this pull request may close these issues.

2 participants