Setting load address doesn't work for PVH loader

[rbradford]

Setting the kernel load address doesn't result in the kernel booting (tested with Cloud Hypervisor and setting it to 1MiB.) bzImage loader works fine with the specified load address.

[rbradford]

My guess is you need to patch the code32_start field with the new address.

[alxiord]

Hey @rbradford, I can't seem to get this to repro with Firecracker, can you please check if it got accidentally fixed in the meantime or detail some repro steps if it's still valid?

To fiddle with Firecracker and move the load address around:

• use my linux-loader integration branch https://github.com/aghecenco/firecracker/tree/loader

• change the load address here: https://github.com/aghecenco/firecracker/blob/3c7f2e3266166edcef0430b295fcc4bbccb0cb3f/src/vmm/src/builder.rs#L391

  diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
  index 77e7280..6258109 100644
  --- a/src/vmm/src/builder.rs
  +++ b/src/vmm/src/builder.rs
  @@ -388,7 +388,7 @@ fn load_kernel(
  
       let load_result = Loader::load::<std::fs::File, GuestMemoryMmap>(
           guest_memory,
  -        Some(GuestAddress(arch::get_kernel_start())),
  +        Some(GuestAddress(0x0040_0000)),
           &mut kernel_file,
           // High memory address is x86-specific.
           // In our case, it's the same as the kernel load point.

• check if this test still passes:

  cargo test test_build_microvm

Or:

git checkout -b firecracker-aghecenco-loader
git pull https://github.com/aghecenco/firecracker.git loader
# change kernel_start in src/vmm/src/builder.rs...
tools/devtool shell -p
cargo test test_build_microvm

[alxiord]

Fiddled with cloud-hypervisor too, trying out various addresses here
https://github.com/cloud-hypervisor/cloud-hypervisor/blob/83c18de56abd4a428d133f402313f21f8c3b4436/vmm/src/vm.rs#L498
with the default image, and it booted every time. Will close as I can't repro, @rbradford if you see it again please reopen.

[josephlr]

I think @rbradford might have been trying this with the rust-hypervisor-firmware. For that binary, setting a load address other than None will prevent booting.

However, that's not a bug in linux-loader. The firmware ELF binary is not arbitrarily relocatable, it must be loaded at the addresses specified in the PHDRs. So I think this is fine to close. Trying to relocate a non-relocatable binary (by setting the load address) should fail, and that's fine.

[rbradford]

My mistake, the issue is seen with the PVH loader rather than the "vanilla" ELF loader. If you apply the following patch to CH: https://github.com/rbradford/cloud-hypervisor/tree/2020-05-26-kernel-loading-debugging

And ensure CONFIG_PVH=y in your kernel configuration you see the following output:

cloud-hypervisor: 109.781806ms: INFO:vmm/src/vm.rs:497 -- Loading kernel at 0x200000
cloud-hypervisor: 157.77579ms: INFO:vmm/src/vm.rs:597 -- Using boot protocol: PVH boot protocol with start address 0x1000470 

But without PVH:

cloud-hypervisor: 106.164703ms: INFO:vmm/src/vm.rs:497 -- Loading kernel at 0x200000
cloud-hypervisor: 128.539535ms: INFO:vmm/src/vm.rs:597 -- Using boot protocol: Linux 64-bit boot protocol with start address 0x1200000 

It's quite possible that the way i'm testing using a different load address is wrong or it could in fact be a Cloud Hypervisor issue.

[rbradford]

I'm all in favour of closing this if we decide it's a Cloud Hypervisor issue. But the point i'm trying to make is that that load_address parameter isn't clear if there are particular caveats about its use.

[aljimenezb]

I'm taking a look.

[aljimenezb]

The short answer is that this is not a Cloud Hypervisor issue, and for the PVH entry point to work correctly the kernel must be initially loaded in guest memory at the default location.
This default is typically 0x1000000 (16MB) and is specified by the CONFIG_PHYSICAL_START setting when building the kernel. (https://cateee.net/lkddb/web-lkddb/PHYSICAL_START.html).

I am not sure yet if it is possible to make the PVH entry point work when the kernel is loaded at a different location than the one specified at build time, but it would require Linux kernel changes.

For quickly fixing this issue I propose a patch that simply does not return a PVH entry point as an option if a kernel_start offset has been requested by the VMM.

diff --git a/src/loader/x86_64/elf/mod.rs b/src/loader/x86_64/elf/mod.rs
index 1b319d7..d89217e 100644
--- a/src/loader/x86_64/elf/mod.rs
+++ b/src/loader/x86_64/elf/mod.rs
@@ -212,8 +212,10 @@ impl KernelLoader for Elf {
         // Read in each section pointed to by the program headers.
         for phdr in phdrs {
             if phdr.p_type != elf::PT_LOAD || phdr.p_filesz == 0 {
-                if phdr.p_type == elf::PT_NOTE {
+                if phdr.p_type == elf::PT_NOTE && kernel_start.is_none() {
                     // This segment describes a Note, check if PVH entry point is encoded.
+                    // This is only useful if an offset from the default kernel load address
+                    // has not been requested using the kernel_start parameter.
                     loader_result.pvh_entry_addr = parse_elf_note(&phdr, kernel_image)?;
                 }
                 continue;

[aljimenezb]

That being said we should discuss the kernel_start parameter and whether it has any useful applications. I can see that it was inherited from the crosvm code, but I am not familiar with that project or its use cases. I'm not trying to use this as a straw man (as I mentioned above PVH does not support this feature and cannot be used with it), but since we have several eyes on this issue now it is a good opportunity to get things fixed if needed.

One detail I noticed is that the description of the kernel_start parameter is misleading.
It does not specify the address in guest memory where the kernel is loaded, but rather it provides an offset that is added to the default load address of the kernel in guest memory (determined at kernel build time by the value of CONFIG_PHYSICAL_START).
So a VMM requesting to use this parameter must already have an idea of the default load address of the kernel, which means:

• assuming the default load address of 16MB is used
• asking the user to provide it
• using functionality like the one present in this crate to parse the ELF header and read the e_entry field

Following from the above, it is not clear to me what this functionality buys us in a real use case scenario. I'd appreciate if someone with more experience in this area could give an example of when a VMM would need to use this functionality of loading the guest kernel at a non-default location in guest memory.

[aljimenezb]

Ping? I can fix the issue by defaulting to use the Linux boot protocol when kernel_start is Some offset, but I am wondering why do we have this parameter at all...

This functionality might have been needed by crosvm (how/why?), but with my limited experience I cannot think of a current use case or need for it.

If we can't or don't want to have that discussion right now, then please let me know and I'll just fix the bug.

[alexandruag]

Hi, this is a good question and an interesting conversation. You are right about kernel_start being an offset instead of an absolute address. I also don't have a good real use case example for x86. At the same time the KernelLoader trait is shared by all implementations, including stuff like pe for aarch64, and I'm not sure how things stand there. I'll dig a bit more into it the following days, and maybe other ppl chime in as well.

[aljimenezb]

Thank you for the ack, and for looking into this. I'm completely unfamiliar with aarch64, so perhaps there is a good reason there.
On the x86 side, I think that using kernel_start provides a similar functionality to what KASLR does by randomizing the physical address at which the kernel is placed after the decompression stage. I'm not sure how much value this adds in terms of security (IIUC, the virtual address space will still be randomized even if the physical load address is not). That is my best case so far, but I'd like to hear from others.

[alxiord]

According to https://www.kernel.org/doc/Documentation/arm64/booting.txt:

The Image must be placed text_offset bytes from a 2MB aligned base
address anywhere in usable system RAM and called there. The region
between the 2 MB aligned base address and the start of the image has no
special significance to the kernel, and may be used for other purposes.
At least image_size bytes from the start of the image must be free for
use by the kernel.
NOTE: versions prior to v4.6 cannot make use of memory below the
physical offset of the Image so it is recommended that the Image be
placed as close as possible to the start of system RAM.

text_offset is specified in the Image header, and as far as I understand the user is free to load the Image anywhere in RAM, as long as it fits and it's offset by text_offset from a valid address. So it looks like kernel_start is relevant on aarch64.

@crab2313 @michael2012z could you confirm/infirm?

In the meantime @aljimenezb would you submit a PR with the patch and explanation?

[aljimenezb]

If I correctly understand your comment and the KernelLoader implementation for PE, then kernel_start is the "2 MB aligned base address" mentioned above. Since kernel_start is provided by the VMM, we should verify that it meets the proper alignment, so I'll post a simple patch to do that.

So our kernel_start plays different roles in ARM vs x86. I'm still not convinced that the physical address offset functionality it provides in x86 brings any security benefits, since it moves the whole kernel by a fixed offset in phys address space, and a single info leak is enough to determine what that offset is (IIUC this is the problem with KASLR).

I suppose it does not hurt to use Linux boot protocol instead of PVH if we want to keep kernel_start around, but I still think this is essentially code bloat in the load() implementation for Elf. It seems unlikely that any VMM will use/need it and it is only being exercised by code coverage tests...

In the meantime @aljimenezb would you submit a PR with the patch and explanation?

Will do soon.

[michael2012z]

@aghecenco , I share the same understanding with @aljimenezb on kernel_start , it is the "2 MB aligned base address". In Firecracker it is "2G", an absolute address in guest memory: https://github.com/firecracker-microvm/firecracker/blob/master/src/arch/src/aarch64/layout.rs#L53.