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

Can TD-Shim skip relocations for the ELF payload? #420

Open
dimakuv opened this issue Oct 4, 2022 · 12 comments · May be fixed by #424
Open

Can TD-Shim skip relocations for the ELF payload? #420

dimakuv opened this issue Oct 4, 2022 · 12 comments · May be fixed by #424
Assignees

Comments

@dimakuv
Copy link

dimakuv commented Oct 4, 2022

I am developing my ELF payload that TD-Shim loads.

My ELF payload is a generic OS kernel that can be started in different ways. One of these ways is as an ELF payload via the TD-Shim.

Therefore, my ELF payload has a generic startup sequence that has the step of self-relocation. In other words, one of the first things my ELF payload does is to find all relocations (by reading its own ELF header and metadata) and apply them.

I am confused by the TD-Shim linker logic:

  • On the one hand, td-shim-ld has an option --relocate-payload. So I skip this option, which should mean relocate-payload = false and thus self.payload_relocation == false, which should not perform any relocations in my ELF payload:

    if self.payload_relocation {
    let mut payload_reloc_buf = vec![0x0u8; MAX_PAYLOAD_CONTENT_SIZE];
    let reloc = pe::relocate(
    &payload_bin.data,
    &mut payload_reloc_buf,
    TD_SHIM_PAYLOAD_BASE as usize + payload_header.data.len(),
    )
    .ok_or_else(|| {
    io::Error::new(io::ErrorKind::Other, "Can not relocate payload content")
    })?;
    trace!("shim payload relocated to 0x{:x}", reloc);
    output_file.write(&payload_reloc_buf, "payload content")?;
    } else {

  • On the other hand, td-shim-ld seems to relocate the ELF payload anyway, disregarding the value of --relocate-payload:

    let mut ipl_reloc_buf = vec![0x00u8; MAX_IPL_CONTENT_SIZE];
    // relocate ipl to 1M
    let reloc = elf::relocate_elf_with_per_program_header(
    &ipl_bin.data,
    &mut ipl_reloc_buf,
    0x100000 as usize,
    )
    .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "Can not relocate IPL content"))?;

Maybe I don't understand the difference between payload and ipl? Is there a way to skip relocations of my ELF payload?

@gaojiaqi7
Copy link
Member

td-shim relocates the payload at runtime unconditionally. We can add a feature flag to disable the relocation at runtime

@dimakuv
Copy link
Author

dimakuv commented Oct 10, 2022

@gaojiaqi7 Thanks for reply. Yes, adding such a flag would be nice.

@gaojiaqi7
Copy link
Member

hi @dimakuv Can you take a try with the #424 so that I can make sure the change is good

@dimakuv
Copy link
Author

dimakuv commented Oct 11, 2022

Thanks @gaojiaqi7! I'll try this PR tomorrow.

@dimakuv
Copy link
Author

dimakuv commented Oct 12, 2022

@gaojiaqi7 Got this error after updating to your #424 branch:

$ cargo run -p td-shim-tools --features="linker" --no-default-features \
    --bin td-shim-ld -- \
    target/x86_64-unknown-uefi/release/ResetVector.bin \
    target/x86_64-unknown-uefi/release/td-shim.efi \
    -p <my ELF payload> \
    -o target/x86_64-unknown-uefi/release/final-elf.bin

    ...
   Compiling td-shim v0.1.0 (/home/dimakuv/td-shim/td-shim)
   Compiling td-shim-tools v0.1.0 (/home/dimakuv/td-shim/td-shim-tools)
    Finished dev [unoptimized + debuginfo] target(s) in 1.01s
     Running `target/debug/td-shim-ld target/x86_64-unknown-uefi/release/ResetVector.bin target/x86_64-unknown-uefi/release/td-shim.efi -p <my ELF payload> -o target/x86_64-unknown-uefi/release/final-elf.bin`
Error: Custom { kind: Other, error: "Can not relocate IPL content" }

Note that I didn't yet apply the disable-relocation feature (wanted to test without this flag first). What could this mean?

@gaojiaqi7
Copy link
Member

I can't reproduce this error with the example td-payload. Can you try to recompile the td-shim? we changed its target has to x86_64-unknown-none recently

@dimakuv
Copy link
Author

dimakuv commented Oct 12, 2022

Ok, a clean rebuild fixed this issue.

My payload works fine without the disable-relocation feature (so, with defaults). For this to work, I modified my payload to not perform relocations by itself.

But when I tried --features="disable-relocation" and with my payload unmodified, everything started silently failing. (I will need to debug this tomorrow, just giving you a quick reply now.)

I have a stupid question: where do I need to add this feature? Here's roughly how I tested:

git clone https://github.com/gaojiaqi7/td-shim.git
git checkout td-loader --

git submodule update --init --recursive
 ./sh_script/preparation.sh

cargo xbuild -p td-shim --target x86_64-unknown-none --release --features=main,tdx,disable-relocation --no-default-features

cargo run -p td-shim-tools --features="linker" --no-default-features --bin td-shim-ld -- \
    target/x86_64-unknown-none/release/ResetVector.bin \
    target/x86_64-unknown-none/release/td-shim \
    -p <my ELF payload> \
    -o target/x86_64-unknown-none/release/final.bin

Is this sequence correct?

@gaojiaqi7
Copy link
Member

Yes, it is correct. The feature is set to td-shim

@gaojiaqi7
Copy link
Member

Therefore, my ELF payload has a generic startup sequence that has the step of self-relocation. In other words, one of the first things my ELF payload does is to find all relocations (by reading its own ELF header and metadata) and apply them.

@dimakuv how does your ELF payload read its own ELF header and metadata, using offset or something else? Dose the payload needs loader to map its segments to the specified virtual address?

@dimakuv
Copy link
Author

dimakuv commented Oct 13, 2022

@gaojiaqi7 The ELF payload reads its own ELF header and metadata using a global symbol that is located at the very start of the ELF file image (and thus points to the ELF header).

Similar to this code:

There is no need for the loader to map the segments to the specific virtual address. The address can be anything.

@gaojiaqi7
Copy link
Member

Hi @dimakuv I updated the branch today, can you try again with the latest?

@dimakuv
Copy link
Author

dimakuv commented Oct 18, 2022

@gaojiaqi7 No, still the same problem. And I still didn't have time to debug it fully.

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 a pull request may close this issue.

2 participants