From 05c0d3fc0b19a78e56e80fc3600d0f1a97627305 Mon Sep 17 00:00:00 2001 From: Karol Trzcinski Date: Fri, 27 Mar 2020 11:00:29 +0100 Subject: [PATCH 1/2] elf: Add function to read section content with allocation In many functions there is need to read user section content and check firmware version or write section content to some manifest or dictionary. Previously to do it, in each function was loop to search for proper section in proper module, then allocate buffer, read content and check for possible error between each step what is quite overwhelming. After change there will be one function responsible for this task. Signed-off-by: Karol Trzcinski --- src/elf.c | 46 +++++++++++++++++++++++++++++++++++++ src/include/rimage/rimage.h | 2 ++ 2 files changed, 48 insertions(+) diff --git a/src/elf.c b/src/elf.c index dc6fd4d..663b2e0 100644 --- a/src/elf.c +++ b/src/elf.c @@ -516,6 +516,52 @@ int elf_find_section(const struct module *module, const char *name) return ret; } +int elf_read_section(const struct image *image, const char *section_name, + const Elf32_Shdr **dst_section, void **dst_buff) +{ + const struct module *module; + const Elf32_Shdr *section; + int section_index = -1; + int read; + int i; + + /* when there is more than one module, then first one is bootloader */ + for (i = image->num_modules == 1 ? 0 : 1; i < image->num_modules; i++) { + module = &image->module[i]; + section_index = elf_find_section(module, section_name); + if (section_index >= 0) + break; + } + + if (section_index < 0) { + fprintf(stderr, "error: section %s can't be found\n", + section_name); + return -EINVAL; + } + + section = &module->section[section_index]; + if (dst_section) + *dst_section = section; + + /* alloc buffer for section content */ + *dst_buff = calloc(1, section->size); + if (!*dst_buff) + return -ENOMEM; + + /* fill buffer with section content */ + fseek(module->fd, section->off, SEEK_SET); + read = fread(*dst_buff, 1, section->size, module->fd); + if (read != section->size) { + fprintf(stderr, + "error: can't read %s section %d\n", section_name, + -errno); + free(*dst_buff); + return -errno; + } + + return section->size; +} + int elf_parse_module(struct image *image, int module_index, const char *name) { struct module *module; diff --git a/src/include/rimage/rimage.h b/src/include/rimage/rimage.h index e129d52..68c14c5 100644 --- a/src/include/rimage/rimage.h +++ b/src/include/rimage/rimage.h @@ -177,6 +177,8 @@ void elf_free_module(struct image *image, int module_index); int elf_is_rom(struct image *image, Elf32_Shdr *section); int elf_validate_modules(struct image *image); int elf_find_section(const struct module *module, const char *name); +int elf_read_section(const struct image *image, const char *name, + const Elf32_Shdr **dst_section, void **dst_buff); int elf_validate_section(struct image *image, struct module *module, Elf32_Shdr *section, int index); From 9c70c6e5fde227ff97eefd1ad4d47702fb472503 Mon Sep 17 00:00:00 2001 From: Karol Trzcinski Date: Tue, 7 Apr 2020 12:40:06 +0200 Subject: [PATCH 2/2] ext_man: Create extended manifest Extended manifest is a place to store metadata about firmware, known during compilation time - for example firmware version or used compiler. Given information are read on host side before firmware startup. This part of output binary is not signed. Signed-off-by: Karol Trzcinski --- CMakeLists.txt | 1 + src/ext_manifest.c | 161 ++++++++++++++++++++++ src/include/rimage/rimage.h | 2 + src/include/sof/kernel/ext_manifest.h | 81 +++++++++++ src/include/sof/kernel/ext_manifest_gen.h | 34 +++++ src/rimage.c | 11 ++ 6 files changed, 290 insertions(+) create mode 100644 src/ext_manifest.c create mode 100644 src/include/sof/kernel/ext_manifest.h create mode 100644 src/include/sof/kernel/ext_manifest_gen.h diff --git a/CMakeLists.txt b/CMakeLists.txt index a7b29c0..4d176dd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,6 +17,7 @@ add_executable(rimage src/hash.c src/pkcs1_5.c src/manifest.c + src/ext_manifest.c src/elf.c src/rimage.c ) diff --git a/src/ext_manifest.c b/src/ext_manifest.c new file mode 100644 index 0000000..90604a2 --- /dev/null +++ b/src/ext_manifest.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: BSD-3-Clause +// +// Copyright(c) 2020 Intel Corporation. All rights reserved. +// +// Author: Karol Trzcinski + +#include +#include +#include +#include + +#include "kernel/ext_manifest_gen.h" +#include "kernel/ext_manifest.h" +#include "rimage.h" + +const struct ext_man_header ext_man_template = { + .magic = EXT_MAN_MAGIC_NUMBER, + .header_version = EXT_MAN_VERSION, + .header_size = sizeof(struct ext_man_header), + .full_size = 0, /* runtime variable */ +}; + +static int ext_man_open_file(struct image *image) +{ + /* open extended manifest outfile for writing */ + sprintf(image->out_ext_man_file, "%s.xman", image->out_file); + unlink(image->out_ext_man_file); + + image->out_ext_man_fd = fopen(image->out_ext_man_file, "wb"); + if (!image->out_ext_man_fd) { + fprintf(stderr, "error: unable to open %s for writing %d\n", + image->out_ext_man_file, errno); + return errno; + } + + return 0; +} + +static int ext_man_validate(uint32_t section_size, const void *section_data) +{ + uint8_t *sbuf = (uint8_t *)section_data; + struct ext_man_elem_header head; + uint32_t offset = 0; + + /* copy each head to local struct to omit memory align issues */ + while (offset < section_size) { + memcpy(&head, &sbuf[offset], sizeof(head)); + fprintf(stdout, "Extended manifest found module, type: 0x%04X size: 0x%04X (%4d) offset: 0x%04X\n", + head.type, head.elem_size, head.elem_size, offset); + if (head.elem_size == 0 || head.elem_size % EXT_MAN_ALIGN) { + fprintf(stderr, + "error: invalid extended manifest element size\n"); + return -EINVAL; + } + offset += head.elem_size; + } + + /* sum of packets size != section size */ + if (offset != section_size) { + fprintf(stderr, + "error: fw_metadata section is inconsistent, section size: 0x%04X != 0x%04X sum of packets size\n", + section_size, offset); + return -EINVAL; + } else { + return 0; + } +} + +static int ext_man_build(const struct image *image, + struct ext_man_header **dst_buff) +{ + struct ext_man_header ext_man; + const Elf32_Shdr *section; + uint8_t *buffer = NULL; + uint8_t *sec_buffer = NULL; + size_t offset; + int ret = 0; + + ret = elf_read_section(image, ".fw_metadata", §ion, + (void **)&sec_buffer); + if (ret < 0) { + fprintf(stderr, + "error: failed to read .fw_metadata section content, code %d\n", + ret); + goto out; + } + + /* fill ext_man struct, size aligned to 4 to avoid unaligned accesses */ + memcpy(&ext_man, &ext_man_template, sizeof(struct ext_man_header)); + ext_man.full_size = ext_man.header_size; + ext_man.full_size += section->size; + if (ext_man.full_size % 4) { + fprintf(stderr, + "error: extended manifest size must be aligned to 4\n"); + ret = -EINVAL; + goto out; + } + + /* alloc buffer for ext_man */ + buffer = calloc(1, ext_man.full_size); + if (!buffer) { + ret = -ENOMEM; + goto out; + } + + /* fill buffer with ext_man and section content */ + memcpy(buffer, &ext_man, ext_man.header_size); + offset = ext_man.header_size; + + memcpy(&buffer[offset],sec_buffer, section->size); + + *dst_buff = (struct ext_man_header *)buffer; + +out: + return ret; +} + +int ext_man_write(struct image *image) +{ + struct ext_man_header *ext_man = NULL; + int count; + int ret; + + ret = ext_man_open_file(image); + if (ret) + goto out; + + ret = ext_man_build(image, &ext_man); + if (ret) + goto out; + + /* validate metadata section */ + ret = ext_man_validate(ext_man->full_size - ext_man->header_size, + (char *)ext_man + ext_man->header_size); + if (ret) { + ret = -errno; + goto out; + } + + /* write extended metadata to file */ + count = fwrite(ext_man, 1, ext_man->full_size, image->out_ext_man_fd); + + if (count != ext_man->full_size) { + fprintf(stderr, + "error: can't write extended manifest to file %d\n", + -errno); + ret = -errno; + goto out; + } + + fprintf(stdout, "Extended manifest saved to file %s size 0x%04X (%d) bytes\n\n", + image->out_ext_man_file, ext_man->full_size, + ext_man->full_size); + +out: + if (ext_man) + free(ext_man); + if (image->out_ext_man_fd) + fclose(image->out_ext_man_fd); + return ret; +} diff --git a/src/include/rimage/rimage.h b/src/include/rimage/rimage.h index 68c14c5..1f7d13f 100644 --- a/src/include/rimage/rimage.h +++ b/src/include/rimage/rimage.h @@ -115,9 +115,11 @@ struct image { void *rom_image; FILE *out_rom_fd; FILE *out_man_fd; + FILE *out_ext_man_fd; FILE *out_unsigned_fd; char out_rom_file[256]; char out_man_file[256]; + char out_ext_man_file[256]; char out_unsigned_file[256]; /* fw version and build id */ diff --git a/src/include/sof/kernel/ext_manifest.h b/src/include/sof/kernel/ext_manifest.h new file mode 100644 index 0000000..d27bafe --- /dev/null +++ b/src/include/sof/kernel/ext_manifest.h @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * + * Copyright(c) 2020 Intel Corporation. All rights reserved. + * + * Author: Karol Trzcinski + */ + +/* + * Extended manifest is a place to store metadata about firmware, known during + * compilation time - for example firmware version or used compiler. + * Given information are read on host side before firmware startup. + * This part of output binary is not signed. + * + * To add new content to ext_man, in firmware code define struct which starts + * with ext_man_elem_head followed by usage dependent content and place whole + * struct in "fw_metadata" section. Moreover kernel code should be modified to + * properly read new packet. + * + * Extended manifest is designed to be extensible. In header there is a field + * which describe header length, so after appending some data to header then it + * can be easily skipped by device with older version of this header. + * Unknown ext_man elements should be just skipped by host, + * to be backward compatible. Field `ext_man_elem_header.elem_size` should be + * used in such a situation. + */ + +#ifndef __KERNEL_EXT_MANIFEST_H__ +#define __KERNEL_EXT_MANIFEST_H__ + +#include + +#ifndef __packed +#define __packed __attribute__((packed)) +#endif + +/* In ASCII `XMan` */ +#define EXT_MAN_MAGIC_NUMBER 0x6e614d58 + +/* Build u32 number in format MMmmmppp */ +#define EXT_MAN_BUILD_VERSION(MAJOR, MINOR, PATH) ( \ + ((uint32_t)(MAJOR) << 24) | \ + ((uint32_t)(MINOR) << 12) | \ + (uint32_t)(PATH)) + +/* check extended manifest version consistency */ +#define EXT_MAN_VERSION_INCOMPATIBLE(host_ver, cli_ver) ( \ + ((host_ver) & GENMASK(31, 24)) != \ + ((cli_ver) & GENMASK(31, 24))) + +/* used extended manifest header version */ +#define EXT_MAN_VERSION EXT_MAN_BUILD_VERSION(1, 0, 0) + +/* struct size alignment for ext_man elements */ +#define EXT_MAN_ALIGN 16 + +/* extended manifest header, deleting any field breaks backward compatibility */ +struct ext_man_header { + uint32_t magic; /**< identification number, */ + /**< EXT_MAN_MAGIC_NUMBER */ + uint32_t full_size; /**< [bytes] full size of ext_man, */ + /**< (header + content + padding) */ + uint32_t header_size; /**< [bytes] makes header extensionable, */ + /**< after append new field to ext_man header */ + /**< then backward compatible won't be lost */ + uint32_t header_version; /**< value of EXT_MAN_VERSION */ + /**< not related with following content */ + + /* just after this header should be list of ext_man_elem_* elements */ +} __packed; + +/* Now define extended manifest elements */ + +/* extended manifest element header */ +struct ext_man_elem_header { + uint32_t type; /**< EXT_MAN_ELEM_* */ + uint32_t elem_size; /**< in bytes, including header size */ + + /* just after this header should be type dependent content */ +} __packed; + +#endif /* __KERNEL_EXT_MANIFEST_H__ */ diff --git a/src/include/sof/kernel/ext_manifest_gen.h b/src/include/sof/kernel/ext_manifest_gen.h new file mode 100644 index 0000000..16bebcf --- /dev/null +++ b/src/include/sof/kernel/ext_manifest_gen.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * + * Copyright(c) 2020 Intel Corporation. All rights reserved. + * + * Author: Karol Trzcinski + */ + +/* + * Extended manifest is a place to store metadata about firmware, known during + * compilation time - for example firmware version or used compiler. + * Given information are read on host side before firmware startup. + * This part of output binary is not signed. + * + * To add new content to ext_man, in firmware code define struct which starts + * with ext_man_elem_head followed by usage dependent content and place whole + * struct in "fw_metadata" section. Moreover kernel code should be modified to + * properly read new packet. + * + * Extended manifest designed to be extensible. In header there is a field which + * describe header length, so after appending some data to header then it can be + * easily skipped by device with older version of this header. + * From other side, unknown ext_man elements should be just skipped by host, + * to be backward compatible. Field ext_man_elem_header.elem_size should be + * used in such a situation. + */ + +#ifndef __EXT_MAN_H__ +#define __EXT_MAN_H__ + +#include "rimage.h" + +int ext_man_write(struct image *image); + +#endif /* __EXT_MAN_H__ */ diff --git a/src/rimage.c b/src/rimage.c index 11bee8a..e2cced0 100644 --- a/src/rimage.c +++ b/src/rimage.c @@ -8,6 +8,7 @@ #include #include +#include "kernel/ext_manifest_gen.h" #include "rimage.h" #include "manifest.h" @@ -187,6 +188,16 @@ int main(int argc, char *argv[]) ret = image.adsp->write_firmware_meu(&image); else ret = image.adsp->write_firmware(&image); + if (ret) + goto out; + + ret = ext_man_write(&image); + if (ret < 0) { + fprintf(stderr, "warning: unable to write extended manifest, %d\n", + ret); + /* ext man is optional until FW side merge to master */ + ret = 0; + } out: /* close files */