| Message ID | 20201117002805.13902-9-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: add capsule update support | expand |
On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > This is a utility mainly for test purpose. > mkeficapsule -f: create a test capsule file for FIT image firmware > > Having said that, you will be able to customize the code to fit > your specific requirements for your platform. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > tools/Makefile | 2 + > tools/mkeficapsule.c | 238 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 240 insertions(+) > create mode 100644 tools/mkeficapsule.c > > diff --git a/tools/Makefile b/tools/Makefile > index 51123fd92983..66d9376803e3 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > + > # We build some files with extra pedantic flags to try to minimize things > # that won't build on some weird host compiler -- though there are lots of > # exceptions for files that aren't complaint. > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > new file mode 100644 > index 000000000000..db95426457cc > --- /dev/null > +++ b/tools/mkeficapsule.c > @@ -0,0 +1,238 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2018 Linaro Limited > + * Author: AKASHI Takahiro > + */ > + > +#include <getopt.h> > +#include <malloc.h> > +#include <stdbool.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <linux/types.h> > +#include <sys/stat.h> > +#include <sys/types.h> > + > +typedef __u8 u8; > +typedef __u16 u16; > +typedef __u32 u32; > +typedef __u64 u64; > +typedef __s16 s16; > +typedef __s32 s32; > + > +#define aligned_u64 __aligned_u64 > + > +#ifndef __packed > +#define __packed __attribute__((packed)) > +#endif > + > +#include <efi.h> > +#include <efi_api.h> > + > +static const char *tool_name = "mkeficapsule"; > + > +efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > +efi_guid_t efi_guid_image_type_uboot_fit = > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > +efi_guid_t efi_guid_image_type_uboot_raw = > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > + > +static struct option options[] = { > + {"fit", required_argument, NULL, 'f'}, > + {"raw", required_argument, NULL, 'r'}, > + {"index", required_argument, NULL, 'i'}, > + {"instance", required_argument, NULL, 'I'}, > + {"version", required_argument, NULL, 'v'}, > + {"help", no_argument, NULL, 'h'}, > + {NULL, 0, NULL, 0}, > +}; > + > +static void print_usage(void) > +{ > + printf("Usage: %s [options] <output file>\n" > + "Options:\n" > + "\t--fit <fit image> new FIT image file\n" > + "\t--raw <raw image> new raw image file\n" > + "\t--index <index> update image index\n" > + "\t--instance <instance> update hardware instance\n" > + "\t--version <version> firmware version\n" > + "\t--help print a help message\n", > + tool_name); > +} > + > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > + unsigned long version, unsigned long index, > + unsigned long instance) > +{ > + struct efi_capsule_header header; > + struct efi_firmware_management_capsule_header capsule; > + struct efi_firmware_management_capsule_image_header image; > + FILE *f, *g; > + struct stat bin_stat; > + u8 *data; > + size_t size; > + > +#ifdef DEBUG > + printf("For output: %s\n", path); > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > + version, index, instance); > +#endif > + > + g = fopen(bin, "r"); > + if (!g) { > + printf("cannot open %s\n", bin); > + return -1; > + } > + if (stat(bin, &bin_stat) < 0) { > + printf("cannot determine the size of %s\n", bin); > + goto err_1; > + } > + data = malloc(bin_stat.st_size); > + if (!data) { > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > + goto err_1; > + } > + f = fopen(path, "w"); > + if (!f) { > + printf("cannot open %s\n", path); > + goto err_2; > + } > + header.capsule_guid = efi_guid_fm_capsule; > + header.header_size = sizeof(header); > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > + header.capsule_image_size = sizeof(header) > + + sizeof(capsule) + sizeof(u64) > + + sizeof(image) > + + bin_stat.st_size; > + > + size = fwrite(&header, 1, sizeof(header), f); > + if (size < sizeof(header)) { > + printf("write failed (%lx)\n", size); > + goto err_3; > + } > + > + capsule.version = 0x00000001; > + capsule.embedded_driver_count = 0; > + capsule.payload_item_count = 1; > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); With the complete series applied, building sandbox_defconfig: tools/mkeficapsule.c: In function ‘main’: tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside array bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ Please, ensure that the code compiles without warnings. I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 system. Best regards Heinrich > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > + if (size < (sizeof(capsule) + sizeof(u64))) { > + printf("write failed (%lx)\n", size); > + goto err_3; > + } > + > + image.version = version; > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > + image.update_image_index = index; > + image.update_image_size = bin_stat.st_size; > + image.update_vendor_code_size = 0; /* none */ > + image.update_hardware_instance = instance; > + image.image_capsule_support = 0; > + > + size = fwrite(&image, 1, sizeof(image), f); > + if (size < sizeof(image)) { > + printf("write failed (%lx)\n", size); > + goto err_3; > + } > + size = fread(data, 1, bin_stat.st_size, g); > + if (size < bin_stat.st_size) { > + printf("read failed (%lx)\n", size); > + goto err_3; > + } > + size = fwrite(data, 1, bin_stat.st_size, f); > + if (size < bin_stat.st_size) { > + printf("write failed (%lx)\n", size); > + goto err_3; > + } > + > + fclose(f); > + fclose(g); > + free(data); > + > + return 0; > + > +err_3: > + fclose(f); > +err_2: > + free(data); > +err_1: > + fclose(g); > + > + return -1; > +} > + > +/* > + * Usage: > + * $ mkeficapsule -f <firmware binary> <output file> > + */ > +int main(int argc, char **argv) > +{ > + char *file; > + efi_guid_t *guid; > + unsigned long index, instance, version; > + int c, idx; > + > + file = NULL; > + guid = NULL; > + index = 0; > + instance = 0; > + version = 0; > + for (;;) { > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > + if (c == -1) > + break; > + > + switch (c) { > + case 'f': > + if (file) { > + printf("Image already specified\n"); > + return -1; > + } > + file = optarg; > + guid = &efi_guid_image_type_uboot_fit; > + break; > + case 'r': > + if (file) { > + printf("Image already specified\n"); > + return -1; > + } > + file = optarg; > + guid = &efi_guid_image_type_uboot_raw; > + break; > + case 'i': > + index = strtoul(optarg, NULL, 0); > + break; > + case 'I': > + instance = strtoul(optarg, NULL, 0); > + break; > + case 'v': > + version = strtoul(optarg, NULL, 0); > + break; > + case 'h': > + print_usage(); > + return 0; > + } > + } > + > + /* need a output file */ > + if (argc != optind + 1) { > + print_usage(); > + return -1; > + } > + > + /* need a fit image file or raw image file */ > + if (!file) { > + print_usage(); > + return -1; > + } > + > + if (create_fwbin(argv[optind], file, guid, version, index, instance) > + < 0) { > + printf("Creating firmware capsule failed\n"); > + return -1; > + } > + > + return 0; > +} >
Heinrich, On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > > This is a utility mainly for test purpose. > > mkeficapsule -f: create a test capsule file for FIT image firmware > > > > Having said that, you will be able to customize the code to fit > > your specific requirements for your platform. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > tools/Makefile | 2 + > > tools/mkeficapsule.c | 238 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 240 insertions(+) > > create mode 100644 tools/mkeficapsule.c > > > > diff --git a/tools/Makefile b/tools/Makefile > > index 51123fd92983..66d9376803e3 100644 > > --- a/tools/Makefile > > +++ b/tools/Makefile > > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > > + > > # We build some files with extra pedantic flags to try to minimize things > > # that won't build on some weird host compiler -- though there are lots of > > # exceptions for files that aren't complaint. > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > > new file mode 100644 > > index 000000000000..db95426457cc > > --- /dev/null > > +++ b/tools/mkeficapsule.c > > @@ -0,0 +1,238 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright 2018 Linaro Limited > > + * Author: AKASHI Takahiro > > + */ > > + > > +#include <getopt.h> > > +#include <malloc.h> > > +#include <stdbool.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <linux/types.h> > > +#include <sys/stat.h> > > +#include <sys/types.h> > > + > > +typedef __u8 u8; > > +typedef __u16 u16; > > +typedef __u32 u32; > > +typedef __u64 u64; > > +typedef __s16 s16; > > +typedef __s32 s32; > > + > > +#define aligned_u64 __aligned_u64 > > + > > +#ifndef __packed > > +#define __packed __attribute__((packed)) > > +#endif > > + > > +#include <efi.h> > > +#include <efi_api.h> > > + > > +static const char *tool_name = "mkeficapsule"; > > + > > +efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > +efi_guid_t efi_guid_image_type_uboot_fit = > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > +efi_guid_t efi_guid_image_type_uboot_raw = > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > > + > > +static struct option options[] = { > > + {"fit", required_argument, NULL, 'f'}, > > + {"raw", required_argument, NULL, 'r'}, > > + {"index", required_argument, NULL, 'i'}, > > + {"instance", required_argument, NULL, 'I'}, > > + {"version", required_argument, NULL, 'v'}, > > + {"help", no_argument, NULL, 'h'}, > > + {NULL, 0, NULL, 0}, > > +}; > > + > > +static void print_usage(void) > > +{ > > + printf("Usage: %s [options] <output file>\n" > > + "Options:\n" > > + "\t--fit <fit image> new FIT image file\n" > > + "\t--raw <raw image> new raw image file\n" > > + "\t--index <index> update image index\n" > > + "\t--instance <instance> update hardware instance\n" > > + "\t--version <version> firmware version\n" > > + "\t--help print a help message\n", > > + tool_name); > > +} > > + > > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > > + unsigned long version, unsigned long index, > > + unsigned long instance) > > +{ > > + struct efi_capsule_header header; > > + struct efi_firmware_management_capsule_header capsule; > > + struct efi_firmware_management_capsule_image_header image; > > + FILE *f, *g; > > + struct stat bin_stat; > > + u8 *data; > > + size_t size; > > + > > +#ifdef DEBUG > > + printf("For output: %s\n", path); > > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > > + version, index, instance); > > +#endif > > + > > + g = fopen(bin, "r"); > > + if (!g) { > > + printf("cannot open %s\n", bin); > > + return -1; > > + } > > + if (stat(bin, &bin_stat) < 0) { > > + printf("cannot determine the size of %s\n", bin); > > + goto err_1; > > + } > > + data = malloc(bin_stat.st_size); > > + if (!data) { > > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > > + goto err_1; > > + } > > + f = fopen(path, "w"); > > + if (!f) { > > + printf("cannot open %s\n", path); > > + goto err_2; > > + } > > + header.capsule_guid = efi_guid_fm_capsule; > > + header.header_size = sizeof(header); > > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > > + header.capsule_image_size = sizeof(header) > > + + sizeof(capsule) + sizeof(u64) > > + + sizeof(image) > > + + bin_stat.st_size; > > + > > + size = fwrite(&header, 1, sizeof(header), f); > > + if (size < sizeof(header)) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + > > + capsule.version = 0x00000001; > > + capsule.embedded_driver_count = 0; > > + capsule.payload_item_count = 1; > > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > With the complete series applied, building sandbox_defconfig: > > tools/mkeficapsule.c: In function ‘main’: > tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside array > bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] > 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > Please, ensure that the code compiles without warnings. Fixing this warning would be easy, but I didn't see it with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. So I wonder if it is mandatory that the code compiles without warnings, and if so, which compiler and which version are required? -Takahiro Akashi > I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 > system. > > Best regards > > Heinrich > > > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > > + if (size < (sizeof(capsule) + sizeof(u64))) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + > > + image.version = version; > > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > > + image.update_image_index = index; > > + image.update_image_size = bin_stat.st_size; > > + image.update_vendor_code_size = 0; /* none */ > > + image.update_hardware_instance = instance; > > + image.image_capsule_support = 0; > > + > > + size = fwrite(&image, 1, sizeof(image), f); > > + if (size < sizeof(image)) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + size = fread(data, 1, bin_stat.st_size, g); > > + if (size < bin_stat.st_size) { > > + printf("read failed (%lx)\n", size); > > + goto err_3; > > + } > > + size = fwrite(data, 1, bin_stat.st_size, f); > > + if (size < bin_stat.st_size) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + > > + fclose(f); > > + fclose(g); > > + free(data); > > + > > + return 0; > > + > > +err_3: > > + fclose(f); > > +err_2: > > + free(data); > > +err_1: > > + fclose(g); > > + > > + return -1; > > +} > > + > > +/* > > + * Usage: > > + * $ mkeficapsule -f <firmware binary> <output file> > > + */ > > +int main(int argc, char **argv) > > +{ > > + char *file; > > + efi_guid_t *guid; > > + unsigned long index, instance, version; > > + int c, idx; > > + > > + file = NULL; > > + guid = NULL; > > + index = 0; > > + instance = 0; > > + version = 0; > > + for (;;) { > > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > > + if (c == -1) > > + break; > > + > > + switch (c) { > > + case 'f': > > + if (file) { > > + printf("Image already specified\n"); > > + return -1; > > + } > > + file = optarg; > > + guid = &efi_guid_image_type_uboot_fit; > > + break; > > + case 'r': > > + if (file) { > > + printf("Image already specified\n"); > > + return -1; > > + } > > + file = optarg; > > + guid = &efi_guid_image_type_uboot_raw; > > + break; > > + case 'i': > > + index = strtoul(optarg, NULL, 0); > > + break; > > + case 'I': > > + instance = strtoul(optarg, NULL, 0); > > + break; > > + case 'v': > > + version = strtoul(optarg, NULL, 0); > > + break; > > + case 'h': > > + print_usage(); > > + return 0; > > + } > > + } > > + > > + /* need a output file */ > > + if (argc != optind + 1) { > > + print_usage(); > > + return -1; > > + } > > + > > + /* need a fit image file or raw image file */ > > + if (!file) { > > + print_usage(); > > + return -1; > > + } > > + > > + if (create_fwbin(argv[optind], file, guid, version, index, instance) > > + < 0) { > > + printf("Creating firmware capsule failed\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > >
On Wed, Nov 25, 2020 at 10:05:06AM +0900, AKASHI Takahiro wrote: > Heinrich, > > On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > > On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > > > This is a utility mainly for test purpose. > > > mkeficapsule -f: create a test capsule file for FIT image firmware > > > > > > Having said that, you will be able to customize the code to fit > > > your specific requirements for your platform. > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > --- > > > tools/Makefile | 2 + > > > tools/mkeficapsule.c | 238 +++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 240 insertions(+) > > > create mode 100644 tools/mkeficapsule.c > > > > > > diff --git a/tools/Makefile b/tools/Makefile > > > index 51123fd92983..66d9376803e3 100644 > > > --- a/tools/Makefile > > > +++ b/tools/Makefile > > > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > > > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > > > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > > > > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > > > + > > > # We build some files with extra pedantic flags to try to minimize things > > > # that won't build on some weird host compiler -- though there are lots of > > > # exceptions for files that aren't complaint. > > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > > > new file mode 100644 > > > index 000000000000..db95426457cc > > > --- /dev/null > > > +++ b/tools/mkeficapsule.c > > > @@ -0,0 +1,238 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright 2018 Linaro Limited > > > + * Author: AKASHI Takahiro > > > + */ > > > + > > > +#include <getopt.h> > > > +#include <malloc.h> > > > +#include <stdbool.h> > > > +#include <stdio.h> > > > +#include <stdlib.h> > > > +#include <string.h> > > > +#include <linux/types.h> > > > +#include <sys/stat.h> > > > +#include <sys/types.h> > > > + > > > +typedef __u8 u8; > > > +typedef __u16 u16; > > > +typedef __u32 u32; > > > +typedef __u64 u64; > > > +typedef __s16 s16; > > > +typedef __s32 s32; > > > + > > > +#define aligned_u64 __aligned_u64 > > > + > > > +#ifndef __packed > > > +#define __packed __attribute__((packed)) > > > +#endif > > > + > > > +#include <efi.h> > > > +#include <efi_api.h> > > > + > > > +static const char *tool_name = "mkeficapsule"; > > > + > > > +efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > +efi_guid_t efi_guid_image_type_uboot_fit = > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > > +efi_guid_t efi_guid_image_type_uboot_raw = > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > > > + > > > +static struct option options[] = { > > > + {"fit", required_argument, NULL, 'f'}, > > > + {"raw", required_argument, NULL, 'r'}, > > > + {"index", required_argument, NULL, 'i'}, > > > + {"instance", required_argument, NULL, 'I'}, > > > + {"version", required_argument, NULL, 'v'}, > > > + {"help", no_argument, NULL, 'h'}, > > > + {NULL, 0, NULL, 0}, > > > +}; > > > + > > > +static void print_usage(void) > > > +{ > > > + printf("Usage: %s [options] <output file>\n" > > > + "Options:\n" > > > + "\t--fit <fit image> new FIT image file\n" > > > + "\t--raw <raw image> new raw image file\n" > > > + "\t--index <index> update image index\n" > > > + "\t--instance <instance> update hardware instance\n" > > > + "\t--version <version> firmware version\n" > > > + "\t--help print a help message\n", > > > + tool_name); > > > +} > > > + > > > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > > > + unsigned long version, unsigned long index, > > > + unsigned long instance) > > > +{ > > > + struct efi_capsule_header header; > > > + struct efi_firmware_management_capsule_header capsule; > > > + struct efi_firmware_management_capsule_image_header image; > > > + FILE *f, *g; > > > + struct stat bin_stat; > > > + u8 *data; > > > + size_t size; > > > + > > > +#ifdef DEBUG > > > + printf("For output: %s\n", path); > > > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > > > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > > > + version, index, instance); > > > +#endif > > > + > > > + g = fopen(bin, "r"); > > > + if (!g) { > > > + printf("cannot open %s\n", bin); > > > + return -1; > > > + } > > > + if (stat(bin, &bin_stat) < 0) { > > > + printf("cannot determine the size of %s\n", bin); > > > + goto err_1; > > > + } > > > + data = malloc(bin_stat.st_size); > > > + if (!data) { > > > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > > > + goto err_1; > > > + } > > > + f = fopen(path, "w"); > > > + if (!f) { > > > + printf("cannot open %s\n", path); > > > + goto err_2; > > > + } > > > + header.capsule_guid = efi_guid_fm_capsule; > > > + header.header_size = sizeof(header); > > > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > > > + header.capsule_image_size = sizeof(header) > > > + + sizeof(capsule) + sizeof(u64) > > > + + sizeof(image) > > > + + bin_stat.st_size; > > > + > > > + size = fwrite(&header, 1, sizeof(header), f); > > > + if (size < sizeof(header)) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + > > > + capsule.version = 0x00000001; > > > + capsule.embedded_driver_count = 0; > > > + capsule.payload_item_count = 1; > > > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > > > With the complete series applied, building sandbox_defconfig: > > > > tools/mkeficapsule.c: In function ‘main’: > > tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside array > > bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] > > 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > > > Please, ensure that the code compiles without warnings. > > Fixing this warning would be easy, but I didn't see it > with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. Oops, I found that this is not a warning, but a potential bug. The code does overrun a variable, capsule, allocated on the stack while it is apparently harmless as the neighboring variable, image, is initialized later. > So I wonder if it is mandatory that the code compiles without warnings, > and if so, which compiler and which version are required? Yet, this question remains. -Takahiro Akashi > > > > I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 > > system. > > > > Best regards > > > > Heinrich > > > > > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > > > + if (size < (sizeof(capsule) + sizeof(u64))) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + > > > + image.version = version; > > > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > > > + image.update_image_index = index; > > > + image.update_image_size = bin_stat.st_size; > > > + image.update_vendor_code_size = 0; /* none */ > > > + image.update_hardware_instance = instance; > > > + image.image_capsule_support = 0; > > > + > > > + size = fwrite(&image, 1, sizeof(image), f); > > > + if (size < sizeof(image)) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + size = fread(data, 1, bin_stat.st_size, g); > > > + if (size < bin_stat.st_size) { > > > + printf("read failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + size = fwrite(data, 1, bin_stat.st_size, f); > > > + if (size < bin_stat.st_size) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + > > > + fclose(f); > > > + fclose(g); > > > + free(data); > > > + > > > + return 0; > > > + > > > +err_3: > > > + fclose(f); > > > +err_2: > > > + free(data); > > > +err_1: > > > + fclose(g); > > > + > > > + return -1; > > > +} > > > + > > > +/* > > > + * Usage: > > > + * $ mkeficapsule -f <firmware binary> <output file> > > > + */ > > > +int main(int argc, char **argv) > > > +{ > > > + char *file; > > > + efi_guid_t *guid; > > > + unsigned long index, instance, version; > > > + int c, idx; > > > + > > > + file = NULL; > > > + guid = NULL; > > > + index = 0; > > > + instance = 0; > > > + version = 0; > > > + for (;;) { > > > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > > > + if (c == -1) > > > + break; > > > + > > > + switch (c) { > > > + case 'f': > > > + if (file) { > > > + printf("Image already specified\n"); > > > + return -1; > > > + } > > > + file = optarg; > > > + guid = &efi_guid_image_type_uboot_fit; > > > + break; > > > + case 'r': > > > + if (file) { > > > + printf("Image already specified\n"); > > > + return -1; > > > + } > > > + file = optarg; > > > + guid = &efi_guid_image_type_uboot_raw; > > > + break; > > > + case 'i': > > > + index = strtoul(optarg, NULL, 0); > > > + break; > > > + case 'I': > > > + instance = strtoul(optarg, NULL, 0); > > > + break; > > > + case 'v': > > > + version = strtoul(optarg, NULL, 0); > > > + break; > > > + case 'h': > > > + print_usage(); > > > + return 0; > > > + } > > > + } > > > + > > > + /* need a output file */ > > > + if (argc != optind + 1) { > > > + print_usage(); > > > + return -1; > > > + } > > > + > > > + /* need a fit image file or raw image file */ > > > + if (!file) { > > > + print_usage(); > > > + return -1; > > > + } > > > + > > > + if (create_fwbin(argv[optind], file, guid, version, index, instance) > > > + < 0) { > > > + printf("Creating firmware capsule failed\n"); > > > + return -1; > > > + } > > > + > > > + return 0; > > > +} > > > > >
Sughosh, On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > > This is a utility mainly for test purpose. > > mkeficapsule -f: create a test capsule file for FIT image firmware > > > > Having said that, you will be able to customize the code to fit > > your specific requirements for your platform. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > tools/Makefile | 2 + > > tools/mkeficapsule.c | 238 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 240 insertions(+) > > create mode 100644 tools/mkeficapsule.c > > > > diff --git a/tools/Makefile b/tools/Makefile > > index 51123fd92983..66d9376803e3 100644 > > --- a/tools/Makefile > > +++ b/tools/Makefile > > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > > + > > # We build some files with extra pedantic flags to try to minimize things > > # that won't build on some weird host compiler -- though there are lots of > > # exceptions for files that aren't complaint. > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > > new file mode 100644 > > index 000000000000..db95426457cc > > --- /dev/null > > +++ b/tools/mkeficapsule.c > > @@ -0,0 +1,238 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright 2018 Linaro Limited > > + * Author: AKASHI Takahiro > > + */ > > + > > +#include <getopt.h> > > +#include <malloc.h> > > +#include <stdbool.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <linux/types.h> > > +#include <sys/stat.h> > > +#include <sys/types.h> > > + > > +typedef __u8 u8; > > +typedef __u16 u16; > > +typedef __u32 u32; > > +typedef __u64 u64; > > +typedef __s16 s16; > > +typedef __s32 s32; > > + > > +#define aligned_u64 __aligned_u64 > > + > > +#ifndef __packed > > +#define __packed __attribute__((packed)) > > +#endif > > + > > +#include <efi.h> > > +#include <efi_api.h> > > + > > +static const char *tool_name = "mkeficapsule"; > > + > > +efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > +efi_guid_t efi_guid_image_type_uboot_fit = > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > +efi_guid_t efi_guid_image_type_uboot_raw = > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > > + > > +static struct option options[] = { > > + {"fit", required_argument, NULL, 'f'}, > > + {"raw", required_argument, NULL, 'r'}, > > + {"index", required_argument, NULL, 'i'}, > > + {"instance", required_argument, NULL, 'I'}, > > + {"version", required_argument, NULL, 'v'}, > > + {"help", no_argument, NULL, 'h'}, > > + {NULL, 0, NULL, 0}, > > +}; > > + > > +static void print_usage(void) > > +{ > > + printf("Usage: %s [options] <output file>\n" > > + "Options:\n" > > + "\t--fit <fit image> new FIT image file\n" > > + "\t--raw <raw image> new raw image file\n" > > + "\t--index <index> update image index\n" > > + "\t--instance <instance> update hardware instance\n" > > + "\t--version <version> firmware version\n" > > + "\t--help print a help message\n", > > + tool_name); > > +} > > + > > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > > + unsigned long version, unsigned long index, > > + unsigned long instance) > > +{ > > + struct efi_capsule_header header; > > + struct efi_firmware_management_capsule_header capsule; > > + struct efi_firmware_management_capsule_image_header image; > > + FILE *f, *g; > > + struct stat bin_stat; > > + u8 *data; > > + size_t size; > > + > > +#ifdef DEBUG > > + printf("For output: %s\n", path); > > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > > + version, index, instance); > > +#endif > > + > > + g = fopen(bin, "r"); > > + if (!g) { > > + printf("cannot open %s\n", bin); > > + return -1; > > + } > > + if (stat(bin, &bin_stat) < 0) { > > + printf("cannot determine the size of %s\n", bin); > > + goto err_1; > > + } > > + data = malloc(bin_stat.st_size); > > + if (!data) { > > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > > + goto err_1; > > + } > > + f = fopen(path, "w"); > > + if (!f) { > > + printf("cannot open %s\n", path); > > + goto err_2; > > + } > > + header.capsule_guid = efi_guid_fm_capsule; > > + header.header_size = sizeof(header); > > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > > + header.capsule_image_size = sizeof(header) > > + + sizeof(capsule) + sizeof(u64) > > + + sizeof(image) > > + + bin_stat.st_size; > > + > > + size = fwrite(&header, 1, sizeof(header), f); > > + if (size < sizeof(header)) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + > > + capsule.version = 0x00000001; > > + capsule.embedded_driver_count = 0; > > + capsule.payload_item_count = 1; > > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > With the complete series applied, building sandbox_defconfig: > > tools/mkeficapsule.c: In function ‘main’: > tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside array > bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] > 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > Please, ensure that the code compiles without warnings. > > I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 > system. > > Best regards > > Heinrich > > > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > > + if (size < (sizeof(capsule) + sizeof(u64))) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + > > + image.version = version; After some cleanup works, I found that the version was wrongly set here. The value of 'version' in this structure must be the one of this structure's layout, not the one for firmware itself. So I decided to remove "--version" option from the command. (Note U-Boot has no notion of "firmware version" anyway.) I hope that this change won't hinder your effort in extending this command for capsule authentication. Thanks, -Takahiro Akashi > > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > > + image.update_image_index = index; > > + image.update_image_size = bin_stat.st_size; > > + image.update_vendor_code_size = 0; /* none */ > > + image.update_hardware_instance = instance; > > + image.image_capsule_support = 0; > > + > > + size = fwrite(&image, 1, sizeof(image), f); > > + if (size < sizeof(image)) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + size = fread(data, 1, bin_stat.st_size, g); > > + if (size < bin_stat.st_size) { > > + printf("read failed (%lx)\n", size); > > + goto err_3; > > + } > > + size = fwrite(data, 1, bin_stat.st_size, f); > > + if (size < bin_stat.st_size) { > > + printf("write failed (%lx)\n", size); > > + goto err_3; > > + } > > + > > + fclose(f); > > + fclose(g); > > + free(data); > > + > > + return 0; > > + > > +err_3: > > + fclose(f); > > +err_2: > > + free(data); > > +err_1: > > + fclose(g); > > + > > + return -1; > > +} > > + > > +/* > > + * Usage: > > + * $ mkeficapsule -f <firmware binary> <output file> > > + */ > > +int main(int argc, char **argv) > > +{ > > + char *file; > > + efi_guid_t *guid; > > + unsigned long index, instance, version; > > + int c, idx; > > + > > + file = NULL; > > + guid = NULL; > > + index = 0; > > + instance = 0; > > + version = 0; > > + for (;;) { > > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > > + if (c == -1) > > + break; > > + > > + switch (c) { > > + case 'f': > > + if (file) { > > + printf("Image already specified\n"); > > + return -1; > > + } > > + file = optarg; > > + guid = &efi_guid_image_type_uboot_fit; > > + break; > > + case 'r': > > + if (file) { > > + printf("Image already specified\n"); > > + return -1; > > + } > > + file = optarg; > > + guid = &efi_guid_image_type_uboot_raw; > > + break; > > + case 'i': > > + index = strtoul(optarg, NULL, 0); > > + break; > > + case 'I': > > + instance = strtoul(optarg, NULL, 0); > > + break; > > + case 'v': > > + version = strtoul(optarg, NULL, 0); > > + break; > > + case 'h': > > + print_usage(); > > + return 0; > > + } > > + } > > + > > + /* need a output file */ > > + if (argc != optind + 1) { > > + print_usage(); > > + return -1; > > + } > > + > > + /* need a fit image file or raw image file */ > > + if (!file) { > > + print_usage(); > > + return -1; > > + } > > + > > + if (create_fwbin(argv[optind], file, guid, version, index, instance) > > + < 0) { > > + printf("Creating firmware capsule failed\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > >
Takahiro, On Wed, 25 Nov 2020 at 10:47, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > Sughosh, > > On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > > On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > > > This is a utility mainly for test purpose. > > > mkeficapsule -f: create a test capsule file for FIT image firmware > > > > > > Having said that, you will be able to customize the code to fit > > > your specific requirements for your platform. > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > --- > > > tools/Makefile | 2 + > > > tools/mkeficapsule.c | 238 > +++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 240 insertions(+) > > > create mode 100644 tools/mkeficapsule.c > > > > > > diff --git a/tools/Makefile b/tools/Makefile > > > index 51123fd92983..66d9376803e3 100644 > > > --- a/tools/Makefile > > > +++ b/tools/Makefile > > > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > > > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > > > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > > > > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > > > + > > > # We build some files with extra pedantic flags to try to minimize > things > > > # that won't build on some weird host compiler -- though there are > lots of > > > # exceptions for files that aren't complaint. > > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > > > new file mode 100644 > > > index 000000000000..db95426457cc > > > --- /dev/null > > > +++ b/tools/mkeficapsule.c > > > @@ -0,0 +1,238 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright 2018 Linaro Limited > > > + * Author: AKASHI Takahiro > > > + */ > > > + > > > +#include <getopt.h> > > > +#include <malloc.h> > > > +#include <stdbool.h> > > > +#include <stdio.h> > > > +#include <stdlib.h> > > > +#include <string.h> > > > +#include <linux/types.h> > > > +#include <sys/stat.h> > > > +#include <sys/types.h> > > > + > > > +typedef __u8 u8; > > > +typedef __u16 u16; > > > +typedef __u32 u32; > > > +typedef __u64 u64; > > > +typedef __s16 s16; > > > +typedef __s32 s32; > > > + > > > +#define aligned_u64 __aligned_u64 > > > + > > > +#ifndef __packed > > > +#define __packed __attribute__((packed)) > > > +#endif > > > + > > > +#include <efi.h> > > > +#include <efi_api.h> > > > + > > > +static const char *tool_name = "mkeficapsule"; > > > + > > > +efi_guid_t efi_guid_fm_capsule = > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > +efi_guid_t efi_guid_image_type_uboot_fit = > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > > +efi_guid_t efi_guid_image_type_uboot_raw = > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > > > + > > > +static struct option options[] = { > > > + {"fit", required_argument, NULL, 'f'}, > > > + {"raw", required_argument, NULL, 'r'}, > > > + {"index", required_argument, NULL, 'i'}, > > > + {"instance", required_argument, NULL, 'I'}, > > > + {"version", required_argument, NULL, 'v'}, > > > + {"help", no_argument, NULL, 'h'}, > > > + {NULL, 0, NULL, 0}, > > > +}; > > > + > > > +static void print_usage(void) > > > +{ > > > + printf("Usage: %s [options] <output file>\n" > > > + "Options:\n" > > > + "\t--fit <fit image> new FIT image file\n" > > > + "\t--raw <raw image> new raw image file\n" > > > + "\t--index <index> update image index\n" > > > + "\t--instance <instance> update hardware instance\n" > > > + "\t--version <version> firmware version\n" > > > + "\t--help print a help message\n", > > > + tool_name); > > > +} > > > + > > > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > > > + unsigned long version, unsigned long index, > > > + unsigned long instance) > > > +{ > > > + struct efi_capsule_header header; > > > + struct efi_firmware_management_capsule_header capsule; > > > + struct efi_firmware_management_capsule_image_header image; > > > + FILE *f, *g; > > > + struct stat bin_stat; > > > + u8 *data; > > > + size_t size; > > > + > > > +#ifdef DEBUG > > > + printf("For output: %s\n", path); > > > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > > > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > > > + version, index, instance); > > > +#endif > > > + > > > + g = fopen(bin, "r"); > > > + if (!g) { > > > + printf("cannot open %s\n", bin); > > > + return -1; > > > + } > > > + if (stat(bin, &bin_stat) < 0) { > > > + printf("cannot determine the size of %s\n", bin); > > > + goto err_1; > > > + } > > > + data = malloc(bin_stat.st_size); > > > + if (!data) { > > > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > > > + goto err_1; > > > + } > > > + f = fopen(path, "w"); > > > + if (!f) { > > > + printf("cannot open %s\n", path); > > > + goto err_2; > > > + } > > > + header.capsule_guid = efi_guid_fm_capsule; > > > + header.header_size = sizeof(header); > > > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > > > + header.capsule_image_size = sizeof(header) > > > + + sizeof(capsule) + sizeof(u64) > > > + + sizeof(image) > > > + + bin_stat.st_size; > > > + > > > + size = fwrite(&header, 1, sizeof(header), f); > > > + if (size < sizeof(header)) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + > > > + capsule.version = 0x00000001; > > > + capsule.embedded_driver_count = 0; > > > + capsule.payload_item_count = 1; > > > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > > > With the complete series applied, building sandbox_defconfig: > > > > tools/mkeficapsule.c: In function ‘main’: > > tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside array > > bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] > > 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > > > Please, ensure that the code compiles without warnings. > > > > I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 > > system. > > > > Best regards > > > > Heinrich > > > > > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > > > + if (size < (sizeof(capsule) + sizeof(u64))) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + > > > + image.version = version; > > After some cleanup works, I found that the version was wrongly > set here. The value of 'version' in this structure must be the > one of this structure's layout, not the one for firmware itself. > So I decided to remove "--version" option from the command. > (Note U-Boot has no notion of "firmware version" anyway.) > > I hope that this change won't hinder your effort in extending > this command for capsule authentication. > Your change to remove the 'version' option would not hinder my changes to add the public-key certificate to the dtb for capsule authentication. -sughosh > Thanks, > -Takahiro Akashi > > > > > > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > > > + image.update_image_index = index; > > > + image.update_image_size = bin_stat.st_size; > > > + image.update_vendor_code_size = 0; /* none */ > > > + image.update_hardware_instance = instance; > > > + image.image_capsule_support = 0; > > > + > > > + size = fwrite(&image, 1, sizeof(image), f); > > > + if (size < sizeof(image)) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + size = fread(data, 1, bin_stat.st_size, g); > > > + if (size < bin_stat.st_size) { > > > + printf("read failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + size = fwrite(data, 1, bin_stat.st_size, f); > > > + if (size < bin_stat.st_size) { > > > + printf("write failed (%lx)\n", size); > > > + goto err_3; > > > + } > > > + > > > + fclose(f); > > > + fclose(g); > > > + free(data); > > > + > > > + return 0; > > > + > > > +err_3: > > > + fclose(f); > > > +err_2: > > > + free(data); > > > +err_1: > > > + fclose(g); > > > + > > > + return -1; > > > +} > > > + > > > +/* > > > + * Usage: > > > + * $ mkeficapsule -f <firmware binary> <output file> > > > + */ > > > +int main(int argc, char **argv) > > > +{ > > > + char *file; > > > + efi_guid_t *guid; > > > + unsigned long index, instance, version; > > > + int c, idx; > > > + > > > + file = NULL; > > > + guid = NULL; > > > + index = 0; > > > + instance = 0; > > > + version = 0; > > > + for (;;) { > > > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > > > + if (c == -1) > > > + break; > > > + > > > + switch (c) { > > > + case 'f': > > > + if (file) { > > > + printf("Image already specified\n"); > > > + return -1; > > > + } > > > + file = optarg; > > > + guid = &efi_guid_image_type_uboot_fit; > > > + break; > > > + case 'r': > > > + if (file) { > > > + printf("Image already specified\n"); > > > + return -1; > > > + } > > > + file = optarg; > > > + guid = &efi_guid_image_type_uboot_raw; > > > + break; > > > + case 'i': > > > + index = strtoul(optarg, NULL, 0); > > > + break; > > > + case 'I': > > > + instance = strtoul(optarg, NULL, 0); > > > + break; > > > + case 'v': > > > + version = strtoul(optarg, NULL, 0); > > > + break; > > > + case 'h': > > > + print_usage(); > > > + return 0; > > > + } > > > + } > > > + > > > + /* need a output file */ > > > + if (argc != optind + 1) { > > > + print_usage(); > > > + return -1; > > > + } > > > + > > > + /* need a fit image file or raw image file */ > > > + if (!file) { > > > + print_usage(); > > > + return -1; > > > + } > > > + > > > + if (create_fwbin(argv[optind], file, guid, version, index, > instance) > > > + < 0) { > > > + printf("Creating firmware capsule failed\n"); > > > + return -1; > > > + } > > > + > > > + return 0; > > > +} > > > > > >
Am 25. November 2020 02:05:06 MEZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>: >Heinrich, > >On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: >> On 11/17/20 1:28 AM, AKASHI Takahiro wrote: >> > This is a utility mainly for test purpose. >> > mkeficapsule -f: create a test capsule file for FIT image >firmware >> > >> > Having said that, you will be able to customize the code to fit >> > your specific requirements for your platform. >> > >> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> > --- >> > tools/Makefile | 2 + >> > tools/mkeficapsule.c | 238 >+++++++++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 240 insertions(+) >> > create mode 100644 tools/mkeficapsule.c >> > >> > diff --git a/tools/Makefile b/tools/Makefile >> > index 51123fd92983..66d9376803e3 100644 >> > --- a/tools/Makefile >> > +++ b/tools/Makefile >> > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs >> > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler >> > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include >> > >> > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule >> > + >> > # We build some files with extra pedantic flags to try to >minimize things >> > # that won't build on some weird host compiler -- though there >are lots of >> > # exceptions for files that aren't complaint. >> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c >> > new file mode 100644 >> > index 000000000000..db95426457cc >> > --- /dev/null >> > +++ b/tools/mkeficapsule.c >> > @@ -0,0 +1,238 @@ >> > +// SPDX-License-Identifier: GPL-2.0 >> > +/* >> > + * Copyright 2018 Linaro Limited >> > + * Author: AKASHI Takahiro >> > + */ >> > + >> > +#include <getopt.h> >> > +#include <malloc.h> >> > +#include <stdbool.h> >> > +#include <stdio.h> >> > +#include <stdlib.h> >> > +#include <string.h> >> > +#include <linux/types.h> >> > +#include <sys/stat.h> >> > +#include <sys/types.h> >> > + >> > +typedef __u8 u8; >> > +typedef __u16 u16; >> > +typedef __u32 u32; >> > +typedef __u64 u64; >> > +typedef __s16 s16; >> > +typedef __s32 s32; >> > + >> > +#define aligned_u64 __aligned_u64 >> > + >> > +#ifndef __packed >> > +#define __packed __attribute__((packed)) >> > +#endif >> > + >> > +#include <efi.h> >> > +#include <efi_api.h> >> > + >> > +static const char *tool_name = "mkeficapsule"; >> > + >> > +efi_guid_t efi_guid_fm_capsule = >EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; >> > +efi_guid_t efi_guid_image_type_uboot_fit = >> > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; >> > +efi_guid_t efi_guid_image_type_uboot_raw = >> > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; >> > + >> > +static struct option options[] = { >> > + {"fit", required_argument, NULL, 'f'}, >> > + {"raw", required_argument, NULL, 'r'}, >> > + {"index", required_argument, NULL, 'i'}, >> > + {"instance", required_argument, NULL, 'I'}, >> > + {"version", required_argument, NULL, 'v'}, >> > + {"help", no_argument, NULL, 'h'}, >> > + {NULL, 0, NULL, 0}, >> > +}; >> > + >> > +static void print_usage(void) >> > +{ >> > + printf("Usage: %s [options] <output file>\n" >> > + "Options:\n" >> > + "\t--fit <fit image> new FIT image file\n" >> > + "\t--raw <raw image> new raw image file\n" >> > + "\t--index <index> update image index\n" >> > + "\t--instance <instance> update hardware instance\n" >> > + "\t--version <version> firmware version\n" >> > + "\t--help print a help message\n", >> > + tool_name); >> > +} >> > + >> > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, >> > + unsigned long version, unsigned long index, >> > + unsigned long instance) >> > +{ >> > + struct efi_capsule_header header; >> > + struct efi_firmware_management_capsule_header capsule; >> > + struct efi_firmware_management_capsule_image_header image; >> > + FILE *f, *g; >> > + struct stat bin_stat; >> > + u8 *data; >> > + size_t size; >> > + >> > +#ifdef DEBUG >> > + printf("For output: %s\n", path); >> > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); >> > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", >> > + version, index, instance); >> > +#endif >> > + >> > + g = fopen(bin, "r"); >> > + if (!g) { >> > + printf("cannot open %s\n", bin); >> > + return -1; >> > + } >> > + if (stat(bin, &bin_stat) < 0) { >> > + printf("cannot determine the size of %s\n", bin); >> > + goto err_1; >> > + } >> > + data = malloc(bin_stat.st_size); >> > + if (!data) { >> > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); >> > + goto err_1; >> > + } >> > + f = fopen(path, "w"); >> > + if (!f) { >> > + printf("cannot open %s\n", path); >> > + goto err_2; >> > + } >> > + header.capsule_guid = efi_guid_fm_capsule; >> > + header.header_size = sizeof(header); >> > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ >> > + header.capsule_image_size = sizeof(header) >> > + + sizeof(capsule) + sizeof(u64) >> > + + sizeof(image) >> > + + bin_stat.st_size; >> > + >> > + size = fwrite(&header, 1, sizeof(header), f); >> > + if (size < sizeof(header)) { >> > + printf("write failed (%lx)\n", size); >> > + goto err_3; >> > + } >> > + >> > + capsule.version = 0x00000001; >> > + capsule.embedded_driver_count = 0; >> > + capsule.payload_item_count = 1; >> > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); >> >> With the complete series applied, building sandbox_defconfig: >> >> tools/mkeficapsule.c: In function ‘main’: >> tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside >array >> bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] >> 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); >> | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ >> >> Please, ensure that the code compiles without warnings. > >Fixing this warning would be easy, but I didn't see it >with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. > >So I wonder if it is mandatory that the code compiles without warnings, >and if so, which compiler and which version are required? > >-Takahiro Akashi > Our settings for gitlab CI and Travis CI are that all warnings are treated as errors. So we must build without warning. Best regards Heinrich > >> I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 >> system. >> >> Best regards >> >> Heinrich >> >> > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); >> > + if (size < (sizeof(capsule) + sizeof(u64))) { >> > + printf("write failed (%lx)\n", size); >> > + goto err_3; >> > + } >> > + >> > + image.version = version; >> > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); >> > + image.update_image_index = index; >> > + image.update_image_size = bin_stat.st_size; >> > + image.update_vendor_code_size = 0; /* none */ >> > + image.update_hardware_instance = instance; >> > + image.image_capsule_support = 0; >> > + >> > + size = fwrite(&image, 1, sizeof(image), f); >> > + if (size < sizeof(image)) { >> > + printf("write failed (%lx)\n", size); >> > + goto err_3; >> > + } >> > + size = fread(data, 1, bin_stat.st_size, g); >> > + if (size < bin_stat.st_size) { >> > + printf("read failed (%lx)\n", size); >> > + goto err_3; >> > + } >> > + size = fwrite(data, 1, bin_stat.st_size, f); >> > + if (size < bin_stat.st_size) { >> > + printf("write failed (%lx)\n", size); >> > + goto err_3; >> > + } >> > + >> > + fclose(f); >> > + fclose(g); >> > + free(data); >> > + >> > + return 0; >> > + >> > +err_3: >> > + fclose(f); >> > +err_2: >> > + free(data); >> > +err_1: >> > + fclose(g); >> > + >> > + return -1; >> > +} >> > + >> > +/* >> > + * Usage: >> > + * $ mkeficapsule -f <firmware binary> <output file> >> > + */ >> > +int main(int argc, char **argv) >> > +{ >> > + char *file; >> > + efi_guid_t *guid; >> > + unsigned long index, instance, version; >> > + int c, idx; >> > + >> > + file = NULL; >> > + guid = NULL; >> > + index = 0; >> > + instance = 0; >> > + version = 0; >> > + for (;;) { >> > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); >> > + if (c == -1) >> > + break; >> > + >> > + switch (c) { >> > + case 'f': >> > + if (file) { >> > + printf("Image already specified\n"); >> > + return -1; >> > + } >> > + file = optarg; >> > + guid = &efi_guid_image_type_uboot_fit; >> > + break; >> > + case 'r': >> > + if (file) { >> > + printf("Image already specified\n"); >> > + return -1; >> > + } >> > + file = optarg; >> > + guid = &efi_guid_image_type_uboot_raw; >> > + break; >> > + case 'i': >> > + index = strtoul(optarg, NULL, 0); >> > + break; >> > + case 'I': >> > + instance = strtoul(optarg, NULL, 0); >> > + break; >> > + case 'v': >> > + version = strtoul(optarg, NULL, 0); >> > + break; >> > + case 'h': >> > + print_usage(); >> > + return 0; >> > + } >> > + } >> > + >> > + /* need a output file */ >> > + if (argc != optind + 1) { >> > + print_usage(); >> > + return -1; >> > + } >> > + >> > + /* need a fit image file or raw image file */ >> > + if (!file) { >> > + print_usage(); >> > + return -1; >> > + } >> > + >> > + if (create_fwbin(argv[optind], file, guid, version, index, >instance) >> > + < 0) { >> > + printf("Creating firmware capsule failed\n"); >> > + return -1; >> > + } >> > + >> > + return 0; >> > +} >> > >>
On Wed, Nov 25, 2020 at 12:01:11PM +0530, Sughosh Ganu wrote: > Takahiro, > > On Wed, 25 Nov 2020 at 10:47, AKASHI Takahiro <takahiro.akashi@linaro.org> > wrote: > > > Sughosh, > > > > On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > > > On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > > > > This is a utility mainly for test purpose. > > > > mkeficapsule -f: create a test capsule file for FIT image firmware > > > > > > > > Having said that, you will be able to customize the code to fit > > > > your specific requirements for your platform. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > --- > > > > tools/Makefile | 2 + > > > > tools/mkeficapsule.c | 238 > > +++++++++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 240 insertions(+) > > > > create mode 100644 tools/mkeficapsule.c > > > > > > > > diff --git a/tools/Makefile b/tools/Makefile > > > > index 51123fd92983..66d9376803e3 100644 > > > > --- a/tools/Makefile > > > > +++ b/tools/Makefile > > > > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > > > > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > > > > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > > > > > > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > > > > + > > > > # We build some files with extra pedantic flags to try to minimize > > things > > > > # that won't build on some weird host compiler -- though there are > > lots of > > > > # exceptions for files that aren't complaint. > > > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > > > > new file mode 100644 > > > > index 000000000000..db95426457cc > > > > --- /dev/null > > > > +++ b/tools/mkeficapsule.c > > > > @@ -0,0 +1,238 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * Copyright 2018 Linaro Limited > > > > + * Author: AKASHI Takahiro > > > > + */ > > > > + > > > > +#include <getopt.h> > > > > +#include <malloc.h> > > > > +#include <stdbool.h> > > > > +#include <stdio.h> > > > > +#include <stdlib.h> > > > > +#include <string.h> > > > > +#include <linux/types.h> > > > > +#include <sys/stat.h> > > > > +#include <sys/types.h> > > > > + > > > > +typedef __u8 u8; > > > > +typedef __u16 u16; > > > > +typedef __u32 u32; > > > > +typedef __u64 u64; > > > > +typedef __s16 s16; > > > > +typedef __s32 s32; > > > > + > > > > +#define aligned_u64 __aligned_u64 > > > > + > > > > +#ifndef __packed > > > > +#define __packed __attribute__((packed)) > > > > +#endif > > > > + > > > > +#include <efi.h> > > > > +#include <efi_api.h> > > > > + > > > > +static const char *tool_name = "mkeficapsule"; > > > > + > > > > +efi_guid_t efi_guid_fm_capsule = > > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > > +efi_guid_t efi_guid_image_type_uboot_fit = > > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > > > +efi_guid_t efi_guid_image_type_uboot_raw = > > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > > > > + > > > > +static struct option options[] = { > > > > + {"fit", required_argument, NULL, 'f'}, > > > > + {"raw", required_argument, NULL, 'r'}, > > > > + {"index", required_argument, NULL, 'i'}, > > > > + {"instance", required_argument, NULL, 'I'}, > > > > + {"version", required_argument, NULL, 'v'}, > > > > + {"help", no_argument, NULL, 'h'}, > > > > + {NULL, 0, NULL, 0}, > > > > +}; > > > > + > > > > +static void print_usage(void) > > > > +{ > > > > + printf("Usage: %s [options] <output file>\n" > > > > + "Options:\n" > > > > + "\t--fit <fit image> new FIT image file\n" > > > > + "\t--raw <raw image> new raw image file\n" > > > > + "\t--index <index> update image index\n" > > > > + "\t--instance <instance> update hardware instance\n" > > > > + "\t--version <version> firmware version\n" > > > > + "\t--help print a help message\n", > > > > + tool_name); > > > > +} > > > > + > > > > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > > > > + unsigned long version, unsigned long index, > > > > + unsigned long instance) > > > > +{ > > > > + struct efi_capsule_header header; > > > > + struct efi_firmware_management_capsule_header capsule; > > > > + struct efi_firmware_management_capsule_image_header image; > > > > + FILE *f, *g; > > > > + struct stat bin_stat; > > > > + u8 *data; > > > > + size_t size; > > > > + > > > > +#ifdef DEBUG > > > > + printf("For output: %s\n", path); > > > > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > > > > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > > > > + version, index, instance); > > > > +#endif > > > > + > > > > + g = fopen(bin, "r"); > > > > + if (!g) { > > > > + printf("cannot open %s\n", bin); > > > > + return -1; > > > > + } > > > > + if (stat(bin, &bin_stat) < 0) { > > > > + printf("cannot determine the size of %s\n", bin); > > > > + goto err_1; > > > > + } > > > > + data = malloc(bin_stat.st_size); > > > > + if (!data) { > > > > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > > > > + goto err_1; > > > > + } > > > > + f = fopen(path, "w"); > > > > + if (!f) { > > > > + printf("cannot open %s\n", path); > > > > + goto err_2; > > > > + } > > > > + header.capsule_guid = efi_guid_fm_capsule; > > > > + header.header_size = sizeof(header); > > > > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > > > > + header.capsule_image_size = sizeof(header) > > > > + + sizeof(capsule) + sizeof(u64) > > > > + + sizeof(image) > > > > + + bin_stat.st_size; > > > > + > > > > + size = fwrite(&header, 1, sizeof(header), f); > > > > + if (size < sizeof(header)) { > > > > + printf("write failed (%lx)\n", size); > > > > + goto err_3; > > > > + } > > > > + > > > > + capsule.version = 0x00000001; > > > > + capsule.embedded_driver_count = 0; > > > > + capsule.payload_item_count = 1; > > > > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > > > > > With the complete series applied, building sandbox_defconfig: > > > > > > tools/mkeficapsule.c: In function ‘main’: > > > tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside array > > > bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] > > > 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > > > > > Please, ensure that the code compiles without warnings. > > > > > > I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 > > > system. > > > > > > Best regards > > > > > > Heinrich > > > > > > > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > > > > + if (size < (sizeof(capsule) + sizeof(u64))) { > > > > + printf("write failed (%lx)\n", size); > > > > + goto err_3; > > > > + } > > > > + > > > > + image.version = version; > > > > After some cleanup works, I found that the version was wrongly > > set here. The value of 'version' in this structure must be the > > one of this structure's layout, not the one for firmware itself. > > So I decided to remove "--version" option from the command. > > (Note U-Boot has no notion of "firmware version" anyway.) > > > > I hope that this change won't hinder your effort in extending > > this command for capsule authentication. > > > > Your change to remove the 'version' option would not hinder my changes to > add the public-key certificate to the dtb for capsule authentication. Thank you for the confirmation. -Takahiro Akashi > -sughosh > > > > Thanks, > > -Takahiro Akashi > > > > > > > > > > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > > > > + image.update_image_index = index; > > > > + image.update_image_size = bin_stat.st_size; > > > > + image.update_vendor_code_size = 0; /* none */ > > > > + image.update_hardware_instance = instance; > > > > + image.image_capsule_support = 0; > > > > + > > > > + size = fwrite(&image, 1, sizeof(image), f); > > > > + if (size < sizeof(image)) { > > > > + printf("write failed (%lx)\n", size); > > > > + goto err_3; > > > > + } > > > > + size = fread(data, 1, bin_stat.st_size, g); > > > > + if (size < bin_stat.st_size) { > > > > + printf("read failed (%lx)\n", size); > > > > + goto err_3; > > > > + } > > > > + size = fwrite(data, 1, bin_stat.st_size, f); > > > > + if (size < bin_stat.st_size) { > > > > + printf("write failed (%lx)\n", size); > > > > + goto err_3; > > > > + } > > > > + > > > > + fclose(f); > > > > + fclose(g); > > > > + free(data); > > > > + > > > > + return 0; > > > > + > > > > +err_3: > > > > + fclose(f); > > > > +err_2: > > > > + free(data); > > > > +err_1: > > > > + fclose(g); > > > > + > > > > + return -1; > > > > +} > > > > + > > > > +/* > > > > + * Usage: > > > > + * $ mkeficapsule -f <firmware binary> <output file> > > > > + */ > > > > +int main(int argc, char **argv) > > > > +{ > > > > + char *file; > > > > + efi_guid_t *guid; > > > > + unsigned long index, instance, version; > > > > + int c, idx; > > > > + > > > > + file = NULL; > > > > + guid = NULL; > > > > + index = 0; > > > > + instance = 0; > > > > + version = 0; > > > > + for (;;) { > > > > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > > > > + if (c == -1) > > > > + break; > > > > + > > > > + switch (c) { > > > > + case 'f': > > > > + if (file) { > > > > + printf("Image already specified\n"); > > > > + return -1; > > > > + } > > > > + file = optarg; > > > > + guid = &efi_guid_image_type_uboot_fit; > > > > + break; > > > > + case 'r': > > > > + if (file) { > > > > + printf("Image already specified\n"); > > > > + return -1; > > > > + } > > > > + file = optarg; > > > > + guid = &efi_guid_image_type_uboot_raw; > > > > + break; > > > > + case 'i': > > > > + index = strtoul(optarg, NULL, 0); > > > > + break; > > > > + case 'I': > > > > + instance = strtoul(optarg, NULL, 0); > > > > + break; > > > > + case 'v': > > > > + version = strtoul(optarg, NULL, 0); > > > > + break; > > > > + case 'h': > > > > + print_usage(); > > > > + return 0; > > > > + } > > > > + } > > > > + > > > > + /* need a output file */ > > > > + if (argc != optind + 1) { > > > > + print_usage(); > > > > + return -1; > > > > + } > > > > + > > > > + /* need a fit image file or raw image file */ > > > > + if (!file) { > > > > + print_usage(); > > > > + return -1; > > > > + } > > > > + > > > > + if (create_fwbin(argv[optind], file, guid, version, index, > > instance) > > > > + < 0) { > > > > + printf("Creating firmware capsule failed\n"); > > > > + return -1; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > > > > > >
Heinrich, On Wed, Nov 25, 2020 at 07:42:31AM +0100, Heinrich Schuchardt wrote: > Am 25. November 2020 02:05:06 MEZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>: > >Heinrich, > > > >On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > >> On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > >> > This is a utility mainly for test purpose. > >> > mkeficapsule -f: create a test capsule file for FIT image > >firmware > >> > > >> > Having said that, you will be able to customize the code to fit > >> > your specific requirements for your platform. > >> > > >> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >> > --- > >> > tools/Makefile | 2 + > >> > tools/mkeficapsule.c | 238 > >+++++++++++++++++++++++++++++++++++++++++++ > >> > 2 files changed, 240 insertions(+) > >> > create mode 100644 tools/mkeficapsule.c > >> > > >> > diff --git a/tools/Makefile b/tools/Makefile > >> > index 51123fd92983..66d9376803e3 100644 > >> > --- a/tools/Makefile > >> > +++ b/tools/Makefile > >> > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > >> > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > >> > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > >> > > >> > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > >> > + > >> > # We build some files with extra pedantic flags to try to > >minimize things > >> > # that won't build on some weird host compiler -- though there > >are lots of > >> > # exceptions for files that aren't complaint. > >> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > >> > new file mode 100644 > >> > index 000000000000..db95426457cc > >> > --- /dev/null > >> > +++ b/tools/mkeficapsule.c > >> > @@ -0,0 +1,238 @@ > >> > +// SPDX-License-Identifier: GPL-2.0 > >> > +/* > >> > + * Copyright 2018 Linaro Limited > >> > + * Author: AKASHI Takahiro > >> > + */ > >> > + > >> > +#include <getopt.h> > >> > +#include <malloc.h> > >> > +#include <stdbool.h> > >> > +#include <stdio.h> > >> > +#include <stdlib.h> > >> > +#include <string.h> > >> > +#include <linux/types.h> > >> > +#include <sys/stat.h> > >> > +#include <sys/types.h> > >> > + > >> > +typedef __u8 u8; > >> > +typedef __u16 u16; > >> > +typedef __u32 u32; > >> > +typedef __u64 u64; > >> > +typedef __s16 s16; > >> > +typedef __s32 s32; > >> > + > >> > +#define aligned_u64 __aligned_u64 > >> > + > >> > +#ifndef __packed > >> > +#define __packed __attribute__((packed)) > >> > +#endif > >> > + > >> > +#include <efi.h> > >> > +#include <efi_api.h> > >> > + > >> > +static const char *tool_name = "mkeficapsule"; > >> > + > >> > +efi_guid_t efi_guid_fm_capsule = > >EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > >> > +efi_guid_t efi_guid_image_type_uboot_fit = > >> > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > >> > +efi_guid_t efi_guid_image_type_uboot_raw = > >> > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > >> > + > >> > +static struct option options[] = { > >> > + {"fit", required_argument, NULL, 'f'}, > >> > + {"raw", required_argument, NULL, 'r'}, > >> > + {"index", required_argument, NULL, 'i'}, > >> > + {"instance", required_argument, NULL, 'I'}, > >> > + {"version", required_argument, NULL, 'v'}, > >> > + {"help", no_argument, NULL, 'h'}, > >> > + {NULL, 0, NULL, 0}, > >> > +}; > >> > + > >> > +static void print_usage(void) > >> > +{ > >> > + printf("Usage: %s [options] <output file>\n" > >> > + "Options:\n" > >> > + "\t--fit <fit image> new FIT image file\n" > >> > + "\t--raw <raw image> new raw image file\n" > >> > + "\t--index <index> update image index\n" > >> > + "\t--instance <instance> update hardware instance\n" > >> > + "\t--version <version> firmware version\n" > >> > + "\t--help print a help message\n", > >> > + tool_name); > >> > +} > >> > + > >> > +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > >> > + unsigned long version, unsigned long index, > >> > + unsigned long instance) > >> > +{ > >> > + struct efi_capsule_header header; > >> > + struct efi_firmware_management_capsule_header capsule; > >> > + struct efi_firmware_management_capsule_image_header image; > >> > + FILE *f, *g; > >> > + struct stat bin_stat; > >> > + u8 *data; > >> > + size_t size; > >> > + > >> > +#ifdef DEBUG > >> > + printf("For output: %s\n", path); > >> > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > >> > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > >> > + version, index, instance); > >> > +#endif > >> > + > >> > + g = fopen(bin, "r"); > >> > + if (!g) { > >> > + printf("cannot open %s\n", bin); > >> > + return -1; > >> > + } > >> > + if (stat(bin, &bin_stat) < 0) { > >> > + printf("cannot determine the size of %s\n", bin); > >> > + goto err_1; > >> > + } > >> > + data = malloc(bin_stat.st_size); > >> > + if (!data) { > >> > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > >> > + goto err_1; > >> > + } > >> > + f = fopen(path, "w"); > >> > + if (!f) { > >> > + printf("cannot open %s\n", path); > >> > + goto err_2; > >> > + } > >> > + header.capsule_guid = efi_guid_fm_capsule; > >> > + header.header_size = sizeof(header); > >> > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > >> > + header.capsule_image_size = sizeof(header) > >> > + + sizeof(capsule) + sizeof(u64) > >> > + + sizeof(image) > >> > + + bin_stat.st_size; > >> > + > >> > + size = fwrite(&header, 1, sizeof(header), f); > >> > + if (size < sizeof(header)) { > >> > + printf("write failed (%lx)\n", size); > >> > + goto err_3; > >> > + } > >> > + > >> > + capsule.version = 0x00000001; > >> > + capsule.embedded_driver_count = 0; > >> > + capsule.payload_item_count = 1; > >> > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > >> > >> With the complete series applied, building sandbox_defconfig: > >> > >> tools/mkeficapsule.c: In function ‘main’: > >> tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside > >array > >> bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} [-Warray-bounds] > >> 120 | capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > >> | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > >> > >> Please, ensure that the code compiles without warnings. > > > >Fixing this warning would be easy, but I didn't see it > >with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. > > > >So I wonder if it is mandatory that the code compiles without warnings, > >and if so, which compiler and which version are required? First, can you please make a comment here against my question? > > > >-Takahiro Akashi > > > > Our settings for gitlab CI and Travis CI are that all warnings are treated as errors. As I said, I've never seen this warning/error in Travis CI. I don't know how we can confirm the result of gitlab CI. -Takahiro Akashi > So we must build without warning. > > Best regards > > Heinrich > > > > >> I have been using GCC 10.2 as supplied by Debian Bullseye on an ARM64 > >> system. > >> > >> Best regards > >> > >> Heinrich > >> > >> > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > >> > + if (size < (sizeof(capsule) + sizeof(u64))) { > >> > + printf("write failed (%lx)\n", size); > >> > + goto err_3; > >> > + } > >> > + > >> > + image.version = version; > >> > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > >> > + image.update_image_index = index; > >> > + image.update_image_size = bin_stat.st_size; > >> > + image.update_vendor_code_size = 0; /* none */ > >> > + image.update_hardware_instance = instance; > >> > + image.image_capsule_support = 0; > >> > + > >> > + size = fwrite(&image, 1, sizeof(image), f); > >> > + if (size < sizeof(image)) { > >> > + printf("write failed (%lx)\n", size); > >> > + goto err_3; > >> > + } > >> > + size = fread(data, 1, bin_stat.st_size, g); > >> > + if (size < bin_stat.st_size) { > >> > + printf("read failed (%lx)\n", size); > >> > + goto err_3; > >> > + } > >> > + size = fwrite(data, 1, bin_stat.st_size, f); > >> > + if (size < bin_stat.st_size) { > >> > + printf("write failed (%lx)\n", size); > >> > + goto err_3; > >> > + } > >> > + > >> > + fclose(f); > >> > + fclose(g); > >> > + free(data); > >> > + > >> > + return 0; > >> > + > >> > +err_3: > >> > + fclose(f); > >> > +err_2: > >> > + free(data); > >> > +err_1: > >> > + fclose(g); > >> > + > >> > + return -1; > >> > +} > >> > + > >> > +/* > >> > + * Usage: > >> > + * $ mkeficapsule -f <firmware binary> <output file> > >> > + */ > >> > +int main(int argc, char **argv) > >> > +{ > >> > + char *file; > >> > + efi_guid_t *guid; > >> > + unsigned long index, instance, version; > >> > + int c, idx; > >> > + > >> > + file = NULL; > >> > + guid = NULL; > >> > + index = 0; > >> > + instance = 0; > >> > + version = 0; > >> > + for (;;) { > >> > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > >> > + if (c == -1) > >> > + break; > >> > + > >> > + switch (c) { > >> > + case 'f': > >> > + if (file) { > >> > + printf("Image already specified\n"); > >> > + return -1; > >> > + } > >> > + file = optarg; > >> > + guid = &efi_guid_image_type_uboot_fit; > >> > + break; > >> > + case 'r': > >> > + if (file) { > >> > + printf("Image already specified\n"); > >> > + return -1; > >> > + } > >> > + file = optarg; > >> > + guid = &efi_guid_image_type_uboot_raw; > >> > + break; > >> > + case 'i': > >> > + index = strtoul(optarg, NULL, 0); > >> > + break; > >> > + case 'I': > >> > + instance = strtoul(optarg, NULL, 0); > >> > + break; > >> > + case 'v': > >> > + version = strtoul(optarg, NULL, 0); > >> > + break; > >> > + case 'h': > >> > + print_usage(); > >> > + return 0; > >> > + } > >> > + } > >> > + > >> > + /* need a output file */ > >> > + if (argc != optind + 1) { > >> > + print_usage(); > >> > + return -1; > >> > + } > >> > + > >> > + /* need a fit image file or raw image file */ > >> > + if (!file) { > >> > + print_usage(); > >> > + return -1; > >> > + } > >> > + > >> > + if (create_fwbin(argv[optind], file, guid, version, index, > >instance) > >> > + < 0) { > >> > + printf("Creating firmware capsule failed\n"); > >> > + return -1; > >> > + } > >> > + > >> > + return 0; > >> > +} > >> > > >> >
Am 25. November 2020 08:32:08 MEZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>: >Heinrich, > >On Wed, Nov 25, 2020 at 07:42:31AM +0100, Heinrich Schuchardt wrote: >> Am 25. November 2020 02:05:06 MEZ schrieb AKASHI Takahiro ><takahiro.akashi@linaro.org>: >> >Heinrich, >> > >> >On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: >> >> On 11/17/20 1:28 AM, AKASHI Takahiro wrote: >> >> > This is a utility mainly for test purpose. >> >> > mkeficapsule -f: create a test capsule file for FIT image >> >firmware >> >> > >> >> > Having said that, you will be able to customize the code to fit >> >> > your specific requirements for your platform. >> >> > >> >> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> >> > --- >> >> > tools/Makefile | 2 + >> >> > tools/mkeficapsule.c | 238 >> >+++++++++++++++++++++++++++++++++++++++++++ >> >> > 2 files changed, 240 insertions(+) >> >> > create mode 100644 tools/mkeficapsule.c >> >> > >> >> > diff --git a/tools/Makefile b/tools/Makefile >> >> > index 51123fd92983..66d9376803e3 100644 >> >> > --- a/tools/Makefile >> >> > +++ b/tools/Makefile >> >> > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs >> >> > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler >> >> > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include >> >> > >> >> > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule >> >> > + >> >> > # We build some files with extra pedantic flags to try to >> >minimize things >> >> > # that won't build on some weird host compiler -- though there >> >are lots of >> >> > # exceptions for files that aren't complaint. >> >> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c >> >> > new file mode 100644 >> >> > index 000000000000..db95426457cc >> >> > --- /dev/null >> >> > +++ b/tools/mkeficapsule.c >> >> > @@ -0,0 +1,238 @@ >> >> > +// SPDX-License-Identifier: GPL-2.0 >> >> > +/* >> >> > + * Copyright 2018 Linaro Limited >> >> > + * Author: AKASHI Takahiro >> >> > + */ >> >> > + >> >> > +#include <getopt.h> >> >> > +#include <malloc.h> >> >> > +#include <stdbool.h> >> >> > +#include <stdio.h> >> >> > +#include <stdlib.h> >> >> > +#include <string.h> >> >> > +#include <linux/types.h> >> >> > +#include <sys/stat.h> >> >> > +#include <sys/types.h> >> >> > + >> >> > +typedef __u8 u8; >> >> > +typedef __u16 u16; >> >> > +typedef __u32 u32; >> >> > +typedef __u64 u64; >> >> > +typedef __s16 s16; >> >> > +typedef __s32 s32; >> >> > + >> >> > +#define aligned_u64 __aligned_u64 >> >> > + >> >> > +#ifndef __packed >> >> > +#define __packed __attribute__((packed)) >> >> > +#endif >> >> > + >> >> > +#include <efi.h> >> >> > +#include <efi_api.h> >> >> > + >> >> > +static const char *tool_name = "mkeficapsule"; >> >> > + >> >> > +efi_guid_t efi_guid_fm_capsule = >> >EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; >> >> > +efi_guid_t efi_guid_image_type_uboot_fit = >> >> > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; >> >> > +efi_guid_t efi_guid_image_type_uboot_raw = >> >> > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; >> >> > + >> >> > +static struct option options[] = { >> >> > + {"fit", required_argument, NULL, 'f'}, >> >> > + {"raw", required_argument, NULL, 'r'}, >> >> > + {"index", required_argument, NULL, 'i'}, >> >> > + {"instance", required_argument, NULL, 'I'}, >> >> > + {"version", required_argument, NULL, 'v'}, >> >> > + {"help", no_argument, NULL, 'h'}, >> >> > + {NULL, 0, NULL, 0}, >> >> > +}; >> >> > + >> >> > +static void print_usage(void) >> >> > +{ >> >> > + printf("Usage: %s [options] <output file>\n" >> >> > + "Options:\n" >> >> > + "\t--fit <fit image> new FIT image file\n" >> >> > + "\t--raw <raw image> new raw image file\n" >> >> > + "\t--index <index> update image index\n" >> >> > + "\t--instance <instance> update hardware instance\n" >> >> > + "\t--version <version> firmware version\n" >> >> > + "\t--help print a help message\n", >> >> > + tool_name); >> >> > +} >> >> > + >> >> > +static int create_fwbin(char *path, char *bin, efi_guid_t >*guid, >> >> > + unsigned long version, unsigned long index, >> >> > + unsigned long instance) >> >> > +{ >> >> > + struct efi_capsule_header header; >> >> > + struct efi_firmware_management_capsule_header capsule; >> >> > + struct efi_firmware_management_capsule_image_header image; >> >> > + FILE *f, *g; >> >> > + struct stat bin_stat; >> >> > + u8 *data; >> >> > + size_t size; >> >> > + >> >> > +#ifdef DEBUG >> >> > + printf("For output: %s\n", path); >> >> > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); >> >> > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", >> >> > + version, index, instance); >> >> > +#endif >> >> > + >> >> > + g = fopen(bin, "r"); >> >> > + if (!g) { >> >> > + printf("cannot open %s\n", bin); >> >> > + return -1; >> >> > + } >> >> > + if (stat(bin, &bin_stat) < 0) { >> >> > + printf("cannot determine the size of %s\n", bin); >> >> > + goto err_1; >> >> > + } >> >> > + data = malloc(bin_stat.st_size); >> >> > + if (!data) { >> >> > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); >> >> > + goto err_1; >> >> > + } >> >> > + f = fopen(path, "w"); >> >> > + if (!f) { >> >> > + printf("cannot open %s\n", path); >> >> > + goto err_2; >> >> > + } >> >> > + header.capsule_guid = efi_guid_fm_capsule; >> >> > + header.header_size = sizeof(header); >> >> > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ >> >> > + header.capsule_image_size = sizeof(header) >> >> > + + sizeof(capsule) + sizeof(u64) >> >> > + + sizeof(image) >> >> > + + bin_stat.st_size; >> >> > + >> >> > + size = fwrite(&header, 1, sizeof(header), f); >> >> > + if (size < sizeof(header)) { >> >> > + printf("write failed (%lx)\n", size); >> >> > + goto err_3; >> >> > + } >> >> > + >> >> > + capsule.version = 0x00000001; >> >> > + capsule.embedded_driver_count = 0; >> >> > + capsule.payload_item_count = 1; >> >> > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); >> >> >> >> With the complete series applied, building sandbox_defconfig: >> >> >> >> tools/mkeficapsule.c: In function ‘main’: >> >> tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside >> >array >> >> bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} >[-Warray-bounds] >> >> 120 | capsule.item_offset_list[0] = sizeof(capsule) + >sizeof(u64); >> >> | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ >> >> >> >> Please, ensure that the code compiles without warnings. >> > >> >Fixing this warning would be easy, but I didn't see it >> >with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. >> > >> >So I wonder if it is mandatory that the code compiles without >warnings, >> >and if so, which compiler and which version are required? > >First, can you please make a comment here against my question? > >> > >> >-Takahiro Akashi >> > >> >> Our settings for gitlab CI and Travis CI are that all warnings are >treated as errors. > >As I said, I've never seen this warning/error in Travis CI. >I don't know how we can confirm the result of gitlab CI. > >-Takahiro Akashi Just make sure that GCC 10+ does not complain locally. Tom will update the CI in January. I dont want to see a build error then. Best regards Heinrich > > >> So we must build without warning. >> >> Best regards >> >> Heinrich >> >> > >> >> I have been using GCC 10.2 as supplied by Debian Bullseye on an >ARM64 >> >> system. >> >> >> >> Best regards >> >> >> >> Heinrich >> >> >> >> > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); >> >> > + if (size < (sizeof(capsule) + sizeof(u64))) { >> >> > + printf("write failed (%lx)\n", size); >> >> > + goto err_3; >> >> > + } >> >> > + >> >> > + image.version = version; >> >> > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); >> >> > + image.update_image_index = index; >> >> > + image.update_image_size = bin_stat.st_size; >> >> > + image.update_vendor_code_size = 0; /* none */ >> >> > + image.update_hardware_instance = instance; >> >> > + image.image_capsule_support = 0; >> >> > + >> >> > + size = fwrite(&image, 1, sizeof(image), f); >> >> > + if (size < sizeof(image)) { >> >> > + printf("write failed (%lx)\n", size); >> >> > + goto err_3; >> >> > + } >> >> > + size = fread(data, 1, bin_stat.st_size, g); >> >> > + if (size < bin_stat.st_size) { >> >> > + printf("read failed (%lx)\n", size); >> >> > + goto err_3; >> >> > + } >> >> > + size = fwrite(data, 1, bin_stat.st_size, f); >> >> > + if (size < bin_stat.st_size) { >> >> > + printf("write failed (%lx)\n", size); >> >> > + goto err_3; >> >> > + } >> >> > + >> >> > + fclose(f); >> >> > + fclose(g); >> >> > + free(data); >> >> > + >> >> > + return 0; >> >> > + >> >> > +err_3: >> >> > + fclose(f); >> >> > +err_2: >> >> > + free(data); >> >> > +err_1: >> >> > + fclose(g); >> >> > + >> >> > + return -1; >> >> > +} >> >> > + >> >> > +/* >> >> > + * Usage: >> >> > + * $ mkeficapsule -f <firmware binary> <output file> >> >> > + */ >> >> > +int main(int argc, char **argv) >> >> > +{ >> >> > + char *file; >> >> > + efi_guid_t *guid; >> >> > + unsigned long index, instance, version; >> >> > + int c, idx; >> >> > + >> >> > + file = NULL; >> >> > + guid = NULL; >> >> > + index = 0; >> >> > + instance = 0; >> >> > + version = 0; >> >> > + for (;;) { >> >> > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); >> >> > + if (c == -1) >> >> > + break; >> >> > + >> >> > + switch (c) { >> >> > + case 'f': >> >> > + if (file) { >> >> > + printf("Image already specified\n"); >> >> > + return -1; >> >> > + } >> >> > + file = optarg; >> >> > + guid = &efi_guid_image_type_uboot_fit; >> >> > + break; >> >> > + case 'r': >> >> > + if (file) { >> >> > + printf("Image already specified\n"); >> >> > + return -1; >> >> > + } >> >> > + file = optarg; >> >> > + guid = &efi_guid_image_type_uboot_raw; >> >> > + break; >> >> > + case 'i': >> >> > + index = strtoul(optarg, NULL, 0); >> >> > + break; >> >> > + case 'I': >> >> > + instance = strtoul(optarg, NULL, 0); >> >> > + break; >> >> > + case 'v': >> >> > + version = strtoul(optarg, NULL, 0); >> >> > + break; >> >> > + case 'h': >> >> > + print_usage(); >> >> > + return 0; >> >> > + } >> >> > + } >> >> > + >> >> > + /* need a output file */ >> >> > + if (argc != optind + 1) { >> >> > + print_usage(); >> >> > + return -1; >> >> > + } >> >> > + >> >> > + /* need a fit image file or raw image file */ >> >> > + if (!file) { >> >> > + print_usage(); >> >> > + return -1; >> >> > + } >> >> > + >> >> > + if (create_fwbin(argv[optind], file, guid, version, index, >> >instance) >> >> > + < 0) { >> >> > + printf("Creating firmware capsule failed\n"); >> >> > + return -1; >> >> > + } >> >> > + >> >> > + return 0; >> >> > +} >> >> > >> >> >>
On 11/27/20 3:22 PM, Heinrich Schuchardt wrote: > Am 25. November 2020 08:32:08 MEZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>: >> Heinrich, >> >> On Wed, Nov 25, 2020 at 07:42:31AM +0100, Heinrich Schuchardt wrote: >>> Am 25. November 2020 02:05:06 MEZ schrieb AKASHI Takahiro >> <takahiro.akashi@linaro.org>: >>>> Heinrich, >>>> >>>> On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: >>>>> On 11/17/20 1:28 AM, AKASHI Takahiro wrote: >>>>>> This is a utility mainly for test purpose. >>>>>> mkeficapsule -f: create a test capsule file for FIT image >>>> firmware >>>>>> >>>>>> Having said that, you will be able to customize the code to fit >>>>>> your specific requirements for your platform. >>>>>> >>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>>>>> --- >>>>>> tools/Makefile | 2 + >>>>>> tools/mkeficapsule.c | 238 >>>> +++++++++++++++++++++++++++++++++++++++++++ >>>>>> 2 files changed, 240 insertions(+) >>>>>> create mode 100644 tools/mkeficapsule.c >>>>>> >>>>>> diff --git a/tools/Makefile b/tools/Makefile >>>>>> index 51123fd92983..66d9376803e3 100644 >>>>>> --- a/tools/Makefile >>>>>> +++ b/tools/Makefile >>>>>> @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs >>>>>> hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler >>>>>> HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include >>>>>> >>>>>> +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule >>>>>> + >>>>>> # We build some files with extra pedantic flags to try to >>>> minimize things >>>>>> # that won't build on some weird host compiler -- though there >>>> are lots of >>>>>> # exceptions for files that aren't complaint. >>>>>> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c >>>>>> new file mode 100644 >>>>>> index 000000000000..db95426457cc >>>>>> --- /dev/null >>>>>> +++ b/tools/mkeficapsule.c >>>>>> @@ -0,0 +1,238 @@ >>>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>>> +/* >>>>>> + * Copyright 2018 Linaro Limited >>>>>> + * Author: AKASHI Takahiro >>>>>> + */ >>>>>> + >>>>>> +#include <getopt.h> >>>>>> +#include <malloc.h> >>>>>> +#include <stdbool.h> >>>>>> +#include <stdio.h> >>>>>> +#include <stdlib.h> >>>>>> +#include <string.h> >>>>>> +#include <linux/types.h> >>>>>> +#include <sys/stat.h> >>>>>> +#include <sys/types.h> >>>>>> + >>>>>> +typedef __u8 u8; >>>>>> +typedef __u16 u16; >>>>>> +typedef __u32 u32; >>>>>> +typedef __u64 u64; >>>>>> +typedef __s16 s16; >>>>>> +typedef __s32 s32; >>>>>> + >>>>>> +#define aligned_u64 __aligned_u64 >>>>>> + >>>>>> +#ifndef __packed >>>>>> +#define __packed __attribute__((packed)) >>>>>> +#endif >>>>>> + >>>>>> +#include <efi.h> >>>>>> +#include <efi_api.h> >>>>>> + >>>>>> +static const char *tool_name = "mkeficapsule"; >>>>>> + >>>>>> +efi_guid_t efi_guid_fm_capsule = >>>> EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; >>>>>> +efi_guid_t efi_guid_image_type_uboot_fit = >>>>>> + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; >>>>>> +efi_guid_t efi_guid_image_type_uboot_raw = >>>>>> + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; >>>>>> + >>>>>> +static struct option options[] = { >>>>>> + {"fit", required_argument, NULL, 'f'}, >>>>>> + {"raw", required_argument, NULL, 'r'}, >>>>>> + {"index", required_argument, NULL, 'i'}, >>>>>> + {"instance", required_argument, NULL, 'I'}, >>>>>> + {"version", required_argument, NULL, 'v'}, >>>>>> + {"help", no_argument, NULL, 'h'}, >>>>>> + {NULL, 0, NULL, 0}, >>>>>> +}; >>>>>> + >>>>>> +static void print_usage(void) >>>>>> +{ >>>>>> + printf("Usage: %s [options] <output file>\n" >>>>>> + "Options:\n" >>>>>> + "\t--fit <fit image> new FIT image file\n" >>>>>> + "\t--raw <raw image> new raw image file\n" >>>>>> + "\t--index <index> update image index\n" >>>>>> + "\t--instance <instance> update hardware instance\n" >>>>>> + "\t--version <version> firmware version\n" >>>>>> + "\t--help print a help message\n", >>>>>> + tool_name); >>>>>> +} >>>>>> + >>>>>> +static int create_fwbin(char *path, char *bin, efi_guid_t >> *guid, >>>>>> + unsigned long version, unsigned long index, >>>>>> + unsigned long instance) >>>>>> +{ >>>>>> + struct efi_capsule_header header; >>>>>> + struct efi_firmware_management_capsule_header capsule; >>>>>> + struct efi_firmware_management_capsule_image_header image; >>>>>> + FILE *f, *g; >>>>>> + struct stat bin_stat; >>>>>> + u8 *data; >>>>>> + size_t size; >>>>>> + >>>>>> +#ifdef DEBUG >>>>>> + printf("For output: %s\n", path); >>>>>> + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); >>>>>> + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", >>>>>> + version, index, instance); >>>>>> +#endif >>>>>> + >>>>>> + g = fopen(bin, "r"); >>>>>> + if (!g) { >>>>>> + printf("cannot open %s\n", bin); >>>>>> + return -1; >>>>>> + } >>>>>> + if (stat(bin, &bin_stat) < 0) { >>>>>> + printf("cannot determine the size of %s\n", bin); >>>>>> + goto err_1; >>>>>> + } >>>>>> + data = malloc(bin_stat.st_size); >>>>>> + if (!data) { >>>>>> + printf("cannot allocate memory: %lx\n", bin_stat.st_size); >>>>>> + goto err_1; >>>>>> + } >>>>>> + f = fopen(path, "w"); >>>>>> + if (!f) { >>>>>> + printf("cannot open %s\n", path); >>>>>> + goto err_2; >>>>>> + } >>>>>> + header.capsule_guid = efi_guid_fm_capsule; >>>>>> + header.header_size = sizeof(header); >>>>>> + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ >>>>>> + header.capsule_image_size = sizeof(header) >>>>>> + + sizeof(capsule) + sizeof(u64) >>>>>> + + sizeof(image) >>>>>> + + bin_stat.st_size; >>>>>> + >>>>>> + size = fwrite(&header, 1, sizeof(header), f); >>>>>> + if (size < sizeof(header)) { >>>>>> + printf("write failed (%lx)\n", size); >>>>>> + goto err_3; >>>>>> + } >>>>>> + >>>>>> + capsule.version = 0x00000001; >>>>>> + capsule.embedded_driver_count = 0; >>>>>> + capsule.payload_item_count = 1; >>>>>> + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); >>>>> >>>>> With the complete series applied, building sandbox_defconfig: >>>>> >>>>> tools/mkeficapsule.c: In function ‘main’: >>>>> tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside >>>> array >>>>> bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} >> [-Warray-bounds] >>>>> 120 | capsule.item_offset_list[0] = sizeof(capsule) + >> sizeof(u64); >>>>> | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ >>>>> >>>>> Please, ensure that the code compiles without warnings. >>>> >>>> Fixing this warning would be easy, but I didn't see it >>>> with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. >>>> >>>> So I wonder if it is mandatory that the code compiles without >> warnings, >>>> and if so, which compiler and which version are required? >> >> First, can you please make a comment here against my question? >> >>>> >>>> -Takahiro Akashi >>>> >>> >>> Our settings for gitlab CI and Travis CI are that all warnings are >> treated as errors. >> >> As I said, I've never seen this warning/error in Travis CI. >> I don't know how we can confirm the result of gitlab CI. >> >> -Takahiro Akashi > > > Just make sure that GCC 10+ does not complain locally. > > Tom will update the CI in January. I dont want to see a build error then. > > Best regards > > Heinrich Dear Takahiro, reading through your code it is obvious that this in not only a stray warning by GCC but a veritable bug in your patch. You define capsule as: 69 struct efi_firmware_management_capsule_header capsule; capsule.item_offset_list[] has zero bytes. Here you write outside of your structure: 120 capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); To solve this you could make capsule a pointer. struct efi_firmware_management_capsule_header *capsule; capsule = malloc(sizeof(struct efi_firmware_management_capsule_header) + sizeof(u64)); if (!capusule) goto err; capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); Best regards Heinrich > >> >> >>> So we must build without warning. >>> >>> Best regards >>> >>> Heinrich >>> >>>> >>>>> I have been using GCC 10.2 as supplied by Debian Bullseye on an >> ARM64 >>>>> system. >>>>> >>>>> Best regards >>>>> >>>>> Heinrich >>>>> >>>>>> + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); >>>>>> + if (size < (sizeof(capsule) + sizeof(u64))) { >>>>>> + printf("write failed (%lx)\n", size); >>>>>> + goto err_3; >>>>>> + } >>>>>> + >>>>>> + image.version = version; >>>>>> + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); >>>>>> + image.update_image_index = index; >>>>>> + image.update_image_size = bin_stat.st_size; >>>>>> + image.update_vendor_code_size = 0; /* none */ >>>>>> + image.update_hardware_instance = instance; >>>>>> + image.image_capsule_support = 0; >>>>>> + >>>>>> + size = fwrite(&image, 1, sizeof(image), f); >>>>>> + if (size < sizeof(image)) { >>>>>> + printf("write failed (%lx)\n", size); >>>>>> + goto err_3; >>>>>> + } >>>>>> + size = fread(data, 1, bin_stat.st_size, g); >>>>>> + if (size < bin_stat.st_size) { >>>>>> + printf("read failed (%lx)\n", size); >>>>>> + goto err_3; >>>>>> + } >>>>>> + size = fwrite(data, 1, bin_stat.st_size, f); >>>>>> + if (size < bin_stat.st_size) { >>>>>> + printf("write failed (%lx)\n", size); >>>>>> + goto err_3; >>>>>> + } >>>>>> + >>>>>> + fclose(f); >>>>>> + fclose(g); >>>>>> + free(data); >>>>>> + >>>>>> + return 0; >>>>>> + >>>>>> +err_3: >>>>>> + fclose(f); >>>>>> +err_2: >>>>>> + free(data); >>>>>> +err_1: >>>>>> + fclose(g); >>>>>> + >>>>>> + return -1; >>>>>> +} >>>>>> + >>>>>> +/* >>>>>> + * Usage: >>>>>> + * $ mkeficapsule -f <firmware binary> <output file> >>>>>> + */ >>>>>> +int main(int argc, char **argv) >>>>>> +{ >>>>>> + char *file; >>>>>> + efi_guid_t *guid; >>>>>> + unsigned long index, instance, version; >>>>>> + int c, idx; >>>>>> + >>>>>> + file = NULL; >>>>>> + guid = NULL; >>>>>> + index = 0; >>>>>> + instance = 0; >>>>>> + version = 0; >>>>>> + for (;;) { >>>>>> + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); >>>>>> + if (c == -1) >>>>>> + break; >>>>>> + >>>>>> + switch (c) { >>>>>> + case 'f': >>>>>> + if (file) { >>>>>> + printf("Image already specified\n"); >>>>>> + return -1; >>>>>> + } >>>>>> + file = optarg; >>>>>> + guid = &efi_guid_image_type_uboot_fit; >>>>>> + break; >>>>>> + case 'r': >>>>>> + if (file) { >>>>>> + printf("Image already specified\n"); >>>>>> + return -1; >>>>>> + } >>>>>> + file = optarg; >>>>>> + guid = &efi_guid_image_type_uboot_raw; >>>>>> + break; >>>>>> + case 'i': >>>>>> + index = strtoul(optarg, NULL, 0); >>>>>> + break; >>>>>> + case 'I': >>>>>> + instance = strtoul(optarg, NULL, 0); >>>>>> + break; >>>>>> + case 'v': >>>>>> + version = strtoul(optarg, NULL, 0); >>>>>> + break; >>>>>> + case 'h': >>>>>> + print_usage(); >>>>>> + return 0; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + /* need a output file */ >>>>>> + if (argc != optind + 1) { >>>>>> + print_usage(); >>>>>> + return -1; >>>>>> + } >>>>>> + >>>>>> + /* need a fit image file or raw image file */ >>>>>> + if (!file) { >>>>>> + print_usage(); >>>>>> + return -1; >>>>>> + } >>>>>> + >>>>>> + if (create_fwbin(argv[optind], file, guid, version, index, >>>> instance) >>>>>> + < 0) { >>>>>> + printf("Creating firmware capsule failed\n"); >>>>>> + return -1; >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> >>>>> >>> >
Heinrich, On Sun, Nov 29, 2020 at 05:59:41AM +0100, Heinrich Schuchardt wrote: > On 11/27/20 3:22 PM, Heinrich Schuchardt wrote: > > Am 25. November 2020 08:32:08 MEZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>: > > > Heinrich, > > > > > > On Wed, Nov 25, 2020 at 07:42:31AM +0100, Heinrich Schuchardt wrote: > > > > Am 25. November 2020 02:05:06 MEZ schrieb AKASHI Takahiro > > > <takahiro.akashi@linaro.org>: > > > > > Heinrich, > > > > > > > > > > On Tue, Nov 24, 2020 at 09:23:50PM +0100, Heinrich Schuchardt wrote: > > > > > > On 11/17/20 1:28 AM, AKASHI Takahiro wrote: > > > > > > > This is a utility mainly for test purpose. > > > > > > > mkeficapsule -f: create a test capsule file for FIT image > > > > > firmware > > > > > > > > > > > > > > Having said that, you will be able to customize the code to fit > > > > > > > your specific requirements for your platform. > > > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > > > > --- > > > > > > > tools/Makefile | 2 + > > > > > > > tools/mkeficapsule.c | 238 > > > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > > > > 2 files changed, 240 insertions(+) > > > > > > > create mode 100644 tools/mkeficapsule.c > > > > > > > > > > > > > > diff --git a/tools/Makefile b/tools/Makefile > > > > > > > index 51123fd92983..66d9376803e3 100644 > > > > > > > --- a/tools/Makefile > > > > > > > +++ b/tools/Makefile > > > > > > > @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs > > > > > > > hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler > > > > > > > HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include > > > > > > > > > > > > > > +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule > > > > > > > + > > > > > > > # We build some files with extra pedantic flags to try to > > > > > minimize things > > > > > > > # that won't build on some weird host compiler -- though there > > > > > are lots of > > > > > > > # exceptions for files that aren't complaint. > > > > > > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > > > > > > > new file mode 100644 > > > > > > > index 000000000000..db95426457cc > > > > > > > --- /dev/null > > > > > > > +++ b/tools/mkeficapsule.c > > > > > > > @@ -0,0 +1,238 @@ > > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > > +/* > > > > > > > + * Copyright 2018 Linaro Limited > > > > > > > + * Author: AKASHI Takahiro > > > > > > > + */ > > > > > > > + > > > > > > > +#include <getopt.h> > > > > > > > +#include <malloc.h> > > > > > > > +#include <stdbool.h> > > > > > > > +#include <stdio.h> > > > > > > > +#include <stdlib.h> > > > > > > > +#include <string.h> > > > > > > > +#include <linux/types.h> > > > > > > > +#include <sys/stat.h> > > > > > > > +#include <sys/types.h> > > > > > > > + > > > > > > > +typedef __u8 u8; > > > > > > > +typedef __u16 u16; > > > > > > > +typedef __u32 u32; > > > > > > > +typedef __u64 u64; > > > > > > > +typedef __s16 s16; > > > > > > > +typedef __s32 s32; > > > > > > > + > > > > > > > +#define aligned_u64 __aligned_u64 > > > > > > > + > > > > > > > +#ifndef __packed > > > > > > > +#define __packed __attribute__((packed)) > > > > > > > +#endif > > > > > > > + > > > > > > > +#include <efi.h> > > > > > > > +#include <efi_api.h> > > > > > > > + > > > > > > > +static const char *tool_name = "mkeficapsule"; > > > > > > > + > > > > > > > +efi_guid_t efi_guid_fm_capsule = > > > > > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > > > > > +efi_guid_t efi_guid_image_type_uboot_fit = > > > > > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > > > > > > +efi_guid_t efi_guid_image_type_uboot_raw = > > > > > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; > > > > > > > + > > > > > > > +static struct option options[] = { > > > > > > > + {"fit", required_argument, NULL, 'f'}, > > > > > > > + {"raw", required_argument, NULL, 'r'}, > > > > > > > + {"index", required_argument, NULL, 'i'}, > > > > > > > + {"instance", required_argument, NULL, 'I'}, > > > > > > > + {"version", required_argument, NULL, 'v'}, > > > > > > > + {"help", no_argument, NULL, 'h'}, > > > > > > > + {NULL, 0, NULL, 0}, > > > > > > > +}; > > > > > > > + > > > > > > > +static void print_usage(void) > > > > > > > +{ > > > > > > > + printf("Usage: %s [options] <output file>\n" > > > > > > > + "Options:\n" > > > > > > > + "\t--fit <fit image> new FIT image file\n" > > > > > > > + "\t--raw <raw image> new raw image file\n" > > > > > > > + "\t--index <index> update image index\n" > > > > > > > + "\t--instance <instance> update hardware instance\n" > > > > > > > + "\t--version <version> firmware version\n" > > > > > > > + "\t--help print a help message\n", > > > > > > > + tool_name); > > > > > > > +} > > > > > > > + > > > > > > > +static int create_fwbin(char *path, char *bin, efi_guid_t > > > *guid, > > > > > > > + unsigned long version, unsigned long index, > > > > > > > + unsigned long instance) > > > > > > > +{ > > > > > > > + struct efi_capsule_header header; > > > > > > > + struct efi_firmware_management_capsule_header capsule; > > > > > > > + struct efi_firmware_management_capsule_image_header image; > > > > > > > + FILE *f, *g; > > > > > > > + struct stat bin_stat; > > > > > > > + u8 *data; > > > > > > > + size_t size; > > > > > > > + > > > > > > > +#ifdef DEBUG > > > > > > > + printf("For output: %s\n", path); > > > > > > > + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); > > > > > > > + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", > > > > > > > + version, index, instance); > > > > > > > +#endif > > > > > > > + > > > > > > > + g = fopen(bin, "r"); > > > > > > > + if (!g) { > > > > > > > + printf("cannot open %s\n", bin); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + if (stat(bin, &bin_stat) < 0) { > > > > > > > + printf("cannot determine the size of %s\n", bin); > > > > > > > + goto err_1; > > > > > > > + } > > > > > > > + data = malloc(bin_stat.st_size); > > > > > > > + if (!data) { > > > > > > > + printf("cannot allocate memory: %lx\n", bin_stat.st_size); > > > > > > > + goto err_1; > > > > > > > + } > > > > > > > + f = fopen(path, "w"); > > > > > > > + if (!f) { > > > > > > > + printf("cannot open %s\n", path); > > > > > > > + goto err_2; > > > > > > > + } > > > > > > > + header.capsule_guid = efi_guid_fm_capsule; > > > > > > > + header.header_size = sizeof(header); > > > > > > > + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ > > > > > > > + header.capsule_image_size = sizeof(header) > > > > > > > + + sizeof(capsule) + sizeof(u64) > > > > > > > + + sizeof(image) > > > > > > > + + bin_stat.st_size; > > > > > > > + > > > > > > > + size = fwrite(&header, 1, sizeof(header), f); > > > > > > > + if (size < sizeof(header)) { > > > > > > > + printf("write failed (%lx)\n", size); > > > > > > > + goto err_3; > > > > > > > + } > > > > > > > + > > > > > > > + capsule.version = 0x00000001; > > > > > > > + capsule.embedded_driver_count = 0; > > > > > > > + capsule.payload_item_count = 1; > > > > > > > + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > > > > > > > > > > > With the complete series applied, building sandbox_defconfig: > > > > > > > > > > > > tools/mkeficapsule.c: In function ‘main’: > > > > > > tools/mkeficapsule.c:120:26: warning: array subscript 0 is outside > > > > > array > > > > > > bounds of ‘u64[]’ {aka ‘long long unsigned int[]’} > > > [-Warray-bounds] > > > > > > 120 | capsule.item_offset_list[0] = sizeof(capsule) + > > > sizeof(u64); > > > > > > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > > > > > > > > > > > Please, ensure that the code compiles without warnings. > > > > > > > > > > Fixing this warning would be easy, but I didn't see it > > > > > with gcc9.2 (from Linaro/Arm) or even in the results of Travis CI. > > > > > > > > > > So I wonder if it is mandatory that the code compiles without > > > warnings, > > > > > and if so, which compiler and which version are required? > > > > > > First, can you please make a comment here against my question? > > > > > > > > > > > > > -Takahiro Akashi > > > > > > > > > > > > > Our settings for gitlab CI and Travis CI are that all warnings are > > > treated as errors. > > > > > > As I said, I've never seen this warning/error in Travis CI. > > > I don't know how we can confirm the result of gitlab CI. > > > > > > -Takahiro Akashi > > > > > > Just make sure that GCC 10+ does not complain locally. > > > > Tom will update the CI in January. I dont want to see a build error then. > > > > Best regards > > > > Heinrich > > Dear Takahiro, > > reading through your code it is obvious that this in not only a stray > warning by GCC but a veritable bug in your patch. Yes, I know. That is why I said: (https://lists.denx.de/pipermail/u-boot/2020-November/433453.html) > Oops, I found that this is not a warning, but a potential bug. > The code does overrun a variable, capsule, allocated on the stack while it is > apparently harmless as the neighboring variable, image, is initialized later. -Takahiro Akashi > You define capsule as: > > 69 struct efi_firmware_management_capsule_header capsule; > > capsule.item_offset_list[] has zero bytes. > > Here you write outside of your structure: > > 120 capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > To solve this you could make capsule a pointer. > > struct efi_firmware_management_capsule_header *capsule; > > capsule = malloc(sizeof(struct efi_firmware_management_capsule_header) + > sizeof(u64)); > > if (!capusule) > goto err; > > capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); > > Best regards > > Heinrich > > > > > > > > > > > > > So we must build without warning. > > > > > > > > Best regards > > > > > > > > Heinrich > > > > > > > > > > > > > > > I have been using GCC 10.2 as supplied by Debian Bullseye on an > > > ARM64 > > > > > > system. > > > > > > > > > > > > Best regards > > > > > > > > > > > > Heinrich > > > > > > > > > > > > > + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); > > > > > > > + if (size < (sizeof(capsule) + sizeof(u64))) { > > > > > > > + printf("write failed (%lx)\n", size); > > > > > > > + goto err_3; > > > > > > > + } > > > > > > > + > > > > > > > + image.version = version; > > > > > > > + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); > > > > > > > + image.update_image_index = index; > > > > > > > + image.update_image_size = bin_stat.st_size; > > > > > > > + image.update_vendor_code_size = 0; /* none */ > > > > > > > + image.update_hardware_instance = instance; > > > > > > > + image.image_capsule_support = 0; > > > > > > > + > > > > > > > + size = fwrite(&image, 1, sizeof(image), f); > > > > > > > + if (size < sizeof(image)) { > > > > > > > + printf("write failed (%lx)\n", size); > > > > > > > + goto err_3; > > > > > > > + } > > > > > > > + size = fread(data, 1, bin_stat.st_size, g); > > > > > > > + if (size < bin_stat.st_size) { > > > > > > > + printf("read failed (%lx)\n", size); > > > > > > > + goto err_3; > > > > > > > + } > > > > > > > + size = fwrite(data, 1, bin_stat.st_size, f); > > > > > > > + if (size < bin_stat.st_size) { > > > > > > > + printf("write failed (%lx)\n", size); > > > > > > > + goto err_3; > > > > > > > + } > > > > > > > + > > > > > > > + fclose(f); > > > > > > > + fclose(g); > > > > > > > + free(data); > > > > > > > + > > > > > > > + return 0; > > > > > > > + > > > > > > > +err_3: > > > > > > > + fclose(f); > > > > > > > +err_2: > > > > > > > + free(data); > > > > > > > +err_1: > > > > > > > + fclose(g); > > > > > > > + > > > > > > > + return -1; > > > > > > > +} > > > > > > > + > > > > > > > +/* > > > > > > > + * Usage: > > > > > > > + * $ mkeficapsule -f <firmware binary> <output file> > > > > > > > + */ > > > > > > > +int main(int argc, char **argv) > > > > > > > +{ > > > > > > > + char *file; > > > > > > > + efi_guid_t *guid; > > > > > > > + unsigned long index, instance, version; > > > > > > > + int c, idx; > > > > > > > + > > > > > > > + file = NULL; > > > > > > > + guid = NULL; > > > > > > > + index = 0; > > > > > > > + instance = 0; > > > > > > > + version = 0; > > > > > > > + for (;;) { > > > > > > > + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); > > > > > > > + if (c == -1) > > > > > > > + break; > > > > > > > + > > > > > > > + switch (c) { > > > > > > > + case 'f': > > > > > > > + if (file) { > > > > > > > + printf("Image already specified\n"); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + file = optarg; > > > > > > > + guid = &efi_guid_image_type_uboot_fit; > > > > > > > + break; > > > > > > > + case 'r': > > > > > > > + if (file) { > > > > > > > + printf("Image already specified\n"); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + file = optarg; > > > > > > > + guid = &efi_guid_image_type_uboot_raw; > > > > > > > + break; > > > > > > > + case 'i': > > > > > > > + index = strtoul(optarg, NULL, 0); > > > > > > > + break; > > > > > > > + case 'I': > > > > > > > + instance = strtoul(optarg, NULL, 0); > > > > > > > + break; > > > > > > > + case 'v': > > > > > > > + version = strtoul(optarg, NULL, 0); > > > > > > > + break; > > > > > > > + case 'h': > > > > > > > + print_usage(); > > > > > > > + return 0; > > > > > > > + } > > > > > > > + } > > > > > > > + > > > > > > > + /* need a output file */ > > > > > > > + if (argc != optind + 1) { > > > > > > > + print_usage(); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + /* need a fit image file or raw image file */ > > > > > > > + if (!file) { > > > > > > > + print_usage(); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + if (create_fwbin(argv[optind], file, guid, version, index, > > > > > instance) > > > > > > > + < 0) { > > > > > > > + printf("Creating firmware capsule failed\n"); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + return 0; > > > > > > > +} > > > > > > > > > > > > > > > > > > > >
diff --git a/tools/Makefile b/tools/Makefile index 51123fd92983..66d9376803e3 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -218,6 +218,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include +hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule + # We build some files with extra pedantic flags to try to minimize things # that won't build on some weird host compiler -- though there are lots of # exceptions for files that aren't complaint. diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c new file mode 100644 index 000000000000..db95426457cc --- /dev/null +++ b/tools/mkeficapsule.c @@ -0,0 +1,238 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2018 Linaro Limited + * Author: AKASHI Takahiro + */ + +#include <getopt.h> +#include <malloc.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <linux/types.h> +#include <sys/stat.h> +#include <sys/types.h> + +typedef __u8 u8; +typedef __u16 u16; +typedef __u32 u32; +typedef __u64 u64; +typedef __s16 s16; +typedef __s32 s32; + +#define aligned_u64 __aligned_u64 + +#ifndef __packed +#define __packed __attribute__((packed)) +#endif + +#include <efi.h> +#include <efi_api.h> + +static const char *tool_name = "mkeficapsule"; + +efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; +efi_guid_t efi_guid_image_type_uboot_fit = + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; +efi_guid_t efi_guid_image_type_uboot_raw = + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID; + +static struct option options[] = { + {"fit", required_argument, NULL, 'f'}, + {"raw", required_argument, NULL, 'r'}, + {"index", required_argument, NULL, 'i'}, + {"instance", required_argument, NULL, 'I'}, + {"version", required_argument, NULL, 'v'}, + {"help", no_argument, NULL, 'h'}, + {NULL, 0, NULL, 0}, +}; + +static void print_usage(void) +{ + printf("Usage: %s [options] <output file>\n" + "Options:\n" + "\t--fit <fit image> new FIT image file\n" + "\t--raw <raw image> new raw image file\n" + "\t--index <index> update image index\n" + "\t--instance <instance> update hardware instance\n" + "\t--version <version> firmware version\n" + "\t--help print a help message\n", + tool_name); +} + +static int create_fwbin(char *path, char *bin, efi_guid_t *guid, + unsigned long version, unsigned long index, + unsigned long instance) +{ + struct efi_capsule_header header; + struct efi_firmware_management_capsule_header capsule; + struct efi_firmware_management_capsule_image_header image; + FILE *f, *g; + struct stat bin_stat; + u8 *data; + size_t size; + +#ifdef DEBUG + printf("For output: %s\n", path); + printf("\tbin: %s\n\ttype: %pUl\n" bin, guid); + printf("\tversion: %ld\n\tindex: %ld\n\tinstance: %ld\n", + version, index, instance); +#endif + + g = fopen(bin, "r"); + if (!g) { + printf("cannot open %s\n", bin); + return -1; + } + if (stat(bin, &bin_stat) < 0) { + printf("cannot determine the size of %s\n", bin); + goto err_1; + } + data = malloc(bin_stat.st_size); + if (!data) { + printf("cannot allocate memory: %lx\n", bin_stat.st_size); + goto err_1; + } + f = fopen(path, "w"); + if (!f) { + printf("cannot open %s\n", path); + goto err_2; + } + header.capsule_guid = efi_guid_fm_capsule; + header.header_size = sizeof(header); + header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET; /* TODO */ + header.capsule_image_size = sizeof(header) + + sizeof(capsule) + sizeof(u64) + + sizeof(image) + + bin_stat.st_size; + + size = fwrite(&header, 1, sizeof(header), f); + if (size < sizeof(header)) { + printf("write failed (%lx)\n", size); + goto err_3; + } + + capsule.version = 0x00000001; + capsule.embedded_driver_count = 0; + capsule.payload_item_count = 1; + capsule.item_offset_list[0] = sizeof(capsule) + sizeof(u64); + size = fwrite(&capsule, 1, sizeof(capsule) + sizeof(u64), f); + if (size < (sizeof(capsule) + sizeof(u64))) { + printf("write failed (%lx)\n", size); + goto err_3; + } + + image.version = version; + memcpy(&image.update_image_type_id, guid, sizeof(*guid)); + image.update_image_index = index; + image.update_image_size = bin_stat.st_size; + image.update_vendor_code_size = 0; /* none */ + image.update_hardware_instance = instance; + image.image_capsule_support = 0; + + size = fwrite(&image, 1, sizeof(image), f); + if (size < sizeof(image)) { + printf("write failed (%lx)\n", size); + goto err_3; + } + size = fread(data, 1, bin_stat.st_size, g); + if (size < bin_stat.st_size) { + printf("read failed (%lx)\n", size); + goto err_3; + } + size = fwrite(data, 1, bin_stat.st_size, f); + if (size < bin_stat.st_size) { + printf("write failed (%lx)\n", size); + goto err_3; + } + + fclose(f); + fclose(g); + free(data); + + return 0; + +err_3: + fclose(f); +err_2: + free(data); +err_1: + fclose(g); + + return -1; +} + +/* + * Usage: + * $ mkeficapsule -f <firmware binary> <output file> + */ +int main(int argc, char **argv) +{ + char *file; + efi_guid_t *guid; + unsigned long index, instance, version; + int c, idx; + + file = NULL; + guid = NULL; + index = 0; + instance = 0; + version = 0; + for (;;) { + c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx); + if (c == -1) + break; + + switch (c) { + case 'f': + if (file) { + printf("Image already specified\n"); + return -1; + } + file = optarg; + guid = &efi_guid_image_type_uboot_fit; + break; + case 'r': + if (file) { + printf("Image already specified\n"); + return -1; + } + file = optarg; + guid = &efi_guid_image_type_uboot_raw; + break; + case 'i': + index = strtoul(optarg, NULL, 0); + break; + case 'I': + instance = strtoul(optarg, NULL, 0); + break; + case 'v': + version = strtoul(optarg, NULL, 0); + break; + case 'h': + print_usage(); + return 0; + } + } + + /* need a output file */ + if (argc != optind + 1) { + print_usage(); + return -1; + } + + /* need a fit image file or raw image file */ + if (!file) { + print_usage(); + return -1; + } + + if (create_fwbin(argv[optind], file, guid, version, index, instance) + < 0) { + printf("Creating firmware capsule failed\n"); + return -1; + } + + return 0; +}
This is a utility mainly for test purpose. mkeficapsule -f: create a test capsule file for FIT image firmware Having said that, you will be able to customize the code to fit your specific requirements for your platform. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- tools/Makefile | 2 + tools/mkeficapsule.c | 238 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 240 insertions(+) create mode 100644 tools/mkeficapsule.c -- 2.28.0