From b32ac16f9a321852806fcd473136204af6b49787 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 21 Apr 2020 18:44:39 +0200 Subject: [PATCH 01/10] test/py: fix test_efi_secboot/conftest.py If udisksctl is present test/py/tests/test_efi_secboot/conftest.py fails because the disk image is never mounted. Normal users can only mount fuse file systems. Unfortunately fusefat is still in an experimental state and seems not to work here correctly. So as we have to be root or use the sudo command anyway delete all coding referring to udisksctl. -- We should not use mount point /mnt as this directory or one of its sub-directories might already be in use as active mount points. Instead create a new directory in the build root as mount point. -- Remove debug print statements that have been commented out. print without parentheses is anyway invalid in Python 3. And pytest anyway filters out the output if there is no exception reported. Signed-off-by: Heinrich Schuchardt --- test/py/tests/test_efi_secboot/conftest.py | 30 +++++----------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index e542fef6e8..5d99b8b718 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -43,7 +43,8 @@ def efi_boot_env(request, u_boot_config): HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi' try: - non_root = tool_is_in_path('udisksctl') + mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure' + check_call('mkdir -p {}'.format(mnt_point), shell=True) # create a disk/partition check_call('dd if=/dev/zero of=%s bs=1MiB count=%d' @@ -57,25 +58,11 @@ def efi_boot_env(request, u_boot_config): check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc' % (image_path, image_path, 1), shell=True) check_call('rm %s.tmp' % image_path, shell=True) - if non_root: - out_data = check_output('udisksctl loop-setup -f %s -o %d' - % (image_path, 1048576), shell=True).decode() - m = re.search('(?<= as )(.*)\.', out_data) - loop_dev = m.group(1) - # print 'loop device is: %s' % loop_dev - out_data = check_output('udisksctl info -b %s' - % loop_dev, shell=True).decode() - m = re.search('MountPoints:[ \t]+(.*)', out_data) - mnt_point = m.group(1) - else: - loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"' + loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"' % (part_size, image_path), shell=True).decode() - mnt_point = '/mnt' - check_output('sudo mount -t %s -o umask=000 %s %s' + check_output('sudo mount -t %s -o umask=000 %s %s' % (fs_type, loop_dev, mnt_point), shell=True) - # print 'mount point is: %s' % mnt_point - # suffix # *.key: RSA private key in PEM # *.crt: X509 certificate (self-signed) in PEM @@ -134,13 +121,8 @@ def efi_boot_env(request, u_boot_config): % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH), shell=True) - if non_root: - check_call('udisksctl unmount -b %s' % loop_dev, shell=True) - # not needed - # check_call('udisksctl loop-delete -b %s' % loop_dev, shell=True) - else: - check_call('sudo umount %s' % loop_dev, shell=True) - check_call('sudo losetup -d %s' % loop_dev, shell=True) + check_call('sudo umount %s' % loop_dev, shell=True) + check_call('sudo losetup -d %s' % loop_dev, shell=True) except CalledProcessError as e: pytest.skip('Setup failed: %s' % e.cmd) From 4fe050e65f61cc5e7734aee0637a18ac16b10061 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 20 Apr 2020 12:44:56 +0200 Subject: [PATCH 02/10] efi_loader: remove superfluous NULL check in bootefi.c efi_free_pool() and efi_delete_handle() both check if their argument is NULL. The caller should not duplicate this check. Signed-off-by: Heinrich Schuchardt --- cmd/bootefi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index aaed575505..54b4b8f984 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -481,10 +481,8 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) ret = do_bootefi_exec(handle); out: - if (mem_handle) - efi_delete_handle(mem_handle); - if (file_path) - efi_free_pool(file_path); + efi_delete_handle(mem_handle); + efi_free_pool(file_path); return ret; } From 25801acc1fd64b284d250e8cee7c5ea0c5186f1c Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 19 Mar 2020 13:49:34 +0100 Subject: [PATCH 03/10] part: detect EFI system partition Up to now for MBR and GPT partitions the info field 'bootable' was set to 1 if either the partition was an EFI system partition or the bootable flag was set. Turn info field 'bootable' into a bit mask with separate bits for bootable and EFI system partition. This will allow us to identify the EFI system partition in the UEFI sub-system. Signed-off-by: Heinrich Schuchardt --- cmd/gpt.c | 4 ++-- disk/part_dos.c | 14 ++++++++++---- disk/part_efi.c | 16 ++++++++++------ include/part.h | 11 ++++++++++- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/cmd/gpt.c b/cmd/gpt.c index efaf1bcecb..b94f0051cd 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -245,7 +245,7 @@ static void print_gpt_info(void) printf("Block size %lu, name %s\n", curr->gpt_part_info.blksz, curr->gpt_part_info.name); printf("Type %s, bootable %d\n", curr->gpt_part_info.type, - curr->gpt_part_info.bootable); + curr->gpt_part_info.bootable & PART_BOOTABLE); #ifdef CONFIG_PARTITION_UUIDS printf("UUID %s\n", curr->gpt_part_info.uuid); #endif @@ -535,7 +535,7 @@ static int set_gpt_info(struct blk_desc *dev_desc, /* bootable */ if (found_key(tok, "bootable")) - parts[i].bootable = 1; + parts[i].bootable = PART_BOOTABLE; } *parts_count = p_count; diff --git a/disk/part_dos.c b/disk/part_dos.c index 83ff40d310..813379f851 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -45,9 +45,15 @@ static inline int is_extended(int part_type) part_type == 0x85); } -static inline int is_bootable(dos_partition_t *p) +static int get_bootable(dos_partition_t *p) { - return (p->sys_ind == 0xef) || (p->boot_ind == 0x80); + int ret = 0; + + if (p->sys_ind == 0xef) + ret |= PART_EFI_SYSTEM_PARTITION; + if (p->boot_ind == 0x80) + ret |= PART_BOOTABLE; + return ret; } static void print_one_part(dos_partition_t *p, lbaint_t ext_part_sector, @@ -60,7 +66,7 @@ static void print_one_part(dos_partition_t *p, lbaint_t ext_part_sector, "u\t%08x-%02x\t%02x%s%s\n", part_num, lba_start, lba_size, disksig, part_num, p->sys_ind, (is_extended(p->sys_ind) ? " Extd" : ""), - (is_bootable(p) ? " Boot" : "")); + (get_bootable(p) ? " Boot" : "")); } static int test_block_type(unsigned char *buffer) @@ -258,7 +264,7 @@ static int part_get_info_extended(struct blk_desc *dev_desc, (char *)info->name); /* sprintf(info->type, "%d, pt->sys_ind); */ strcpy((char *)info->type, "U-Boot"); - info->bootable = is_bootable(pt); + info->bootable = get_bootable(pt); #if CONFIG_IS_ENABLED(PARTITION_UUIDS) sprintf(info->uuid, "%08x-%02x", disksig, part_num); #endif diff --git a/disk/part_efi.c b/disk/part_efi.c index b2e157d9c1..83876a7bd9 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -71,11 +71,15 @@ static char *print_efiname(gpt_entry *pte) static const efi_guid_t system_guid = PARTITION_SYSTEM_GUID; -static inline int is_bootable(gpt_entry *p) +static int get_bootable(gpt_entry *p) { - return p->attributes.fields.legacy_bios_bootable || - !memcmp(&(p->partition_type_guid), &system_guid, - sizeof(efi_guid_t)); + int ret = 0; + + if (!memcmp(&p->partition_type_guid, &system_guid, sizeof(efi_guid_t))) + ret |= PART_EFI_SYSTEM_PARTITION; + if (p->attributes.fields.legacy_bios_bootable) + ret |= PART_BOOTABLE; + return ret; } static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba, @@ -286,7 +290,7 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part, snprintf((char *)info->name, sizeof(info->name), "%s", print_efiname(&gpt_pte[part - 1])); strcpy((char *)info->type, "U-Boot"); - info->bootable = is_bootable(&gpt_pte[part - 1]); + info->bootable = get_bootable(&gpt_pte[part - 1]); #if CONFIG_IS_ENABLED(PARTITION_UUIDS) uuid_bin_to_str(gpt_pte[part - 1].unique_partition_guid.b, info->uuid, UUID_STR_FORMAT_GUID); @@ -501,7 +505,7 @@ int gpt_fill_pte(struct blk_desc *dev_desc, memset(&gpt_e[i].attributes, 0, sizeof(gpt_entry_attributes)); - if (partitions[i].bootable) + if (partitions[i].bootable & PART_BOOTABLE) gpt_e[i].attributes.fields.legacy_bios_bootable = 1; /* partition name */ diff --git a/include/part.h b/include/part.h index 0b5cf3d5e8..3693527397 100644 --- a/include/part.h +++ b/include/part.h @@ -51,13 +51,22 @@ struct block_drvr { #define PART_TYPE_LEN 32 #define MAX_SEARCH_PARTITIONS 64 +#define PART_BOOTABLE ((int)BIT(0)) +#define PART_EFI_SYSTEM_PARTITION ((int)BIT(1)) + typedef struct disk_partition { lbaint_t start; /* # of first block in partition */ lbaint_t size; /* number of blocks in partition */ ulong blksz; /* block size in bytes */ uchar name[PART_NAME_LEN]; /* partition name */ uchar type[PART_TYPE_LEN]; /* string type description */ - int bootable; /* Active/Bootable flag is set */ + /* + * The bootable is a bitmask with the following fields: + * + * PART_BOOTABLE the MBR bootable flag is set + * PART_EFI_SYSTEM_PARTITION the partition is an EFI system partition + */ + int bootable; #if CONFIG_IS_ENABLED(PARTITION_UUIDS) char uuid[UUID_STR_LEN + 1]; /* filesystem UUID as string, if exists */ #endif From 11078bb262a2a2489cbc5962f2e7faad5b288194 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 19 Mar 2020 15:16:31 +0100 Subject: [PATCH 04/10] efi_loader: identify EFI system partition In subsequent patches UEFI variables shalled be stored on the EFI system partition. Hence we need to identify the EFI system partition. Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 7 +++++++ lib/efi_loader/efi_disk.c | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/include/efi_loader.h b/include/efi_loader.h index 0ba9a1f702..b7bccf50b3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -47,6 +47,13 @@ static inline void *guidcpy(void *dst, const void *src) /* Root node */ extern efi_handle_t efi_root; +/* EFI system partition */ +extern struct efi_system_partition { + enum if_type if_type; + int devnum; + u8 part; +} efi_system_partition; + int __efi_entry_check(void); int __efi_exit_check(void); const char *__efi_nesting(void); diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index fd8fe17567..fd3df80b0b 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -13,6 +13,8 @@ #include #include +struct efi_system_partition efi_system_partition; + const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID; /** @@ -418,6 +420,24 @@ static efi_status_t efi_disk_add_dev( diskobj->ops.media = &diskobj->media; if (disk) *disk = diskobj; + + /* Store first EFI system partition */ + if (part && !efi_system_partition.if_type) { + int r; + disk_partition_t info; + + r = part_get_info(desc, part, &info); + if (r) + return EFI_DEVICE_ERROR; + if (info.bootable & PART_EFI_SYSTEM_PARTITION) { + efi_system_partition.if_type = desc->if_type; + efi_system_partition.devnum = desc->devnum; + efi_system_partition.part = part; + EFI_PRINT("EFI system partition: %s %d:%d\n", + blk_get_if_type_name(desc->if_type), + desc->devnum, part); + } + } return EFI_SUCCESS; } From 788bd90bf828ea65acacb8a7a50e2e42efe9700a Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 16 Apr 2020 20:31:56 +0200 Subject: [PATCH 05/10] doc/efi: rework secure boot description Ensure a uniform formatting. Some rephrasing. Signed-off-by: Heinrich Schuchardt --- doc/uefi/uefi.rst | 110 ++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 48 deletions(-) diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst index a35fbd331c..4fda00d687 100644 --- a/doc/uefi/uefi.rst +++ b/doc/uefi/uefi.rst @@ -100,79 +100,93 @@ See doc/uImage.FIT/howto.txt for an introduction to FIT images. Configuring UEFI secure boot ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -UEFI specification[1] defines a secure way of executing UEFI images +The UEFI specification[1] defines a secure way of executing UEFI images by verifying a signature (or message digest) of image with certificates. This feature on U-Boot is enabled with:: CONFIG_UEFI_SECURE_BOOT=y To make the boot sequence safe, you need to establish a chain of trust; -In UEFI secure boot, you can make it with the UEFI variables, "PK" -(Platform Key), "KEK" (Key Exchange Keys), "db" (white list database) -and "dbx" (black list database). +In UEFI secure boot the chain trust is defined by the following UEFI variables -There are many online documents that describe what UEFI secure boot is -and how it works. Please consult some of them for details. +* PK - Platform Key +* KEK - Key Exchange Keys +* db - white list database +* dbx - black list database -Here is a simple example that you can follow for your initial attempt -(Please note that the actual steps would absolutely depend on your system -and environment.): +An in depth description of UEFI secure boot is beyond the scope of this +document. Please, refer to the UEFI specification and available online +documentation. Here is a simple example that you can follow for your initial +attempt (Please note that the actual steps will depend on your system and +environment.): -1. Install utility commands on your host - * openssl - * efitools - * sbsigntool +Install the required tools on your host -2. Create signing keys and key database files on your host - for PK:: +* openssl +* efitools +* sbsigntool - $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ \ - -keyout PK.key -out PK.crt -nodes -days 365 - $ cert-to-efi-sig-list -g 11111111-2222-3333-4444-123456789abc \ - PK.crt PK.esl; - $ sign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth +Create signing keys and the key database on your host: - for KEK:: +The platform key - $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ \ - -keyout KEK.key -out KEK.crt -nodes -days 365 - $ cert-to-efi-sig-list -g 11111111-2222-3333-4444-123456789abc \ - KEK.crt KEK.esl - $ sign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth +.. code-block:: bash - for db:: + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ \ + -keyout PK.key -out PK.crt -nodes -days 365 + cert-to-efi-sig-list -g 11111111-2222-3333-4444-123456789abc \ + PK.crt PK.esl; + sign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth - $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ \ - -keyout db.key -out db.crt -nodes -days 365 - $ cert-to-efi-sig-list -g 11111111-2222-3333-4444-123456789abc \ - db.crt db.esl - $ sign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth +The key exchange keys - Copy \*.auth to media, say mmc, that is accessible from U-Boot. +.. code-block:: bash -3. Sign an image with one key in "db" on your host:: + openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ \ + -keyout KEK.key -out KEK.crt -nodes -days 365 + cert-to-efi-sig-list -g 11111111-2222-3333-4444-123456789abc \ + KEK.crt KEK.esl + sign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth - $ sbsign --key db.key --cert db.crt helloworld.efi +The whitelist database -4. Install keys on your board:: +.. code-block:: bash - ==> fatload mmc 0:1 PK.auth - ==> setenv -e -nv -bs -rt -at -i ,$filesize PK - ==> fatload mmc 0:1 KEK.auth - ==> setenv -e -nv -bs -rt -at -i ,$filesize KEK - ==> fatload mmc 0:1 db.auth - ==> setenv -e -nv -bs -rt -at -i ,$filesize db + $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_db/ \ + -keyout db.key -out db.crt -nodes -days 365 + $ cert-to-efi-sig-list -g 11111111-2222-3333-4444-123456789abc \ + db.crt db.esl + $ sign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth -5. Set up boot parameters on your board:: +Copy the \*.auth files to media, say mmc, that is accessible from U-Boot. - ==> efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" +Sign an image with one of the keys in "db" on your host -Then your board runs that image from Boot manager (See below). +.. code-block:: bash + + sbsign --key db.key --cert db.crt helloworld.efi + +Now in U-Boot install the keys on your board:: + + fatload mmc 0:1 PK.auth + setenv -e -nv -bs -rt -at -i ,$filesize PK + fatload mmc 0:1 KEK.auth + setenv -e -nv -bs -rt -at -i ,$filesize KEK + fatload mmc 0:1 db.auth + setenv -e -nv -bs -rt -at -i ,$filesize db + +Set up boot parameters on your board:: + + efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed "" + +Now your board can run the signed image via the boot manager (see below). You can also try this sequence by running Pytest, test_efi_secboot, -on sandbox:: +on the sandbox - $ cd - $ pytest.py test/py/tests/test_efi_secboot/test_signed.py --bd sandbox +.. code-block:: bash + + cd + pytest.py test/py/tests/test_efi_secboot/test_signed.py --bd sandbox Executing the boot manager ~~~~~~~~~~~~~~~~~~~~~~~~~~ From d9f3307a826b0b4be818c26dd3f5db49095f846b Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Tue, 21 Apr 2020 09:38:38 +0900 Subject: [PATCH 06/10] efi_loader: remove CONFIG_EFI_SECURE_BOOT in efi_loader.h The guard doesn't make any difference, so remove it. Signed-off-by: AKASHI Takahiro Suggested-by: Heinrich Schuchardt Reviewed-by: Heinrich Schuchardt --- include/efi_loader.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index b7bccf50b3..f92bfe57e6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -11,6 +11,7 @@ #include #include #include +#include #include static inline int guidcmp(const void *g1, const void *g2) @@ -702,9 +703,6 @@ void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); efi_status_t efi_bootmgr_load(efi_handle_t *handle); -#ifdef CONFIG_EFI_SECURE_BOOT -#include - /** * efi_image_regions - A list of memory regions * @@ -774,7 +772,6 @@ bool efi_secure_boot_enabled(void); bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, WIN_CERTIFICATE **auth, size_t *auth_len); -#endif /* CONFIG_EFI_SECURE_BOOT */ #else /* CONFIG_IS_ENABLED(EFI_LOADER) */ From 661c02ceb29d33e2f30cc19dd1a46582a66fd914 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 22 Apr 2020 17:53:34 +0200 Subject: [PATCH 07/10] MAINTAINERS: assign test/py/tests/test_efi*/ to EFI PAYLOAD Some UEFI related files are not assigned currently. Add them to the EFI PAYLOAD area. Signed-off-by: Heinrich Schuchardt --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index dd92af5182..66f0b07263 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -625,6 +625,7 @@ F: include/asm-generic/pe.h F: lib/charset.c F: lib/efi*/ F: test/py/tests/test_efi* +F: test/py/tests/test_efi*/ F: test/unicode_ut.c F: cmd/bootefi.c F: cmd/efidebug.c From b5f4e9e384c36234a0e7f1c60957f369c4c05b5e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 29 Apr 2020 19:20:35 +0200 Subject: [PATCH 08/10] efi_loader: fix 'efidebug boot dump' * Do not recreate a variable name that we already have as u16 string. * Check the return value of malloc() * EFI_NOT_FOUND cannot occur for a variable name returned by GetNextVariableName(). Remove a print statement. * Don't copy a GUID for no reason. * Don't use the run-time service table to call exported functions. * Don't pass NULL to show_efi_boot_opt_data() (fixes Coverity CID 300338). Signed-off-by: Heinrich Schuchardt --- cmd/efidebug.c | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 02ef019694..7ff2ce4ce1 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -682,13 +682,13 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, /** * show_efi_boot_opt_data() - dump UEFI load option * - * @id: load option number + * @varname16: variable name * @data: value of UEFI load option variable * @size: size of the boot option * * Decode the value of UEFI load option variable and print information. */ -static void show_efi_boot_opt_data(int id, void *data, size_t size) +static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t size) { struct efi_load_option lo; char *label, *p; @@ -705,8 +705,8 @@ static void show_efi_boot_opt_data(int id, void *data, size_t size) p = label; utf16_utf8_strncpy(&p, lo.label, label_len16); - printf("Boot%04X:\n", id); - printf(" attributes: %c%c%c (0x%08x)\n", + printf("%ls:\nattributes: %c%c%c (0x%08x)\n", + varname16, /* ACTIVE */ lo.attributes & LOAD_OPTION_ACTIVE ? 'A' : '-', /* FORCE RECONNECT */ @@ -730,37 +730,32 @@ static void show_efi_boot_opt_data(int id, void *data, size_t size) /** * show_efi_boot_opt() - dump UEFI load option * - * @id: Load option number + * @varname16: variable name * * Dump information defined by UEFI load option. */ -static void show_efi_boot_opt(int id) +static void show_efi_boot_opt(u16 *varname16) { - char var_name[9]; - u16 var_name16[9], *p; - efi_guid_t guid; - void *data = NULL; + void *data; efi_uintn_t size; efi_status_t ret; - sprintf(var_name, "Boot%04X", id); - p = var_name16; - utf8_utf16_strncpy(&p, var_name, 9); - guid = efi_global_variable_guid; - size = 0; - ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size, NULL)); + ret = EFI_CALL(efi_get_variable(varname16, &efi_global_variable_guid, + NULL, &size, NULL)); if (ret == EFI_BUFFER_TOO_SMALL) { data = malloc(size); - ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size, - data)); + if (!data) { + printf("ERROR: Out of memory\n"); + return; + } + ret = EFI_CALL(efi_get_variable(varname16, + &efi_global_variable_guid, + NULL, &size, data)); + if (ret == EFI_SUCCESS) + show_efi_boot_opt_data(varname16, data, size); + free(data); } - if (ret == EFI_SUCCESS) - show_efi_boot_opt_data(id, data, size); - else if (ret == EFI_NOT_FOUND) - printf("Boot%04X: not found\n", id); - - free(data); } static int u16_tohex(u16 c) @@ -839,7 +834,7 @@ static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag, id = (id << 4) + digit; } if (i == 4 && !var_name16[8]) - show_efi_boot_opt(id); + show_efi_boot_opt(var_name16); } free(var_name16); From dd9056c06a0557bc08ca986530024568e01ede6f Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 29 Apr 2020 20:21:39 +0200 Subject: [PATCH 09/10] efi_loader: efidebug, avoid illegal memory access For EFI_PERSISTENT_MEMORY_TYPE the 'efidebug memmap' command produces an illegal memory access. * Add the missing descriptive string for EFI_PERSISTENT_MEMORY_TYPE. * Replace the check for EFI_MAX_MEMORY_TYPE by the ARRAY_SIZE() macro. Reported-by: Coverity (CID 300336) Signed-off-by: Heinrich Schuchardt --- cmd/efidebug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 7ff2ce4ce1..53cb3cbfaf 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -395,6 +395,7 @@ static const char * const efi_mem_type_string[] = { [EFI_MMAP_IO] = "IO", [EFI_MMAP_IO_PORT] = "IO PORT", [EFI_PAL_CODE] = "PAL", + [EFI_PERSISTENT_MEMORY_TYPE] = "PERSISTENT", }; static const struct efi_mem_attrs { @@ -482,7 +483,7 @@ static int do_efi_show_memmap(cmd_tbl_t *cmdtp, int flag, printf("================ %.*s %.*s ==========\n", EFI_PHYS_ADDR_WIDTH, sep, EFI_PHYS_ADDR_WIDTH, sep); for (i = 0, map = memmap; i < map_size / sizeof(*map); map++, i++) { - if (map->type < EFI_MAX_MEMORY_TYPE) + if (map->type < ARRAY_SIZE(efi_mem_type_string)) type = efi_mem_type_string[map->type]; else type = "(unknown)"; From f9f5f92bc54b035223e447bc5740544efd0569d9 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 29 Apr 2020 21:15:08 +0200 Subject: [PATCH 10/10] efi_loader: fix 'efidebug bootorder' * don't copy GUIDs for no reason * shorten print format strings by using variable names * don't use the run-time table to access exported functions * check the result of malloc() (fixes Coverity CID 300331) Signed-off-by: Heinrich Schuchardt --- cmd/efidebug.c | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 53cb3cbfaf..d4030fee64 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -852,8 +852,7 @@ static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag, */ static int show_efi_boot_order(void) { - efi_guid_t guid; - u16 *bootorder = NULL; + u16 *bootorder; efi_uintn_t size; int num, i; char var_name[9]; @@ -864,20 +863,25 @@ static int show_efi_boot_order(void) size_t label_len16, label_len; efi_status_t ret; - guid = efi_global_variable_guid; size = 0; - ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL, &size, - NULL)); - if (ret == EFI_BUFFER_TOO_SMALL) { - bootorder = malloc(size); - ret = EFI_CALL(RT->get_variable(L"BootOrder", &guid, NULL, - &size, bootorder)); + ret = EFI_CALL(RT->get_variable(L"BootOrder", &efi_global_variable_guid, + NULL, &size, NULL)); + if (ret != EFI_BUFFER_TOO_SMALL) { + if (ret == EFI_NOT_FOUND) { + printf("BootOrder not defined\n"); + return CMD_RET_SUCCESS; + } else { + return CMD_RET_FAILURE; + } } - if (ret == EFI_NOT_FOUND) { - printf("BootOrder not defined\n"); - ret = CMD_RET_SUCCESS; - goto out; - } else if (ret != EFI_SUCCESS) { + bootorder = malloc(size); + if (!bootorder) { + printf("ERROR: Out of memory\n"); + return CMD_RET_FAILURE; + } + ret = EFI_CALL(efi_get_variable(L"BootOrder", &efi_global_variable_guid, + NULL, &size, bootorder)); + if (ret != EFI_SUCCESS) { ret = CMD_RET_FAILURE; goto out; } @@ -889,11 +893,11 @@ static int show_efi_boot_order(void) utf8_utf16_strncpy(&p16, var_name, 9); size = 0; - ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size, - NULL)); + ret = EFI_CALL(efi_get_variable(var_name16, + &efi_global_variable_guid, NULL, + &size, NULL)); if (ret != EFI_BUFFER_TOO_SMALL) { - printf("%2d: Boot%04X: (not defined)\n", - i + 1, bootorder[i]); + printf("%2d: %s: (not defined)\n", i + 1, var_name); continue; } @@ -902,8 +906,9 @@ static int show_efi_boot_order(void) ret = CMD_RET_FAILURE; goto out; } - ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size, - data)); + ret = EFI_CALL(efi_get_variable(var_name16, + &efi_global_variable_guid, NULL, + &size, data)); if (ret != EFI_SUCCESS) { free(data); ret = CMD_RET_FAILURE; @@ -922,7 +927,7 @@ static int show_efi_boot_order(void) } p = label; utf16_utf8_strncpy(&p, lo.label, label_len16); - printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label); + printf("%2d: %s: %s\n", i + 1, var_name, label); free(label); free(data);