Skip to content

fix test segfaults caused by uninitialized memory #739

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

Merged
merged 2 commits into from
Apr 2, 2025

Conversation

Fabian-Gruenbichler
Copy link
Contributor

while preparing shim 16.0 for our Debian Bookworm based Debian derivative, I noticed the test_get_variable_0 and test_gnvn_0 tests failing (depending on SHIM_DEBUG value).

taking a closer look with gdb breaking at test-mock-variables.c:310 for the test_get_variable_0 case:

running test_install_config_table_0
test_install_config_table_0: passed
running test_get_variable_0

Breakpoint 1, test_get_variable_0 () at test-mock-variables.c:310
310             for (size_t i = 0; i < n_mok_state_variables; i++) {
(gdb) print mok_rt_vars
$1 = {0x7ffff7f6e9e0 <_IO_helper_jumps> "", 0x7ffff7f73760 <_IO_2_1_stdout_> "\207(\255", <incomplete sequence \373>, 0x0, 0x0, 0x0, 0x20676e696e6e7572 <error: Cannot access memory at address 0x20676e696e6e7572>,
  0x7465675f74736574 <error: Cannot access memory at address 0x7465675f74736574>, 0x6c6261697261765f <error: Cannot access memory at address 0x6c6261697261765f>, 0x736170200a305f65 <error: Cannot access memory at address 0x736170200a305f65>,
  0xa646573 <error: Cannot access memory at address 0xa646573>, 0x0 <repeats 20 times>}
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
__strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:79
79      ../sysdeps/x86_64/multiarch/strlen-evex.S: No such file or directory.
(gdb) bt full
#0  __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:79
No locals.
#1  0x000055555555d2e0 in mock_load_variables (dirname=dirname@entry=0x55555556ab30 "test-data/efivars-1", filters=0x7fffffffbb98, filter_out=filter_out@entry=true) at mock-variables.c:1166
        i = <optimized out>
        spacebuf = "         \000", ' ' <repeats 35 times>
        len = 9
        found = false
        dfd = 3
        d = 0x55555557d2a0
        entry = 0x55555557d300
#2  0x000055555555f4c3 in test_get_variable_0 () at test-mock-variables.c:321
        size = 0
        buf = '\000' <repeats 8191 times>
        status = <optimized out>
        empty_guid = <optimized out>
        attrs = 0
        ret = -1
        cmp = <optimized out>
        mok_rt_vars = {0x55555556a4b9 "MokListRT", 0x55555556a4cc "MokListXRT", 0x55555556a4e2 "MokSBStateRT", 0x55555556a4fa "MokIgnoreDB", 0x55555556a510 "SbatLevelRT", 0x55555556a52b "MokListTrustedRT", 0x55555556a546 "MokPolicyRT",
          0x6c6261697261765f <error: Cannot access memory at address 0x6c6261697261765f>, 0x736170200a305f65 <error: Cannot access memory at address 0x736170200a305f65>}
        __func__ = "test_get_variable_0"
        err = <optimized out>
#3  0x000055555555a717 in main () at test-mock-variables.c:868
        rc = <optimized out>
        status = 0
        policies = {0x55555556aded "MOCK_SORT_DESCENDING", 0x55555556ae02 "MOCK_SORT_PREPEND", 0x55555556ae14 "MOCK_SORT_APPEND", 0x55555556ae25 "MOCK_SORT_ASCENDING", 0x55555556ae39 "MOCK_SORT_MAX_SENTINEL"}

with Debian Trixie the test case passes, because there is no random memory contents at that location (slightly newer GCC 12, but might be some other cofounding factor affecting the memory layout or initialization):

test_gnvn_0: passed
running test_gnvn_1
test_gnvn_1: passed
running test_install_config_table_0
test_install_config_table_0: passed
running test_get_variable_0

Breakpoint 1, test_get_variable_0 () at test-mock-variables.c:310
310             for (size_t i = 0; i < n_mok_state_variables; i++) {
(gdb) print mok_rt_vars
$1 = {0x0 <repeats 30 times>}

in case there won't be any non-CONFIG_ONLY vars added later in the array, simple setting the first skipped entry to NULL and breaking the loop should also work.

In test-mock-variables.c, we create a list of names to filter against,
but we skip entries that have MOK_VARIABLE_CONFIG_ONLY set.  This leaves
the list with uninitialized strings, which segfaults (depending on
compiler versions) when mock_load_variables() tries to compare the names
later.

This patch changes the consumers of mock_load_variables() to initialize
the filter name to an empty (but terminated) string instead.  Since
mock_load_variables() is strictly called down the call stack from any
case doing this, we can assign them to an empty string on the stack
safely, and that will cause the comparison to correctly behave.

Signed-off-by: Fabian Grünbichler <[email protected]>
@vathpela vathpela force-pushed the mr/fix-test-segfaults branch from efdf69c to 5ee76f1 Compare April 2, 2025 18:43
@vathpela
Copy link
Member

vathpela commented Apr 2, 2025

I've moved some of the info from this PR into the commit message so it's easier to understand later, but otherwise this looks good to me, thanks!

@vathpela vathpela merged commit d44405e into rhboot:main Apr 2, 2025
1 check passed
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