Skip to content

Add support for kernel info #177

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

Closed

Conversation

Andy-Python-Programmer
Copy link

Fixes: #164

@Andy-Python-Programmer
Copy link
Author

cc @phil-opp?

Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request and sorry for the late review!

Thinking more about this, I'm not sure if the kernel_base and kernel_size fields really make sense since not all kernels are continous in virtual memory. Perhaps you can clarify how these fields are required for implementing unwinding? Perhaps we can find a better solution.

@@ -246,6 +247,13 @@ where
None
};

let kernel_info = KernelInfo {
kernel_base: kernel_bytes.as_ptr() as u64,
Copy link
Member

Choose a reason for hiding this comment

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

This is the physical memory address of the kernel ELF file. To get the virtual address at which the kernel is mapped, we need to look at the segments in the program header of the ELF file. Note that there is no guarantee that these segments are continuous in memory since each section can be mapped arbitrarily using a linker script.

@@ -246,6 +247,13 @@ where
None
};

let kernel_info = KernelInfo {
kernel_base: kernel_bytes.as_ptr() as u64,
kernel_size: kernel_bytes.len() as u64,
Copy link
Member

Choose a reason for hiding this comment

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

This is the size of the kernel ELF file, not the size that the kernel occupies in memory. Also, as noted in the previous comment, the kernel segments can be spread across the whole virtual address space, so I'm not sure if we can even choose a proper kernel_size for all kernels.

@Andy-Python-Programmer
Copy link
Author

Andy-Python-Programmer commented Jun 14, 2021

Thinking more about this, I'm not sure if the kernel_base and kernel_size fields really make sense since not all kernels are continous in virtual memory. Perhaps you can clarify how these fields are required for implementing unwinding? Perhaps we can find a better solution.

@phil-opp Basically in stack unwinding we load the kernel bytes from the provided kernel_base and kernel_size then just ElfFile::new(kernel_slice) and then we walk through the RBP and

for data in symbol_table { // symbol table got from the symtab elf section
    let st_value = data.value() as usize;
    let st_size = data.size() as usize;

    if rip >= st_value && rip < (st_value + st_size) { 
        let mangled_name = data.get_name(&kernel_elf);
        ... 
    }
} // basically what you could do using `readelf` and include it using `include_bytes` but this is runtime thinggy

This is roughly how you would do this

@Andy-Python-Programmer
Copy link
Author

Andy-Python-Programmer commented Jun 16, 2021

@phil-opp a better alternative solution would be to pass the symbol tab (symtab) section in the KernelInfo itself as we are loading the ELF in the bootloader right (so we would save, an ELF load on unwind)? What we actually are looking for is the kernel slide.
Slide that the bootloader applied over the kernel's load address as a positive offset.

@ethindp
Copy link
Contributor

ethindp commented Jun 16, 2021 via email

@bjorn3
Copy link
Contributor

bjorn3 commented Jun 16, 2021

The symbol table can be manually stripped, but it is kept by default. It is not part of the debuginfo but required by the linker to do it's job.

@phil-opp
Copy link
Member

phil-opp commented Aug 1, 2021

@Andy-Python-Programmer Sorry for the delay, the past two months were quite busy for me.

Basically in stack unwinding we load the kernel bytes from the provided kernel_base and kernel_size then just ElfFile::new(kernel_slice)

Thanks for the explanation, I think I understand it now. However, I'm not sure if it would work as implemented. The problem is that I'm not sure if we keep the unused (i.e. not loaded) parts of the ELF file reserved in physical memory after switching to the kernel. And even if we do currently, we might want to optimize this in the future.

So it would probably be better to add a config option for keeping the ELF file in physical memory after loading (similar to the existing config options). Only if it is enabled, we could fill an Optional kernel_elf_address field with the physical address range of the ELF file (and reserve it in the physical memory map if we don't do it already).

The stack_top and stack_size field are independent of this and can be always set. So they should probably go into a different field. We should also note in the docs that these describe a virtual address range.

@phil-opp phil-opp marked this pull request as draft January 2, 2023 11:37
@phil-opp
Copy link
Member

There is a new PR that implements something similar: #346

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.

Pass UnwindInfo in BootInfo
4 participants