Skip to content

Commit

Permalink
Test (and fix) ImageAddress()
Browse files Browse the repository at this point in the history
This adds a test case for our address sanitation checking function
ImageAddresS().  In doing so it addresses two issues:

- previously we allowed the address after the last byte of the image to
  be computed (may need to revert this or fix some callers, we'll see...)
- bespoke overflow checking and using + directly instead of using
  __builtin_add_overflow()

Signed-off-by: Peter Jones <pjones@redhat.com>
  • Loading branch information
vathpela committed Jun 23, 2023
1 parent 36e2da2 commit 5ececf6
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 3 deletions.
3 changes: 3 additions & 0 deletions include/test.mk
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ test-mok-mirror: CFLAGS+=-DHAVE_START_IMAGE -DHAVE_SHIM_LOCK_GUID
test-sbat_FILES = csv.c lib/variables.c lib/guid.c sbat_var.S mock-variables.c
test-sbat :: CFLAGS+=-DHAVE_GET_VARIABLE -DHAVE_GET_VARIABLE_ATTR -DHAVE_SHIM_LOCK_GUID

test-pe-relocate_FILES = globals.c
test-pe-relocate :: CFLAGS+=-DHAVE_SHIM_LOCK_GUID

test-str_FILES = lib/string.c

tests := $(patsubst %.c,%,$(wildcard test-*.c))
Expand Down
9 changes: 6 additions & 3 deletions pe-relocate.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,19 @@
void *
ImageAddress (void *image, uint64_t size, uint64_t address)
{
uint64_t img_addr;

/* ensure our local pointer isn't bigger than our size */
if (address > size)
if (address >= size)
return NULL;

/* Insure our math won't overflow */
if (UINT64_MAX - address < (uint64_t)(intptr_t)image)
img_addr = (uint64_t)(uintptr_t)image;
if (__builtin_add_overflow(img_addr, address, &img_addr))
return NULL;

/* return the absolute pointer */
return image + address;
return (void *)(uintptr_t)img_addr;
}

/*
Expand Down
1 change: 1 addition & 0 deletions test-data/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!/*.efi
Binary file added test-data/grubx64.0.76.el7.1.efi
Binary file not shown.
Binary file added test-data/grubx64.0.76.el7.efi
Binary file not shown.
Binary file added test-data/grubx64.0.80.el7.efi
Binary file not shown.
39 changes: 39 additions & 0 deletions test-pe-relocate.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// SPDX-License-Identifier: BSD-2-Clause-Patent
/*
* test-pe-reloc.c - attempt to test relocate_coff()
* Copyright Peter Jones <pjones@redhat.com>
*/

#ifndef SHIM_UNIT_TEST
#define SHIM_UNIT_TEST
#endif
#include "shim.h"

static int
test_image_address(void)
{
char image[4];
void *ret;

assert_equal_return(ImageAddress(image, sizeof(image), 0), &image[0], -1, "got %p expected %p\n");
assert_equal_return(ImageAddress(image, sizeof(image), 4), NULL, -1, "got %p expected %p\n");
assert_equal_return(ImageAddress((void *)1, 2, 3), NULL, -1, "got %p expected %p\n");
assert_equal_return(ImageAddress((void *)-1ull, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n");
assert_equal_return(ImageAddress((void *)0, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n");
assert_equal_return(ImageAddress((void *)1, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n");
assert_equal_return(ImageAddress((void *)2, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n");
assert_equal_return(ImageAddress((void *)3, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n");

return 0;
}

int
main(void)
{
int status = 0;
test(test_image_address);

return status;
}

// vim:fenc=utf-8:tw=75:noet

0 comments on commit 5ececf6

Please sign in to comment.