Skip to content

Commit

Permalink
Make sbat_var.S parse right with buggy gcc/binutils
Browse files Browse the repository at this point in the history
In #533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

Signed-off-by: Peter Jones <pjones@redhat.com>
  • Loading branch information
vathpela committed Dec 7, 2022
1 parent 1149161 commit 657b248
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
2 changes: 1 addition & 1 deletion include/test.mk
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ test-mock-variables: CFLAGS+=-DHAVE_SHIM_LOCK_GUID
test-mok-mirror_FILES = mok.c globals.c tpm.c lib/guid.c lib/variables.c mock-variables.c
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
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-str_FILES = lib/string.c
Expand Down
6 changes: 4 additions & 2 deletions sbat_var.S
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ sbat_var_payload_header:
.Lsbat_var_payload_header_end:
.balign 1, 0
.Lsbat_var_previous:
.asciz SBAT_VAR_PREVIOUS
.ascii SBAT_VAR_PREVIOUS
.byte 0
.balign 1, 0
.Lsbat_var_latest:
.asciz SBAT_VAR_LATEST
.ascii SBAT_VAR_LATEST
.byte 0
32 changes: 32 additions & 0 deletions test-sbat.c
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,36 @@ test_preserve_sbat_uefi_variable_bad_short(void)
return 0;
}

static int
test_sbat_var_asciz(void)
{
EFI_STATUS status;
char buf[1024] = "";
UINT32 attrs = 0;
UINTN size = sizeof(buf);
char expected[] = SBAT_VAR_PREVIOUS;

status = set_sbat_uefi_variable();
if (status != EFI_SUCCESS)
return -1;

status = RT->GetVariable(SBAT_VAR_NAME, &SHIM_LOCK_GUID, &attrs, &size, buf);
if (status != EFI_SUCCESS)
return -1;

/*
* this should be enough to get past "sbat,", which handles the
* first error.
*/
if (size < (strlen(SBAT_VAR_SIG) + 2) || size != strlen(expected))
return -1;

if (strncmp(expected, buf, size) != 0)
return -1;

return 0;
}

int
main(void)
{
Expand Down Expand Up @@ -1155,6 +1185,8 @@ main(void)
test(test_preserve_sbat_uefi_variable_version_older);
test(test_preserve_sbat_uefi_variable_version_olderlonger);

test(test_sbat_var_asciz);

return 0;
}

Expand Down

0 comments on commit 657b248

Please sign in to comment.