From 657b2483ca6e9fcf2ad8ac7ee577ff546d24c3aa Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Mon, 5 Dec 2022 17:57:36 -0500
Subject: [PATCH] Make sbat_var.S parse right with buggy gcc/binutils

In https://github.com/rhboot/shim/issues/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>
---
 include/test.mk |  2 +-
 sbat_var.S      |  6 ++++--
 test-sbat.c     | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/include/test.mk b/include/test.mk
index c0e240951..c37b84466 100644
--- a/include/test.mk
+++ b/include/test.mk
@@ -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
diff --git a/sbat_var.S b/sbat_var.S
index a115077ae..2a813a403 100644
--- a/sbat_var.S
+++ b/sbat_var.S
@@ -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
diff --git a/test-sbat.c b/test-sbat.c
index 72bebe7ae..65bc6a84b 100644
--- a/test-sbat.c
+++ b/test-sbat.c
@@ -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)
 {
@@ -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;
 }