Linux Module finder is broken, also crashes when creating stack trace

[JodiTheTigger]

Description

Crashes in get_code_id_from_text_fallback when sentry_value_new_stacktrace(nullptr, 0) is called

But also, whenever a debug symbol hash is calculated for any module that doesn't have a build-id

Caused by #431

Fundamental cause of this issue, is that a module is read from memory, instead of from the actual file (pre #431), and the mapped memory does not point to the assumed data. The crash happens when the offset for the section headers just happens to match one of the section's mappings in the module itself, so rubbish data is loaded.

This is exactly as the pre #431 code warned:

    // At least on the android API-16, x86 simulator, the linker apparently
    // does not load the complete file into memory. Or at least, the section
    // headers which are located at the end of the file are not loaded, and
    // we would be poking into invalid memory. To be safe, we mmap the complete
    // file from disk, so we have the on-disk layout, and are independent of how
    // the runtime linker would load or re-order any sections. The exception
    // here is the linux-gate, which is not an actual file on disk, so we
    // actually poke at its memory.

sentry_modulefinder_linux.c
get_code_id_from_text_fallback

        for (int i = 0; i < elf.e_shnum; i++) {
            Elf32_Shdr header;
            ENSURE(sentry__module_read_safely(&header, module,
                elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr)));          // <--- header is read from invalid memory, so is rubbish

            const char *name = names + header.sh_name;                            // <--- header.sh_name is rubbish (and is large)
            if (header.sh_type == SHT_PROGBITS && strcmp(name, ".text") == 0) {   // <--- segfault here since name is out of bonds 
                text = sentry__module_get_addr(
                    module, header.sh_offset, header.sh_size);
                ENSURE(text);
                text_size = header.sh_size;
                break;
            }
        }

When does the problem happen

• During build
• During run-time
• When capturing a hard crash

Environment

• OS: Arm32 - Custom Embedded Linux (kernel: 5.10.87 - SMP PREEMPT_RT - armv7l GNU/Linux)
• Compiler: GCC 10.3.0 Crosscompiler
• CMake version and config: 3.22.1 (-DSENTRY_BACKEND=crashpad)
• sentry version 0.4.12

Steps To Reproduce

random, as dependent on loaded modules not having a build-id, and if their elf section headers overlap with one of their internal elf section file mappings

[JodiTheTigger]

note: reverted sentry_modulefinder_linux.c and sentry_modulefinder_linux.h to commit 8a02abf (before #431) and the app no longer crashes when calling sentry_value_new_stacktrace(nullptr, 0)

[Swatinem]

@JodiTheTigger thanks for the very detailed report!
Can you try this with the code in #641? I tried making the read of name safe. So even if it is is invalid for whatever reason, it should not segfault.

I can also try to revert back to mmap-ing as we did before, however I’m a bit hesitant as to not regress the Android use-case with loading libraries directly from .apk files.

[JodiTheTigger]

Heya, I'm now on Christmas break, so I'll try your fixes when back after the 5th of Jan 2022. Have a happy new year!

[JodiTheTigger]

I can confirm that the code in #641 prevents the crashes caused by reading name which is out of bounds.

---

However, the fundamental issue that for modules without a build-id, the section headers are not read properly still exists. The end result is that the modules are not recognised, and the callstack will not have any symbols. As stated before this is due to moving to reading memory as opposed to the actual files for the section headers.

I'm hardly and expert in this area, so these are only suggestions on how to possibly fix this issue. I'll happily defer to those who know more about this stuff.

• compile time flag to choose file based or memory based module reading code
  • Maybe based on the target OS? so Android, keep the new code, but linux use the old code?
• Code to detect "invalid" section headers, and fall back to reading from file if that's the case
• Some other unknown way to get section headers from memory that we don't know about?

[Swatinem]

@JodiTheTigger since I think you can reproduce this issue reliably, can you also give #642 a shot? That indeed moves back to using mmap for this, and shoud still work for the android usecase.

[JodiTheTigger]

Ok, done and tested, comment left on the MR (tl;dr: doesn't work as file mappings are larger than file mapped (wtf?), so mmaped files are ignored)

[JodiTheTigger]

Ok, I tested the code that was merged to master, and I believe it fixes my issues as described in my comment on the MR of #642

Fee free to close

[Swatinem]

🎉 @JodiTheTigger Thank you very much for the detailed report, and testing the fixes!