[PATCH v9 08/11] tools: add mkeficapsule command for UEFI capsule update
Heinrich Schuchardt
xypron.glpk at gmx.de
Sun Nov 29 05:59:41 CET 2020
On 11/27/20 3:22 PM, Heinrich Schuchardt wrote:
> Am 25. November 2020 08:32:08 MEZ schrieb AKASHI Takahiro <takahiro.akashi at 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 at 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 at 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;
>>>>>> +}
>>>>>>
>>>>>
>>>
>
More information about the U-Boot
mailing list