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

Remove left-over debug code #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

lindi2
Copy link

@lindi2 lindi2 commented Jun 30, 2020

Without this change the code will always assume has_base_relative=True
which will break kernels that are not built with
CONFIG_KALLSYMS_BASE_RELATIVE.

Without this change the code will always assume has_base_relative=True
which will break kernels that are not built with
CONFIG_KALLSYMS_BASE_RELATIVE.
@marin-m
Copy link
Owner

marin-m commented Jun 30, 2020

Hello,

Thanks for noticing. Actually the # DEBUG comment was incorrectly left, because this line of the code is needed now: the loop should break here as the kallsyms addresses or symbols have been theoretically correctly found, except if there are at least 20% null addresses (in which case we consider that things look wrong and we should likely switch the "BASE_RELATIVE" parsing mode), thanks the following code:

            if number_of_null_items / len(tentative_addresses_or_offsets) >= 0.2: # If there are too much null symbols we have likely tried to parse the wrong integer size
                
                if can_skip:
                    continue

When I remove the break statement, hence, certain 64-bit kernels of my test suite are incorrectly considered as BASE_RELATIVE kernels (as there is an unwanted fall-through towards the next for iteration, which should normally happen only when the 20% null values threshold triggers the continue statement above OR when the BASE_RELATIVE setting is tested first because the kernel version string suggests that this is a recent Itanium/IA64 kernel, the most common case I think where BASE_RELATIVE is enabled).

If you have encountered a BASE_RELATIVE-compiled kernel that is not correctly handled by vmlinux-to-elf currently, can you please share it so that we can try to update the code for its correct handling? Otherwise, can you share details about it (architecture, version strings, possible source, etc.)?

Thank you,

@lindi2
Copy link
Author

lindi2 commented Jun 30, 2020

Hmm, I'm afraid I cannot share the kernel at the moment.

Currently the code says that there are 99.9% negative offsets and 0.001% null addresses. Then it proceeds to generate symbols that have 9 hex digits in them (like 101377757).

The comment on the line you mention talks about integer size. Is it really intended to detect CONFIG_KALLSYMS_ABSOLUTE_PERCPU?

@lindi2
Copy link
Author

lindi2 commented Jun 30, 2020

I don't know if it helps but the addresses are all 32-bit and between 0xc0100000 and 0xc1480000.

@marin-m
Copy link
Owner

marin-m commented Jun 30, 2020

The comment on the line you mention talks about integer size. Is it really intended to detect CONFIG_KALLSYMS_ABSOLUTE_PERCPU?

The trickiness here is that in 64-bit kernels, offsets (used when BASE_RELATIVE is set) are encoded as 32-bits integers, while addresses (used when BASE_RELATIVE is unset) are encoded as 64-bits integers. Hence the issue of having ranges of null bytes instead of actual integer values when, because BASE_RELATIVE is guessed wrong, the integer size of values is guessed wrong.

Currently the code says that there are 99.9% negative offsets and 0.001% null addresses. Then it proceeds to generate symbols that have 9 hex digits in them (like 101377757).

This may look like compressed symbols name from the "kallsyms_names" array (see https://github.com/marin-m/vmlinux-to-elf#how-does-it-work-really), comprising ASCII characters as part of their bytes, and can be an idea of something to detect for extending the heuristic.

Hmm, I'm afraid I cannot share the kernel at the moment.

If still you wish to share the kernel privately, you can send it at viveleyaourt AT laposte DOT net (temporary address).

Regards,

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.

3 participants