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

Implementing a TBF Parser #6

Open
george-cosma opened this issue Jul 21, 2023 · 2 comments
Open

Implementing a TBF Parser #6

george-cosma opened this issue Jul 21, 2023 · 2 comments

Comments

@george-cosma
Copy link
Contributor

Motivation

One aspect of tockloader that should be implemented early on is a TBF Header parser. Thankfully, we don't need to start at square one. Tock already has a no-std parser implemented in the form tock-tbf. This would effectively allow us to read the headers without much hassle. There's also elf2tab, which implements a modifiable TBF Header.

Now, if we were to create a 3rd implementation for what is basically the same thing, we would be doing ourselves a disservice. On one hand, we could just use tock-tbf (with possibly a few modifications, i'll get into why later) and be done with it, or try to merge the elf2tab and tock-tbf implementation into a single package that will always allow for parsing, and even editing if we have access to std. This would also open up the possibility of a unified codebase.

Differences

Elf2tab and tock-tbf are different by nature. Most obviously, elf2tab needs std to function. The main culprit for the use of std is the use vectors instead of arrays:

  • elf2tab
#[repr(C)]
#[derive(Debug)]
struct TbfHeaderPersistentAcl {
    base: TbfHeaderTlv,
    write_id: u32,
    read_length: u16,
    read_ids: Vec<u32>,
    access_length: u16,
    access_ids: Vec<u32>,
}
  • tock-tbf
/// A list of persistent access permissions
#[derive(Clone, Copy, Debug)]
pub struct TbfHeaderV2PersistentAcl<const L: usize> {
    write_id: Option<core::num::NonZeroU32>,
    read_length: u16,
    read_ids: [u32; L],
    access_length: u16,
    access_ids: [u32; L],
}

Of course, this makes sense, as when we are reading a header we are able to know ahead of time how much memory we will need ahead of time since the length of them are constant (in the case above, L will be set to a constant: reference), or we know its length via a field, like in the case of the footer (reference). If we want to make the header editable, the simples solution is to use a datatype which can expand in size - hence the use of vectors.

Also, the structs are slightly different. If elf2tab and the documentation both say that the type and length of a TLV header (type, length, value) are to be contained together in one struct. Tock-tbf first reads the type and length into its own struct, and the actual value into its own type of struct, based on the read type.

  • tock-tbf:
#[derive(Clone, Copy, Debug)]
pub struct TbfHeaderV2Main {
    init_fn_offset: u32,
    protected_trailer_size: u32,
    minimum_ram_size: u32,
}
  • documentation:
struct TbfHeaderMain {
    base: TbfHeaderTlv,
    init_fn_offset: u32,         // The function to call to start the application
    protected_trailer_size: u32, // The number of app-immutable bytes after the header
    minimum_ram_size: u32,       // How much RAM the application is requesting
}

An odd issue

If we are to use tock-tbf as-is, we can notice that it is a little picky about what it can parse: only static-lifetime byte arrays (reference). It's a bit inconvenient, that's for sure. We don't know bytes the of the app(s) ahead of time, and not even the size. So the way I see it, we have to Box::leak() the flash. Not a serious issue since I don't think we need to keep tockloader open for a long period of time in the situation we need to list all the apps, or something minor like that. But still, it could be a hindrance in the future.

From what I can see, tock-tbf needs the static lifetime for the footer. The other headers are not dependent on it. I don't see why the lifetime couldn't be coupled to that of the struct, but perhaps I am missing something.

Action plans

With all this being said, here's how I view the situation and what we can do:

  1. Don't try and standardize, just create something just for tockloader. (Relevant xkcd)
  2. Implement two types of structs: one that are read-only and don't require std, and read/write structs that are implemented using std.
    • Pros:
      • Easier to implement: Besides the TLV header mismatch, we just copy the implementation code for parsing and editing, and implement a way to transform a read-only struct to a r/w struct and vice versa. The static still needs to be addressed.
      • Easy to adapt tock/elf2tab: Besides a few modification to the names of the structs, there isn't much that needs to be done to integrate this into tock or elf2tab.
    • Cons:
      • Possibly has a lot of code repetition
  3. Base implementation on tock-tbf structs (adapt to docs' structure), and adapt elf2tab edit function to work on them, and if std is enabled.
    • Pros:
      • Cleaner code: We don't have to juggle two types of structs, just one. What changes is what functions are implemented based on the presence of std or not.
      • Easier to use: The user doesn't need to worry about switching to read or r/w.
    • Cons:
      • Harder to implement
      • Harder to adapt tock/elf2tab

If it weren't for the TLV header type mismatch, I would optimistically support the second plan. As it stands, the two libraries are hard to merge neatly, and both will need modifications if we want a standardized library. Even if we disregard this notion of standardization, if we want to both parse and modify headers, we need to copy bits of code from each one. Might as well go the extra mile.

What do you people think? How should we tackle this issue?

@bradjc
Copy link

bradjc commented Jul 21, 2023

I would vote to see the tock-tbf library expanded and used for both the kernel and this. If it can also be used for elf2tab that is a bonus, but I don't think that is a very important concern. I also think elf2tab could just have a limited number of storage permission ids to match the kernel.

There may be cases in tock-tbf where things like lifetimes were chosen not necessarily specifically but just because they worked. But a more general version that still works for the kernel is fine with me.

If there are still incompatibilities, where something works for the kernel but is not suitable for tockloader-rs, I think it would be fine to have code in tock-tbf library that was only intended for tockloader-rs and was not intended for the kernel. That is to say, I think it is ok if there is code in tock-tbf that wasn't designed for the kernel. But, I think it should still be no std. And it would be better to avoid this case if possible.

@george-cosma
Copy link
Contributor Author

I think I can get behind this idea. Though I'm not sure yet what tockloader would precisely need - so it's hard for me to say if all of the code can be non-std. Worst case, we setup the library such that std (and the functions that need it) can be enabled using a feature flag. Though, as a hunch, editing anything besides the credential footers should work fine without std, but I guess we'll see that as the code is written.

I'll start looking at what the python version of tockloader did with the headers, to see what needs to be implemented.

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

No branches or pull requests

2 participants