[PATCH 0/2] efi_loader: fix efi_image_region_add()
Handle start and end of the half-open intervals correctly. Provide unit tests.
Heinrich Schuchardt (2): efi_loader: fix efi_image_region_add() test: provide tests for efi_image_region_add()
MAINTAINERS | 1 + lib/efi_loader/efi_signature.c | 35 +++---- test/lib/Makefile | 1 + test/lib/efi_image_region.c | 163 +++++++++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 16 deletions(-) create mode 100644 test/lib/efi_image_region.c
-- 2.27.0
Use start and end address consistently as half-open interval. Simplify the code.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_signature.c | 35 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index 6685253856..f63a18cb8f 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -519,15 +519,19 @@ out: }
/** - * efi_image_region_add - add an entry of region + * efi_image_region_add() - add an entry of region * @regs: Pointer to array of regions - * @start: Start address of region - * @end: End address of region + * @start: Start address of region (included) + * @end: End address of region (excluded) * @nocheck: flag against overlapped regions * - * Take one entry of region [@start, @end] and append it to the list - * pointed to by @regs. If @nocheck is false, overlapping among entries - * will be checked first. + * Take one entry of region [@start, @end[ and insert it into the list. + * + * * If @nocheck is false, the list will be sorted ascending by address. + * Overlapping entries will not be allowed. + * + * * If @nocheck is true, the list will be sorted ascending by sequence + * of adding the entries. Overlapping is allowed. * * Return: status code */ @@ -551,22 +555,21 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs, if (nocheck) continue;
- if (start > reg->data + reg->size) + /* new data after registered region */ + if (start >= reg->data + reg->size) continue;
- if ((start >= reg->data && start < reg->data + reg->size) || - (end > reg->data && end < reg->data + reg->size)) { - debug("%s: new region already part of another\n", - __func__); - return EFI_INVALID_PARAMETER; - } - - if (start < reg->data && end < reg->data + reg->size) { + /* new data preceding registered region */ + if (end <= reg->data) { for (j = regs->num - 1; j >= i; j--) - memcpy(®s->reg[j], ®s->reg[j + 1], + memcpy(®s->reg[j + 1], ®s->reg[j], sizeof(*reg)); break; } + + /* new data overlapping registered region */ + debug("%s: new region already part of another\n", __func__); + return EFI_INVALID_PARAMETER; }
reg = ®s->reg[i]; -- 2.27.0
Provide unit tests for efi_image_region_add().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- MAINTAINERS | 1 + test/lib/Makefile | 1 + test/lib/efi_image_region.c | 163 ++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+) create mode 100644 test/lib/efi_image_region.c
diff --git a/MAINTAINERS b/MAINTAINERS index db8cecd5e0..b515bf3d93 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -633,6 +633,7 @@ F: include/pe.h F: include/asm-generic/pe.h F: lib/charset.c F: lib/efi*/ +F: test/lib/efi_* F: test/py/tests/test_efi* F: test/py/tests/test_efi*/ F: test/unicode_ut.c diff --git a/test/lib/Makefile b/test/lib/Makefile index ce9ae4a9be..6ccc2c41bc 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -3,6 +3,7 @@ # (C) Copyright 2018 # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += cmd_ut_lib.o +obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o obj-y += hexdump.o obj-y += lmb.o obj-y += string.o diff --git a/test/lib/efi_image_region.c b/test/lib/efi_image_region.c new file mode 100644 index 0000000000..cc7bfd7bf6 --- /dev/null +++ b/test/lib/efi_image_region.c @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2020, Heinrich Schuchardt xypron.glpk@gmx.de + */ + +#include <common.h> +#include <efi_loader.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> + +#define UT_REG_CAPACITY 6 + +static int lib_test_efi_image_region_add(struct unit_test_state *uts) +{ + struct efi_image_regions *regs; + + regs = calloc(sizeof(*regs) + + sizeof(struct image_region) * UT_REG_CAPACITY, 1); + ut_assert(regs); + + regs->max = UT_REG_CAPACITY; + + ut_asserteq(0, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x4000, + (void *)0x3000, 1)); + ut_asserteq(0, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x3100, + (void *)0x4000, 1)); + ut_asserteq(1, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x2000, + (void *)0x3100, 1)); + ut_asserteq(2, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x1000, + (void *)0x1f00, 1)); + ut_asserteq(3, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x4000, + (void *)0x4e00, 1)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x1f00, + (void *)0x2001, 1)); + ut_asserteq(5, regs->num); + + ut_asserteq_ptr((void *)0x3100, regs->reg[0].data); + ut_asserteq(0x0f00, regs->reg[0].size); + + ut_asserteq_ptr((void *)0x2000, regs->reg[1].data); + ut_asserteq(0x1100, regs->reg[1].size); + + ut_asserteq_ptr((void *)0x1000, regs->reg[2].data); + ut_asserteq(0x0f00, regs->reg[2].size); + + ut_asserteq_ptr((void *)0x4000, regs->reg[3].data); + ut_asserteq(0x0e00, regs->reg[3].size); + + ut_asserteq_ptr((void *)0x1f00, regs->reg[4].data); + ut_asserteq(0x0101, regs->reg[4].size); + + free(regs); + + return 0; +} + +LIB_TEST(lib_test_efi_image_region_add, 0); + +static int lib_test_efi_image_region_sort(struct unit_test_state *uts) +{ + struct efi_image_regions *regs; + + regs = calloc(sizeof(*regs) + + sizeof(struct image_region) * UT_REG_CAPACITY, 1); + ut_assert(regs); + + regs->max = UT_REG_CAPACITY; + + ut_asserteq(0, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x4000, + (void *)0x3000, 0)); + ut_asserteq(0, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x3100, + (void *)0x4000, 0)); + ut_asserteq(1, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x2000, + (void *)0x3100, 0)); + ut_asserteq(2, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x1000, + (void *)0x1f00, 0)); + ut_asserteq(3, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x4000, + (void *)0x4e00, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x1f00, + (void *)0x2001, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x10ff, + (void *)0x11ff, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x0000, + (void *)0x6000, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x3100, + (void *)0x0e00, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x3200, + (void *)0x0e00, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_INVALID_PARAMETER, + efi_image_region_add(regs, (void *)0x3200, + (void *)0x0d00, 0)); + ut_asserteq(4, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x1f00, + (void *)0x2000, 0)); + ut_asserteq(5, regs->num); + ut_asserteq_64(EFI_SUCCESS, + efi_image_region_add(regs, (void *)0x4000, + (void *)0x4000, 0)); + ut_asserteq(6, regs->num); + ut_asserteq_64(EFI_OUT_OF_RESOURCES, + efi_image_region_add(regs, (void *)0x6000, + (void *)0x0100, 0)); + ut_asserteq(6, regs->num); + + ut_asserteq_ptr((void *)0x1000, regs->reg[0].data); + ut_asserteq(0x0f00, regs->reg[0].size); + + ut_asserteq_ptr((void *)0x1f00, regs->reg[1].data); + ut_asserteq(0x0100, regs->reg[1].size); + + ut_asserteq_ptr((void *)0x2000, regs->reg[2].data); + ut_asserteq(0x1100, regs->reg[2].size); + + ut_asserteq_ptr((void *)0x3100, regs->reg[3].data); + ut_asserteq(0x0f00, regs->reg[3].size); + + ut_asserteq_ptr((void *)0x4000, regs->reg[4].data); + ut_asserteq(0x0000, regs->reg[4].size); + + ut_asserteq_ptr((void *)0x4000, regs->reg[5].data); + ut_asserteq(0x0e00, regs->reg[5].size); + + free(regs); + + return 0; +} + +LIB_TEST(lib_test_efi_image_region_sort, 0); -- 2.27.0
participants (1)
-
Heinrich Schuchardt