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

Update ELF metadata #88

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "nanos_sdk"
version = "0.3.0"
version = "0.3.1"
authors = ["yhql"]
edition = "2021"

Expand Down
23 changes: 23 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,29 @@
use std::path::PathBuf;
use std::{env, error::Error};

fn extract_defines_from_cx_makefile(command: &mut cc::Build, makefile: &String) {
let mut makefile = File::open(makefile).unwrap();
let mut content = String::new();
makefile.read_to_string(&mut content).unwrap();
// Extract the defines from the Makefile.conf.cx.
// They all begin with `HAVE` and are ' ' and '\n' separated.
let mut defines = content
.split('\n')
.filter(|line| !line.starts_with('#')) // Remove lines that are commented
.flat_map(|line| line.split(' ').filter(|word| word.starts_with("HAVE")))
.collect::<Vec<&str>>();

// do not forget NATIVE_LITTLE_ENDIAN
let s = String::from("NATIVE_LITTLE_ENDIAN");
defines.push(s.as_str());

// Add the defines found in the Makefile.conf.cx to our build command.
for define in defines {
// scott could use for_each
command.define(define, None);
}
}

fn main() -> Result<(), Box<dyn Error>> {
enum Device {
NanoS,
Expand Down
14 changes: 12 additions & 2 deletions link.ld
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ SECTIONS
_estack = ABSOLUTE(END_STACK);
} > SRAM

.ledger.api_level (INFO): { KEEP(*(.ledger.api_level)) }

.stack_sizes (INFO):
{
KEEP(*(.stack_sizes));
Expand All @@ -63,6 +61,18 @@ SECTIONS
*(.ARM.exidx* .gnu.linkonce.armexidx.*)
*(.debug_info)
}

ledger.target (INFO): { KEEP(*(ledger.target)) }
yogh333 marked this conversation as resolved.
Show resolved Hide resolved
ledger.target_id (INFO): { KEEP(*(ledger.target_id)) }
ledger.target_name (INFO): { KEEP(*(ledger.target_name)) }
ledger.app_name (INFO): { KEEP(*(ledger.app_name)) }
ledger.app_version (INFO): { KEEP(*(ledger.app_version)) }
ledger.api_level (INFO): { KEEP(*(ledger.api_level)) }
ledger.sdk_version (INFO): { KEEP(*(ledger.sdk_version)) }
ledger.rust_sdk_version (INFO): { KEEP(*(ledger.rust_sdk_version)) }
ledger.rust_sdk_name (INFO): { KEEP(*(ledger.rust_sdk_name)) }
ledger.sdk_name (INFO): { KEEP(*(ledger.sdk_name)) }
ledger.sdk_hash (INFO): { KEEP(*(ledger.sdk_hash)) }
}

PROVIDE(_nvram = ABSOLUTE(_nvram_start));
Expand Down
94 changes: 92 additions & 2 deletions src/infos.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
/// Expose the API_LEVEL
#[link_section = ".ledger.api_level"]
const MAX_METADATA_STRING_SIZE: usize = 32;

const fn copy_str_to_u8_array(input: &str) -> [u8; MAX_METADATA_STRING_SIZE] {
let mut result = [0u8; MAX_METADATA_STRING_SIZE];
let bytes = input.as_bytes();
let mut count = 0u32;

// Ensure we don't copy more than MAX_METADATA_STRING_SIZE
let min_length = core::cmp::min(bytes.len(), MAX_METADATA_STRING_SIZE - 1);
// Copy bytes to array
loop {
if count >= min_length as u32 {
result[count as usize] = '\n' as u8;
break;
}
result[count as usize] = bytes[count as usize];
count += 1;
}
result
}

