Skip to content
89 changes: 80 additions & 9 deletions core/embed/io/nrf/stm32u5/nrf_update.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,14 @@
#define IMAGE_HASH_LEN 32
#define IMAGE_TLV_SHA256 0x10

struct image_version {
uint8_t iv_major;
uint8_t iv_minor;
uint16_t iv_revision;
uint32_t iv_build_num;
} __packed;

struct image_header {
uint32_t ih_magic;
uint32_t ih_load_addr;
uint16_t ih_hdr_size; /* Size of image header (bytes). */
uint16_t ih_protect_tlv_size; /* Size of protected TLV area (bytes). */
uint32_t ih_img_size; /* Does not include header. */
uint32_t ih_flags; /* IMAGE_F_[...]. */
struct image_version ih_ver;
nrf_app_version_t ih_ver;
uint32_t _pad1;
} __packed;

Expand Down Expand Up @@ -107,6 +100,75 @@ static int read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
return rc;
}

/**
* Read the image version from the image header.
*
* @param image_ptr pointer to the binary image
* @param out_version Pointer to nrf_app_version_t to receive the version
* @return 0 on success, or a negative errno on failure
*/
static int image_version_read(const uint8_t *image_ptr,
nrf_app_version_t *out_version) {
int rc = -1;

struct image_header *hdr = (struct image_header *)image_ptr;

if (out_version != NULL) {
memcpy(out_version, &hdr->ih_ver, sizeof(nrf_app_version_t));
rc = 0;
}

return rc;
}

/**
* Read the image version from the nRF MCUboot via SMP serial recovery.
*
* @param out_version Pointer to nrf_app_version_t to receive the version
* @return 0 on success, or a negative errno on failure
*/
static int nrf_smp_version_get(nrf_app_version_t *out_version) {
int rc = -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

not much use for the errno here, maybe bool would be enough? applies to image_version_read too.

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 here 8319d25 . Also applied the same approach to "read_image_sha256()" which was the origin for the other 2 functions' refactored.


nrf_reboot_to_bootloader();
nrf_set_dfu_mode(true);

if (smp_image_version_get(out_version)) {
// Success - version string provided via SMP has been decoded and stored
// within "out_version" variable
rc = 0;
}

nrf_reboot();
nrf_set_dfu_mode(false);

return rc;
}

/**
* Comparison of two image versions.
*
* @param v1 Pointer to first nrf_app_version_t
* @param v2 Pointer to second nrf_app_version_t
* @return 0 when equal, 1 when v1 is greater, -1 when v2 is greater
*/
static int version_cmp(const nrf_app_version_t *v1,
const nrf_app_version_t *v2) {
if (v1->major != v2->major) {
return (v1->major < v2->major) ? -1 : 1;
}
if (v1->minor != v2->minor) {
return (v1->minor < v2->minor) ? -1 : 1;
}
if (v1->revision != v2->revision) {
return (v1->revision < v2->revision) ? -1 : 1;
}
if (v1->build_num != v2->build_num) {
return (v1->build_num < v2->build_num) ? -1 : 1;
}
return 0;
}

