Ran automated checker for common typos [1]. Most of these changes have
no functional change *except* for `./ci/sim_run.sh` where, previously
the `bootstrap` feature wasn't being selected properly.
I didn't touch anything in the `./ext/` folder as anything in there
should probably be fixed in the upstream repo.
[1] https://github.com/codespell-project/codespell
Signed-off-by: Sam Bristow <sam@bristow.nz>
Before using the image and header size fields from the image header,
verify that both of these values are sane. In this case, sanity means
that there is no arithmetic overflow when they are added, and that the
result is within the bounds of the containing flash region. This is
done in addition to the check of the header's magic number.
Signed-off-by: David Brown <david.brown@linaro.org>
To facility better checking for integer overflow, add these two
functions to add while checking for overflow. If the result overflows,
they will return an error, instead of performing the addition.
Signed-off-by: David Brown <david.brown@linaro.org>
The ext/mbedtls directory has a copy of the ASN.1 parser from the mbed
TLS library. To allow a future change to bring the entire mbed TLS
library in as a submodule under ext, rename this to mbedtls-asn1 to make
it clear this is only a subset.
Signed-off-by: David Brown <david.brown@linaro.org>
In the TLV iterator code a variable is used to save the offset in flash
where the protected TLVs end; this was declared as uint16_t and could
easily overflow so fix it by updating the type to uint32_t.
Signed-off-by: Fabio Utzig <utzig@apache.org>
Convert this `extern const uint32_t` to a simple define. Provide an
accessor function so that the simulator is able to access the value as
well. This has a minor improvement on the generated code within MCUboot
itself.
Signed-off-by: David Brown <david.brown@linaro.org>
BOOT_MAX_ALIGN is defined as
extern const uint32_t BOOT_MAX_ALIGN;
and is assigned a value in a single file. This causes extra work when
this is used as the size of a local variable in a function.
The value was made a constant in order for the simulator to be able to
access the value. Instead of making it a "real" constant, keep it as a
define, unifying the value of FLASH_MAX_ALIGN and this one, and provide
an accessor function for the test code to be able to access this value.
This causes a minor improvement in the code generated in
`boot_write_status`, but more importantly, eliminates a VLA from the
code, which increases the possible compilers supported by MCUboot.
Signed-off-by: David Brown <david.brown@linaro.org>
This introduces an API which allows for iteration over an image's TLVs
without resorting to low-level implementation details. All previous TLV
low-level handling was updated to comply with this new interface, and it
also makes it easier for external code to handle TLVs.
The API provides two functions:
1) To start a new iterator:
```
int bootutil_tlv_iter_begin(struct image_tlv_iter *it,
const struct image_header *hdr,
const struct flash_area *fap, uint8_t type,
bool prot);
```
2) To iterate over existing TLVs of given type:
```
int bootutil_tlv_iter_next(struct image_tlv_iter *it, uint32_t *off,
uint16_t *len, uint8_t *type);
```
A type of IMAGE_TLV_ANY was added to allow for iteration over all TLVs.
Low-level TLV access functions were removed from API, but low-level
structs are still visible in the namespace.
Signed-off-by: Fabio Utzig <utzig@apache.org>
Upstream PR #547
The recent changes to these are tripping GCC printf-like macro
attribute warnings on my platform. Just cast all the values to ulong
to make everybody's compilers happy.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This allows refactoring a check that was done in multiple places looking
for the same swap types.
Also fixes an issue where dependency checks would be run if a swap type
had a value that indicates fail.
Signed-off-by: Fabio Utzig <utzig@apache.org>
The slot validation routine would result in a non-installed image to be
equivalent to a failed validation, which would set the swap type to
*_FAIL and require special handling in the dependency checks. This
routine was updated, so that it now returns a value that can be used to
check that the error was due to an image not found.
Signed-off-by: Fabio Utzig <utzig@apache.org>
Since a copy/erase during a swap can result in some number of sectors
erased and copied, which is not necessarily equal to 1, update design
document and bootloader function naming to reflect this fact, by using
the postfix "_region" instead of "_sector" in those cases.
This fixes#541
Signed-off-by: Fabio Utzig <utzig@apache.org>
This fixes an issue where dependencies were being decrypted before
hashing when encrypted images are used. When using dependencies, some
"protected" TLVs are added to the image. Those TLVs take part into the
image hash calculation, but are not encrypted and must be sent plain
to the hash update routine.
Signed-off-by: Fabio Utzig <utzig@apache.org>
This fixes two issues related to swap status control:
1. During a swap, the status was written offset by one, because it was
being incremented before it was written to flash. With the increment
happening early the offset was calculated always one position after
where it should be, which would leave the first status index free,
and override the last one (worst case scenario).
2. When an image is too big it requires the swap status to be
temporarily stored on scratch, to allow the last sector on the
primary slot to be erased. The status is written to scratch for 2
status updates, and afterwards copied back to the primary slot, which
then receives future status updates. The code that copied the status
back from scratch to the primary slot was erroneously copying the space
of 3 status writes, which would result in a write over non-erased area
for the third byte.
Signed-off-by: Fabio Utzig <utzig@apache.org>
When only upgrading one slot, the swap type for the non upgraded slot is
BOOT_SWAP_TYPE_FAIL instead of NONE. Account for this, and check slot 0
in this case.
This is probably not the correct solution, but it is unclear of the
timing of when the dependencies of the other images first slot would be
checked.
Signed-off-by: David Brown <david.brown@linaro.org>
Signed-off-by: Fabio Utzig <utzig@apache.org>
This fixes the issue where an unmet dependency would result in a restart
of image dependency checks. To make the code easier to follow, some
function nesting was removed, functions that check slot dependencies
were renamed appropriately and dependency check is not done when no
upgrades are marked.
Signed-off-by: Fabio Utzig <utzig@apache.org>
The checking of image version compatibility is only used by dependencies
code; this moves the version checking function static, and removes it
from the exported API.
Signed-off-by: Fabio Utzig <utzig@apache.org>
`boot_swap_type_multi` should be used instead. The `BOOT_CURR_IMG` macro
already returns the correct value in single image mode.
For compatibility with external tools like mcumgr, `boot_swap_type`
remains available and always returns the swap type of image 0.
Signed-off-by: Fabio Utzig <utzig@apache.org>
An image must be decrypted when it is loaded on the secondary slot and
its header flag indicates it is encrypted. Instead of checking both
things every time the image is read, add a new macro, MUST_DECRYPT, that
does both checks.
Also `BOOT_CURR_ENC` was simplified to be used directly on
`bootutil_img_validate` calls, returning NULL for no encrypted images.
Signed-off-by: Fabio Utzig <utzig@apache.org>
While iterating over the swap table looking for swap type,
avoid asserting on errors; let caller handle error handling.
Signed-off-by: Fabio Utzig <utzig@apache.org>
`boot_read_image_size` was already receiving the bootloader state and
slot; since the image_header is part of the state and can be gathered
from it, it was removed from the list of parameters.
Signed-off-by: Fabio Utzig <utzig@apache.org>
This adds a single trailer writing routine that can write any amount of
data (upto BOOT_MAX_ALIGN); the functions that were writing to the
trailer now just call this one passing the offset and data/len.
Signed-off-by: Fabio Utzig <utzig@apache.org>
Update each function to calculate the offset based on a previous
function offset, so that if one has to be changed not all functions have
to be updated.
Signed-off-by: Fabio Utzig <utzig@apache.org>
When doing a swap some of the information required by the process
is temporarily written to the status area; previously each data
writing routines tried to find the correct area by looking for the
magic and assuming that data is located there (because everything
fits a single sector). This moves the code to find the magic into
a separate routine that is now used by others.
Signed-off-by: Fabio Utzig <utzig@apache.org>
When the hash key length parameter is invalid (bigger than allowed) just
return an error for the caller routines; this will work like a not found
key and allows the removal of assert() code in this module.
Signed-off-by: Fabio Utzig <utzig@apache.org>
TLV offset was determined by manually adding the header and image
size in many places; this makes the addition into a macro receiving
a image_header to ease future changes.
Signed-off-by: Fabio Utzig <utzig@apache.org>
The boot loader state currently exists as global variable (boot_data)
which is accessed by all routines; this updates all routines that
require access to the state to receive it by parameter.
Variables that are declared as "static" (globals) were #ifdef'ed to
only use static when building a native bootloader, to avoid stack
allocation. When bootutil is built to run in the simulator they are
stack allocated to avoid thread share.
Signed-off-by: Fabio Utzig <utzig@apache.org>
Currently to determine which image is being operated on, there is a global
variable called current_image which is used by most macros and APIs to
correctly access the flash areas required by the bootloader. This moves
this variable to the already existing state struct and refactors all
macros and APIs to receive the current image by parameters. To maintain
compatibility some of the macros were not updated and use image 0 when
called.
The definitions of FLASH_AREA_IMAGE_PRIMARY and FLASH_AREA_IMAGE_SECONDARY
for Mynewt compatibility were moved out of bootutil sources to a Mynewt
specific include file.
Signed-off-by: Fabio Utzig <utzig@apache.org>
This struct currently relies on padding being done by the C compiler.
Although this is generally done the same across targets, this isn't
required, and it is better to make any padding explicit. This also
makes it clear that there is extra space in this structure that can be
used for another purpose.
The python imgtool is also fixed to match. It was using a 32-bit int,
which only worked in the little endian case.
Signed-off-by: David Brown <david.brown@linaro.org>
This patch adds the capability to check image dependencies in case
of multi-image boot. The dependencies are described with a new type
of TLV in the manifest.
Change-Id: If45f81a00d4324c881634f50156f9939e1bf8707
Signed-off-by: David Vincze <david.vincze@arm.com>
This patch adds the capability to handle multiple firmware images,
to update them independently. Also update the design documentation.
It separates the completion of aborted image swap operations and the
update of images even more as these should be happening at different
stages of the boot process according to the design proposal of
the multiple image support:
https://github.com/JuulLabs-OSS/mcuboot/pull/317.
Change-Id: I7eb5f632298bb08c805bfaee0359703b2ae19e9d
Signed-off-by: David Vincze <david.vincze@arm.com>
This patch introduces the BOOT_IMAGE_NUMBER macro and current_image
variable to support multiple updatable images and the associated
extended flash layout.
The FLASH_AREA_IMAGE_* object-like macros are replaced with
function-like ones and therefore some functions have been updated,
because the labels of a switch statement and the initialization
values of objects with static storage duration have to be constant
expressions.
Change-Id: Ib7b26ec3c94233e52db4f97825ddb6a3e55bb1d3
Signed-off-by: David Vincze <david.vincze@arm.com>
Overload the swap_type field in image trailer to store as an addition
the image sequence number. It indicates which image's swap was
interrupted. It is required by multi image boot to determine which
image the trailer belongs to if boot status is found on scratch area
when the swap operation is resumed.
Change-Id: I6820fd8277931aff4f0db408376eae8b42a030ed
Signed-off-by: Tamas Ban <tamas.ban@arm.com>
Signed-off-by: David Vincze <david.vincze@arm.com>
When HW / OS provides an always enabled watchdog, this macro can
optionally be implemented to avoid resets which are expected to
occur under normal conditions when swapping very large images or
running on slower devices.
Signed-off-by: Fabio Utzig <utzig@apache.org>
There is a bug in split_go that never opens the loader image flash
area, but opens the app/split image flash arta twice. This prevents
split_image_check from passing and the app from ever loading.
Signed-off-by: Alvaro Prieto <source@alvaroprieto.com>
Fabio Utzig