/// Expose the API_LEVEL to the app
#[used]
static API_LEVEL: u8 = if cfg!(target_os = "nanos") {
0
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be update to with proper value knwon at build time ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #96

Expand All @@ -10,3 +30,73 @@ static API_LEVEL: u8 = if cfg!(target_os = "nanos") {
} else {
0xff
};

#[used]
#[link_section = "ledger.api_level"]
static ELF_API_LEVEL: [u8; 4] = if cfg!(target_os = "nanos") {
*b"0\n\0\0"
} else if cfg!(target_os = "nanox") {
*b"5\n\0\0"
} else if cfg!(target_os = "nanosplus") {
*b"1\n\0\0"
} else {
*b"255\n"
};

Comment on lines +34 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be update now that API_LEVEL is now at build time from SDK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #96

#[used]
#[link_section = "ledger.target"]
static ELF_TARGET: [u8; 8] = if cfg!(target_os = "nanos") {
*b"nanos\n\0\0"
} else if cfg!(target_os = "nanox") {
Comment on lines +48 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there solutions to avoid these trailing "\n\0"? It seems a bit weird having them everywhere as I guess htey won't be used by the tools reading elf sections?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello,

We could use the following code:

let mut elf_target: [u8; 8] = [0u8; 8];
let _ = &mut elf_target[..5].copy_from_slice("nanos".as_bytes());   

but cannot be called to define static const 😞 (and not sure it enhances readability)

Copy link
Contributor

Choose a reason for hiding this comment

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

So I propose to keep it as it is currently (until better proposal)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering how these trailing \n and \0 will affect our tools reading metadata? Mostly Speculos and LedgerBlue for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in #96

*b"nanox\n\0\0"
} else if cfg!(target_os = "nanosplus") {
*b"nanos2\n\0"
} else {
*b"unknown\n"
};

#[used]
#[link_section = "ledger.target_id"]
static ELF_TARGET_ID: [u8; 11] = if cfg!(target_os = "nanos") {
*b"0x31100004\n"
} else if cfg!(target_os = "nanox") {
*b"0x33000004\n"
} else if cfg!(target_os = "nanosplus") {
*b"0x33100004\n"
} else {
*b"0xffffffff\n"
};

#[used]
#[link_section = "ledger.target_name"]
static ELF_TARGET_NAME: [u8; 14] = if cfg!(target_os = "nanos") {
*b"TARGET_NANOS\n\0"
} else if cfg!(target_os = "nanox") {
*b"TARGET_NANOX\n\0"
} else if cfg!(target_os = "nanosplus") {
*b"TARGET_NANOS2\n"
} else {
*b"WRONG_TARGET\n\0"
};

#[used]
#[link_section = "ledger.sdk_hash"]
static ELF_SDK_HASH: [u8; 5] = *b"None\n";

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can live with this, but best would be having the hash of the C SDK used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #96

#[used]
#[link_section = "ledger.sdk_name"]
static ELF_SDK_NAME: [u8; 18] = *b"ledger-secure-sdk\n";

Copy link
Contributor

Choose a reason for hiding this comment

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

in case of a nano, with should probably set nanos-secure-sdk as it doesn't use the unified one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #96

#[used]
#[link_section = "ledger.sdk_version"]
static ELF_SDK_VERSION: [u8; 5] = *b"None\n";

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as for the hash, would be best to have the real C SDK version, else that is not a drama.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #96

#[used]
#[link_section = "ledger.rust_sdk_name"]
static ELF_RUST_SDK_NAME: [u8; MAX_METADATA_STRING_SIZE] =
copy_str_to_u8_array(env!("CARGO_PKG_NAME"));

#[used]
#[link_section = "ledger.rust_sdk_version"]
static ELF_RUST_SDK_VERSION: [u8; MAX_METADATA_STRING_SIZE] =
copy_str_to_u8_array(env!("CARGO_PKG_VERSION"));
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![test_runner(testing::sdk_test_runner)]
#![allow(incomplete_features)]
#![feature(generic_const_exprs)]
#![feature(const_cmp)]
Copy link
Contributor

@yogh333 yogh333 Oct 19, 2023

Choose a reason for hiding this comment

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

Did not find this nightly feature in Unstable Book.

  1. How did you find it ?
  2. Where is it used ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yogh333 it's for let min_length = core::cmp::min(bytes.len(), MAX_METADATA_STRING_SIZE - 1);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But with @yhql 's proposed solution for strings, we probably would not need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And for how I found it, I can't remember ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this if we replace this min by a more manual version:

   let min_length = if bytes.len() <= MAX_METADATA_STRING_SIZE - 1 {
        bytes.len()
    } else {
        MAX_METADATA_STRING_SIZE - 1
    };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in #96


#[cfg(target_os = "nanox")]
pub mod ble;
Expand Down
Loading