bool nrf_update_required(const uint8_t *image_ptr, size_t image_len) {
nrf_info_t info = {0};

Expand All @@ -116,7 +178,16 @@ bool nrf_update_required(const uint8_t *image_ptr, size_t image_len) {
systick_delay_ms(500);
try_cntr++;
if (try_cntr > 3) {
Copy link
Contributor

Choose a reason for hiding this comment

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

lets try smp after the first SPI failure, the repeat both 3 times.

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 here 8319d25 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's subject to further changes because of #6188 issue.

// Assuming corrupted image, but we could also check comm with MCUboot
// Can't communicate with the App via SPI, trying SMP serial recovery over
// UART to nRF MCUboot
nrf_app_version_t image_version, smp_version;

if (image_version_read(image_ptr, &image_version) == 0 &&
nrf_smp_version_get(&smp_version) == 0) {
return version_cmp(&image_version, &smp_version) > 0;
}

// Assuming corrupted image
return true;
}
}
Expand Down
14 changes: 14 additions & 0 deletions core/embed/rust/rust_smp.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,24 @@

#include <trezor_types.h>

/// Represents a parsed version string in the format:
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding comments here, please add it for the rest of functions here too, make it doxygen style and add license header while at 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.

Fixed here b12efb3 .

/// "major.minor.patch[.build_num]" where build_num is optional and defaults to
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The comment refers to the third component as "patch" but the struct field is named revision. For consistency, either update the comment to say "major.minor.revision[.build_num]" or explain why the terminology differs from the struct field names.

Suggested change
/// "major.minor.patch[.build_num]" where build_num is optional and defaults to
/// "major.minor.revision[.build_num]" where build_num is optional and defaults to

Copilot uses AI. Check for mistakes.
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 here b12efb3 .

/// 0 if absent.
typedef struct __packed {
uint8_t major;
uint8_t minor;
uint16_t revision;
uint32_t build_num; // optional part after, 0 if absent
} nrf_app_version_t;

bool smp_echo(const char* text, uint8_t text_len);

void smp_reset(void);

/// Fills nrf_app_version_t with the version of the active nRF app image.
/// Returns true on success, false on failure.
bool smp_image_version_get(nrf_app_version_t* out);

void smp_process_rx_byte(uint8_t byte);

bool smp_upload_app_image(const uint8_t* data, size_t len,
Expand Down
40 changes: 39 additions & 1 deletion core/embed/rust/src/smp/api.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{echo, process_rx_byte, reset, upload};
use super::{echo, image_info, process_rx_byte, reset, upload};

use crate::util::from_c_array;

Expand All @@ -14,6 +14,44 @@ extern "C" fn smp_reset() {
reset::send();
}

#[repr(C)]
pub struct NrfAppVersion {
pub major: u8,
pub minor: u8,
pub revision: u16,
pub build_num: u32,
}

/// Get the nRF app version as parsed integer components.
///
/// Sends an SMP request to the nRF device to retrieve the active application
/// image version, then parses the version string into individual numeric
/// components.
///
/// # Arguments
/// * `out` - Pointer to NrfAppVersion structure to be filled. Must not be NULL.
///
/// # Returns
/// `true` if version was successfully retrieved and parsed, `false` otherwise.
#[no_mangle]
extern "C" fn smp_image_version_get(out: *mut NrfAppVersion) -> bool {
if out.is_null() {
return false;
}
match image_info::get_version_numbers() {
Some(v) => {
unsafe {
(*out).major = v.major;
(*out).minor = v.minor;
(*out).revision = v.revision;
(*out).build_num = v.build_num;
}
true
}
None => false,
}
}

#[no_mangle]
extern "C" fn smp_upload_app_image(
data: *const cty::uint8_t,
Expand Down
202 changes: 202 additions & 0 deletions core/embed/rust/src/smp/image_info.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
use super::{
receiver_acquire, receiver_release, send_request, wait_for_response, MsgType, SmpBuffer,
SmpHeader, SMP_CMD_ID_IMAGE_STATE, SMP_GROUP_IMAGE, SMP_HEADER_SIZE, SMP_OP_READ,
};
use crate::time::Duration;
use minicbor::{data::Type, decode, Decoder, Encoder};

/// MCUboot-compatible version structure matching image header format
#[derive(Clone, Copy, Debug)]
pub struct AppVersion {
pub major: u8,
pub minor: u8,
pub revision: u16,
pub build_num: u32,
}

/// Macro to generate overflow-safe decimal parsers for different integer types
macro_rules! make_parser {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use let major = parts.next()?.parse::<u8>().ok()?; instead of these custom macro-generated functions?

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 here 355574e .

($name:ident, $target:ty, $accumulator:ty) => {
fn $name(seg: &str) -> Option<$target> {
if seg.is_empty() {
return None;
}
let mut val: $accumulator = 0;
for b in seg.as_bytes() {
if *b < b'0' || *b > b'9' {
return None;
}
val = val
.checked_mul(10)?
.checked_add((b - b'0') as $accumulator)?;
if val > <$target>::MAX as $accumulator {
return None;
}
}
Some(val as $target)
}
};
}

// Generate the three parser functions
make_parser!(parse_u8, u8, u16);
make_parser!(parse_u16, u16, u32);
make_parser!(parse_u32, u32, u64);

/// Parse "<major>.<minor>.<revision>[.<build>]" into AppVersion (no allocation)
/// Matches MCUboot image header version format
/// Examples: "1.0.3", "1.0.3.0", "255.255.65535.4294967295"
fn parse_app_version(s: &str) -> Option<AppVersion> {
let mut parts = s.split('.');

let major = parse_u8(parts.next()?)?;
let minor = parse_u8(parts.next()?)?;
let revision = parse_u16(parts.next()?)?;

// Build number is optional (defaults to 0 if absent)
let build_num = match parts.next() {
Copy link
Contributor

Choose a reason for hiding this comment

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

somewhat more rustier version of writing this could be:

let build_num = parts
.next()
.filter(|b| !b.is_empty())
.map_or(Some(0), parse_u32)?;

but both versions should work

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 here 355574e .

Some(b) if !b.is_empty() => parse_u32(b)?,
_ => 0,
};

// Reject if there are more than 4 parts
if parts.next().is_some() {
return None;
}

Some(AppVersion {
major,
minor,
revision,
build_num,
})
}

/// Send SMP Image State request, parse CBOR, and return parsed version numbers.
pub fn get_version_numbers() -> Option<AppVersion> {
let mut cbor_data = [0u8; 16];
let mut data = [0u8; 32];
let mut tx_buf = [0u8; 64];

let mut writer = SmpBuffer::new(&mut cbor_data);
let mut enc = Encoder::new(&mut writer);
// Empty map as request body
unwrap!(enc.map(0));

unwrap!(receiver_acquire());

let data_len = writer.bytes_written();
let header = SmpHeader::new(
SMP_OP_READ,
data_len,
SMP_GROUP_IMAGE,
0,
SMP_CMD_ID_IMAGE_STATE,
)
.to_bytes();

data[..SMP_HEADER_SIZE].copy_from_slice(&header);
data[SMP_HEADER_SIZE..SMP_HEADER_SIZE + data_len].copy_from_slice(&cbor_data[..data_len]);

if send_request(&mut data[..SMP_HEADER_SIZE + data_len], &mut tx_buf).is_err() {
receiver_release();
return None;
}

let mut cbor_payload = [0u8; 256];
let res = wait_for_response(
MsgType::ImageStateResponse,
&mut cbor_payload,
Duration::from_millis(2000),
);
if res.is_err() {
return None;
}

extract_version_numbers_from_cbor(&cbor_payload).ok()
}

// Walk CBOR: { "images": [ { "version": "x.y.z[.t]" , ... } , ... ], ... }
fn extract_version_numbers_from_cbor(cbor: &[u8]) -> Result<AppVersion, decode::Error> {
let mut dec = Decoder::new(cbor);

// Outer map (definite/indefinite)
match dec.map()? {
Some(n) => {
for _ in 0..n {
let key = dec.str()?;
if key == "images" {
return parse_images_array_for_version(&mut dec);
} else {
dec.skip()?;
}
}
}
None => loop {
if let Type::Break = dec.datatype()? {
dec.skip()?;
break;
}
let key = dec.str()?;
if key == "images" {
return parse_images_array_for_version(&mut dec);
} else {
dec.skip()?;
}
},
}

Err(decode::Error::message("images not found"))
}

fn parse_images_array_for_version(dec: &mut Decoder) -> Result<AppVersion, decode::Error> {
match dec.array()? {
Some(n) => {
// Read first element only
if n == 0 {
return Err(decode::Error::message("no images"));
}
parse_image_map_for_version(dec)
}
None => {
// Indefinite array: read first item, then stop
if let Type::Break = dec.datatype()? {
dec.skip()?;
return Err(decode::Error::message("no images"));
}
parse_image_map_for_version(dec)
}
}
}

fn parse_image_map_for_version(dec: &mut Decoder) -> Result<AppVersion, decode::Error> {
match dec.map()? {
Some(n) => {
for _ in 0..n {
let key = dec.str()?;
if key == "version" {
let s = dec.str()?;
return parse_app_version(s)
.ok_or_else(|| decode::Error::message("bad version string"));
} else {
dec.skip()?;
}
}
}
None => loop {
if let Type::Break = dec.datatype()? {
dec.skip()?;
break;
}
let key = dec.str()?;
if key == "version" {
let s = dec.str()?;
return parse_app_version(s)
.ok_or_else(|| decode::Error::message("bad version string"));
} else {
dec.skip()?;
}
},
}
Err(decode::Error::message("version not found"))
}
Loading