From f42a8205e8c7a72e88528bd7a9ca13fd8631e975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20R=C3=B8nningstad?= Date: Fri, 13 Dec 2019 03:27:54 +0100 Subject: [PATCH] serial_recovery: Replace CBOR decoding code with generated code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add the cddl_gen repository as a submodule. - Add a CDDL description file for the serial recovery packets to be decoded. - Add generated code files and cddl_gen's CBOR library to CMakeList.txt for Zephyr. - Convert boot_serial.c to use the new code. - Add a bash script to (re)generate code files using cddl_gen.py. Serial recovery should work exactly as before, but the binary should be about 1k smaller. Signed-off-by: Øyvind Rønningstad --- .gitmodules | 3 + boot/boot_serial/src/boot_serial.c | 128 ++++-------------- .../src/regenerate_serial_recovery_cbor.sh | 44 ++++++ boot/boot_serial/src/serial_recovery.cddl | 15 ++ boot/zephyr/CMakeLists.txt | 2 + ext/cddl_gen | 1 + 6 files changed, 95 insertions(+), 98 deletions(-) create mode 100755 boot/boot_serial/src/regenerate_serial_recovery_cbor.sh create mode 100644 boot/boot_serial/src/serial_recovery.cddl create mode 160000 ext/cddl_gen diff --git a/.gitmodules b/.gitmodules index 4ef347f8..2135412c 100644 --- a/.gitmodules +++ b/.gitmodules @@ -16,3 +16,6 @@ [submodule "boot/cypress/libs/cy-mbedtls-acceleration"] path = boot/cypress/libs/cy-mbedtls-acceleration url = https://github.com/cypresssemiconductorco/cy-mbedtls-acceleration.git +[submodule "boot/boot_serial/src/cddl_gen"] + path = ext/cddl_gen + url = https://github.com/oyvindronningstad/cddl_gen.git diff --git a/boot/boot_serial/src/boot_serial.c b/boot/boot_serial/src/boot_serial.c index 47baa58d..13af2b3f 100644 --- a/boot/boot_serial/src/boot_serial.c +++ b/boot/boot_serial/src/boot_serial.c @@ -43,7 +43,6 @@ #include #include #include -#include #endif /* __ZEPHYR__ */ #include @@ -60,6 +59,8 @@ #include "bootutil_priv.h" #endif +#include "serial_recovery_cbor.h" + MCUBOOT_LOG_MODULE_DECLARE(mcuboot); #define BOOT_SERIAL_INPUT_MAX 512 @@ -226,18 +227,13 @@ bs_list(char *buf, int len) static void bs_upload(char *buf, int len) { - CborParser parser; - struct cbor_buf_reader reader; - struct CborValue root_value; - struct CborValue value; - uint8_t img_data[512]; + const uint8_t *img_data = NULL; long long int off = UINT_MAX; size_t img_blen = 0; uint8_t rem_bytes; long long int data_len = UINT_MAX; int img_num; size_t slen; - char name_str[8]; const struct flash_area *fap = NULL; int rc; #ifdef CONFIG_BOOT_ERASE_PROGRESSIVELY @@ -245,7 +241,6 @@ bs_upload(char *buf, int len) struct flash_sector sector; #endif - memset(img_data, 0, sizeof(img_data)); img_num = 0; /* @@ -258,99 +253,36 @@ bs_upload(char *buf, int len) * } */ - /* - * Object comes within { ... } - */ - cbor_buf_reader_init(&reader, (uint8_t *)buf, len); - cbor_parser_init(&reader.r, 0, &parser, &root_value); + Upload_t upload; + if (!cbor_decode_Upload((const uint8_t *)buf, len, &upload)) { + goto out_invalid_data; + } - if (!cbor_value_is_container(&root_value)) { - goto out_invalid_data; - } - if (cbor_value_enter_container(&root_value, &value)) { - goto out_invalid_data; - } - while (cbor_value_is_valid(&value)) { - /* - * Decode key. - */ - if (cbor_value_calculate_string_length(&value, &slen)) { - goto out_invalid_data; - } - if (!cbor_value_is_text_string(&value) || - slen >= sizeof(name_str) - 1) { - goto out_invalid_data; - } - if (cbor_value_copy_text_string(&value, name_str, &slen, &value)) { - goto out_invalid_data; - } - name_str[slen] = '\0'; - if (!strcmp(name_str, "data")) { - /* - * Image data - */ - if (value.type != CborByteStringType) { - goto out_invalid_data; - } - if (cbor_value_calculate_string_length(&value, &slen) || - slen >= sizeof(img_data)) { - goto out_invalid_data; - } - if (cbor_value_copy_byte_string(&value, img_data, &slen, &value)) { - goto out_invalid_data; - } - img_blen = slen; - } else if (!strcmp(name_str, "off")) { - /* - * Offset of the data. - */ - if (value.type != CborIntegerType) { - goto out_invalid_data; - } - if (cbor_value_get_int64(&value, &off)) { - goto out_invalid_data; - } - if (cbor_value_advance(&value)) { - goto out_invalid_data; - } - } else if (!strcmp(name_str, "len")) { - /* - * Length of the image. This should only be present in the first - * block of data; when offset is 0. - */ - if (value.type != CborIntegerType) { - goto out_invalid_data; - } - if (cbor_value_get_int64(&value, &data_len)) { - goto out_invalid_data; - } - if (cbor_value_advance(&value)) { - goto out_invalid_data; - } - } else if (!strcmp(name_str, "image")) { - /* - * In a multi-image system, image number to upload to, if not - * present will upload to slot 0 of image set 0. - */ - if (value.type != CborIntegerType) { - goto out_invalid_data; - } - if (cbor_value_get_int(&value, &img_num)) { - goto out_invalid_data; - } - if (cbor_value_advance(&value)) { - goto out_invalid_data; - } - } else { - /* - * Unknown keys. - */ - if (cbor_value_advance(&value)) { - goto out_invalid_data; - } + for (int i = 0; i < upload._Upload_members_count; i++) { + _Member_t *member = &upload._Upload_members[i]; + switch(member->_Member_choice) { + case _Member_image: + img_num = member->_Member_image; + break; + case _Member_data: + img_data = member->_Member_data.value; + slen = member->_Member_data.len; + img_blen = slen; + break; + case _Member_len: + data_len = member->_Member_len; + break; + case _Member_off: + off = member->_Member_off; + break; + case _Member_sha: + default: + /* Nothing to do. */ + break; } } - if (off == UINT_MAX) { + + if (off == UINT_MAX || img_data == NULL) { /* * Offset must be set in every block. */ diff --git a/boot/boot_serial/src/regenerate_serial_recovery_cbor.sh b/boot/boot_serial/src/regenerate_serial_recovery_cbor.sh new file mode 100755 index 00000000..79cb6e9d --- /dev/null +++ b/boot/boot_serial/src/regenerate_serial_recovery_cbor.sh @@ -0,0 +1,44 @@ +#!/bin/bash + +if [ "$1" == "--help" ] || [ "$1" == "" ]; then + echo "Regenerate serial_recovery_cbor.c|h if the cddl_gen submodule is updated." + echo "Usage: $0 " + echo " e.g. $0 \"2020 Nordic Semiconductor ASA\"" + exit -1 +fi + +add_copy_notice() { +echo "$(printf '/* + * This file has been %s from the cddl_gen submodule. + * Commit %s + */ + +' "$2" "$(git -C ../../../ext/cddl_gen rev-parse HEAD)"; cat $1;)" > $1 +} + +echo "Copying cbor_decode.c|h" +copy_with_copy_notice() { + cp $1 $2 + add_copy_notice $2 "copied" +} + +copy_with_copy_notice ../../../ext/cddl_gen/src/cbor_decode.c cbor_decode.c +copy_with_copy_notice ../../../ext/cddl_gen/include/cbor_decode.h cbor_decode.h cbor_decode.h + +echo "Generating serial_recovery_cbor.c|h" +python3 ../../../ext/cddl_gen/scripts/cddl_gen.py -i serial_recovery.cddl -t Upload --oc serial_recovery_cbor.c --oh serial_recovery_cbor.h --time-header + +add_copyright() { +echo "$(printf '/* + * Copyright (c) %s + * + * SPDX-License-Identifier: Apache-2.0 + */ + +' "$2"; cat $1;)" > $1 +} + +add_copyright serial_recovery_cbor.c "$1" +add_copyright serial_recovery_cbor.h "$1" +add_copy_notice serial_recovery_cbor.c "generated" +add_copy_notice serial_recovery_cbor.h "generated" diff --git a/boot/boot_serial/src/serial_recovery.cddl b/boot/boot_serial/src/serial_recovery.cddl new file mode 100644 index 00000000..9aae63c9 --- /dev/null +++ b/boot/boot_serial/src/serial_recovery.cddl @@ -0,0 +1,15 @@ +; +; Copyright (c) 2020 Nordic Semiconductor ASA +; +; SPDX-License-Identifier: Apache-2.0 +; + +Member = ("image" => int) / + ("data" => bstr) / + ("len" => int) / + ("off" => int) / + ("sha" => bstr) + +Upload = { + 3**5members: Member +} diff --git a/boot/zephyr/CMakeLists.txt b/boot/zephyr/CMakeLists.txt index 9a6a6b8d..7905278f 100644 --- a/boot/zephyr/CMakeLists.txt +++ b/boot/zephyr/CMakeLists.txt @@ -191,6 +191,8 @@ endif() if(CONFIG_MCUBOOT_SERIAL) zephyr_sources(${BOOT_DIR}/zephyr/serial_adapter.c) zephyr_sources(${BOOT_DIR}/boot_serial/src/boot_serial.c) + zephyr_sources(${BOOT_DIR}/boot_serial/src/serial_recovery_cbor.c) + zephyr_sources(${BOOT_DIR}/boot_serial/src/cbor_decode.c) zephyr_include_directories(${BOOT_DIR}/bootutil/include) zephyr_include_directories(${BOOT_DIR}/boot_serial/include) diff --git a/ext/cddl_gen b/ext/cddl_gen new file mode 160000 index 00000000..9d911cf0 --- /dev/null +++ b/ext/cddl_gen @@ -0,0 +1 @@ +Subproject commit 9d911cf0c7c9f13b5a9fdd5ed6c1012df21e5576