[PATCH 2/2] acpi: dbg2: fix segfault and refactor dbg2_obj_find

Erico Nunes ernunes at redhat.com
Mon Oct 31 14:49:45 UTC 2016


The dbg2 test may crash on some systems due to dbg2_obj_find not being
able to handle malformed or nonstandard NamespaceString attributes.
It has been seen to crash due to NamespaceString being described with a
leading escape backslash, such as '\\_SB_.AHBC.URT1'. In these cases,
regardless of whether the test should pass or not, it should not crash
fwts.
In order to avoid more problems with exact length calculation, the
'expanded' string size is just moved to a reasonably large size which
should be able to fit any NamespaceString.

It has also been observed that the stao table test implements a very
similar routine and therefore may be vulnerable to the same sort of
problem, so that code has been refactored into fwts_acpi_obj_find which
can be used by both stao and dbg2 tests.

The printed strings in case of error logs are now printed with the
unexpanded string, so the expected outputs used in 'make check' have
also been updated to reflect that.

Signed-off-by: Erico Nunes <ernunes at redhat.com>
---
 fwts-test/dbg2-0001/dbg2-0002.log  |  2 +-
 fwts-test/stao-0001/stao-0002.log  |  6 +--
 src/acpi/dbg2/dbg2.c               | 87 +++++---------------------------------
 src/acpi/stao/stao.c               | 68 +++--------------------------
 src/lib/include/fwts_acpi_tables.h |  1 +
 src/lib/src/fwts_acpi_tables.c     | 60 ++++++++++++++++++++++++++
 6 files changed, 80 insertions(+), 144 deletions(-)

diff --git a/fwts-test/dbg2-0001/dbg2-0002.log b/fwts-test/dbg2-0001/dbg2-0002.log
index a55efd3..68be687 100644
--- a/fwts-test/dbg2-0001/dbg2-0002.log
+++ b/fwts-test/dbg2-0001/dbg2-0002.log
@@ -25,7 +25,7 @@ dbg2            FAILED [HIGH] DBG2PortSubTypeReserved: Test 1, DBG2 Info
 dbg2            Structure Port Subtype is 0x0008 which is a reserved type.
 dbg2              Namespace String:         '\_SB.PCI0.EHC1.HUB0.PRT2'
 dbg2            FAILED [HIGH] DBG2DeviceNotFound: Test 1, DBG2 Device
-dbg2            '\_SB_.PCI0.EHC1.HUB0.PRT2' not found in ACPI object name
+dbg2            '\_SB.PCI0.EHC1.HUB0.PRT2' not found in ACPI object name
 dbg2            space.
 dbg2            FAILED [HIGH] DBG2TooShort: Test 1, DBG2 table too short,
 dbg2            expecting 65323 bytes, instead got 107 bytes for a DBG2
diff --git a/fwts-test/stao-0001/stao-0002.log b/fwts-test/stao-0001/stao-0002.log
index 6756845..78b2eb9 100644
--- a/fwts-test/stao-0001/stao-0002.log
+++ b/fwts-test/stao-0001/stao-0002.log
@@ -7,11 +7,11 @@ stao              ACPI String:              '\_SB.C0E0'
 stao              ACPI String:              '\_SB.C022'
 stao              ACPI String:              '\_SB.C02E'
 stao            FAILED [HIGH] STAOAcpiStringNotFound: Test 1, STAO ACPI
-stao            String '\_SB_.C0E0' not found in ACPI object name space.
+stao            String '\_SB.C0E0' not found in ACPI object name space.
 stao            FAILED [HIGH] STAOAcpiStringNotFound: Test 1, STAO ACPI
-stao            String '\_SB_.C022' not found in ACPI object name space.
+stao            String '\_SB.C022' not found in ACPI object name space.
 stao            FAILED [HIGH] STAOAcpiStringNotFound: Test 1, STAO ACPI
-stao            String '\_SB_.C02E' not found in ACPI object name space.
+stao            String '\_SB.C02E' not found in ACPI object name space.
 stao            
 stao            ==========================================================
 stao            0 passed, 3 failed, 0 warning, 0 aborted, 0 skipped, 0
diff --git a/src/acpi/dbg2/dbg2.c b/src/acpi/dbg2/dbg2.c
index a542f1b..38ca65e 100644
--- a/src/acpi/dbg2/dbg2.c
+++ b/src/acpi/dbg2/dbg2.c
@@ -89,80 +89,6 @@ static void dbg2_check_namespace_string(
 	*passed = false;
 }
 
-/*
- *  dbg2_obj_find()
- *	find a given object from the given path name
- */
-static void dbg2_obj_find(
-	fwts_framework *fw,
-	char *obj_name,
-	bool *passed)
-{
-	fwts_list_link  *item;
-	fwts_list *objects;
-	int i = -1;
-	char *ptr1;
-
-        if (fwts_acpi_init(fw) != FWTS_OK) {
-                fwts_log_error(fw, "Cannot initialise ACPI.");
-		return;
-        }
-	if ((objects = fwts_acpi_object_get_names()) == NULL) {
-		fwts_log_info(fw, "Cannot find any ACPI objects");
-		fwts_acpi_deinit(fw);
-		return;
-	}
-
-	/* Find number of '.' in object path */
-	for (i = 0, ptr1 = obj_name; *ptr1; ptr1++) {
-		if (*ptr1 == '.')
-			i++;
-	}
-
-	/*
-	 *  Allocate buffer big enough to take expanded path
-	 *  and pad out any fields that are not 4 chars in size
-	 *  between each . separator
-	 *
-	 *  Converts \_SB.A.BB.CCC.DDDD.EE to
-	 *	     \_SB_.A___.BB__.CCC_.DDDD.EE__
-	 */
-	{
-		char expanded[1 + (5 * (i + 1))];
-		char *ptr2 = expanded;
-
-		for (i = -1, ptr1 = obj_name; ; ptr1++) {
-			if (*ptr1 == '.' || *ptr1 == '\0') {
-				while (i < 4) {
-					*ptr2++ = '_';
-					i++;
-				}
-				i = 0;
-			} else {
-				i++;
-			}
-			*ptr2++ = *ptr1;
-			if (!*ptr1)
-				break;
-		}
-
-		/* Search for object */
-		fwts_list_foreach(item, objects) {
-			char *name = fwts_list_data(char*, item);
-			if (!strcmp(expanded, name))
-				goto done;
-		}
-		/* Not found */
-		*passed = false;
-		fwts_failed(fw, LOG_LEVEL_HIGH,
-			"DBG2DeviceNotFound",
-			"DBG2 Device '%s' not found in ACPI object name space.",
-			expanded);
-	}
-done:
-	fwts_acpi_deinit(fw);
-}
-
 
 /*
  *  DBG2 Table
@@ -333,10 +259,17 @@ static int dbg2_test1(fwts_framework *fw)
 			char *str = (char *)table->data + offset + info->namespace_offset;
 			dbg2_check_namespace_string(fw, str, info->namespace_length, &passed);
 			fwts_log_info_verbatim(fw, "  Namespace String:         '%s'", str);
-			if (strcmp(str, "."))
-				dbg2_obj_find(fw, str, &ok);
+			if (strcmp(str, ".") != 0) {
+				bool found = fwts_acpi_obj_find(fw, str);
+				if (!found) {
+					passed = false;
+					fwts_failed(fw, LOG_LEVEL_HIGH,
+							"DBG2DeviceNotFound",
+							"DBG2 Device '%s' not found in ACPI object name space.",
+							str);
+				}
+			}
 		}
-		passed &= ok;
 
 		dbg2_check_offset(fw, table->length, offset + info->oem_data_offset,
 			"DBG2 Info Structure OEM Data Offset", &passed);
diff --git a/src/acpi/stao/stao.c b/src/acpi/stao/stao.c
index 1461ba6..c529213 100644
--- a/src/acpi/stao/stao.c
+++ b/src/acpi/stao/stao.c
@@ -85,11 +85,7 @@ static int stao_test1(fwts_framework *fw)
 	const fwts_acpi_table_stao *stao = (const fwts_acpi_table_stao *)table->data;
 	bool passed = true;
 	char *ptr, *end;
-	fwts_list_link  *item;
-	fwts_list *objects;
-	int i = -1, strings = 0;
-	char *ptr1, *ptr2;
-	char *expanded;
+	int strings = 0;
 
 	if (stao->header.length > (uint32_t)table->length) {
 		passed = false;
@@ -124,82 +120,28 @@ static int stao_test1(fwts_framework *fw)
 	if (!strings)
 		goto done;
 
-        if (fwts_acpi_init(fw) != FWTS_OK) {
-		fwts_log_error(fw, "Cannot initialise ACPI, skipping ACPI string check");
-		goto done;
-	}
-	if ((objects = fwts_acpi_object_get_names()) == NULL) {
-		fwts_log_info(fw, "Cannot find any ACPI objects");
-		fwts_acpi_deinit(fw);
-		goto deinit;
-	}
 	ptr = (char *)stao->namelist;
 	end = (char *)table->data + table->length;
 
 	while (ptr < end) {
-		bool not_found = true;
+		bool found;
 		size_t len;
 
 		if (!stao_acpi_string(fw, ptr, end, &passed, &len))
 			break;
 
-		/* Find number of '.' in object path */
-		for (i = 0, ptr1 = ptr; *ptr1; ptr1++) {
-			if (*ptr1 == '.')
-				i++;
-		}
-
-		/*
-		 *  Allocate buffer big enough to take expanded path
-		 *  and pad out any fields that are not 4 chars in size
-		 *  between each . separator
-		 *
-		 *  Converts \_SB.A.BB.CCC.DDDD.EE to
-		 *	     \_SB_.A___.BB__.CCC_.DDDD.EE__
-		 */
-		expanded = malloc(1 + (5 * (i + 1)));
-		if (!expanded) {
-			fwts_log_error(fw, "Cannot allocate temporary ACPI string buffer");
-			goto deinit;
-		}
-
-		for (i = -1, ptr1 = ptr, ptr2 = expanded; ; ptr1++) {
-			if (*ptr1 == '.' || *ptr1 == '\0') {
-				while (i < 4) {
-					*ptr2++ = '_';
-					i++;
-				}
-				i = 0;
-			} else {
-				i++;
-			}
-			*ptr2++ = *ptr1;
-			if (!*ptr1)
-				break;
-		}
-
-		/* Search for object */
-		fwts_list_foreach(item, objects) {
-			char *name = fwts_list_data(char *, item);
-			if (!strcmp(expanded, name)) {
-				not_found = false;
-				break;
-			}
-		}
+		found = fwts_acpi_obj_find(fw, ptr);
 
-		if (not_found) {
+		if (!found) {
 			passed = false;
 			fwts_failed(fw, LOG_LEVEL_HIGH,
 				"STAOAcpiStringNotFound",
 				"STAO ACPI String '%s' not found in ACPI object name space.",
-				expanded);
+				ptr);
 		}
-		free(expanded);
 		ptr += len + 1;
 	}
 
-deinit:
-	fwts_acpi_deinit(fw);
 done:
 	if (passed)
 		fwts_passed(fw, "No issues found in STAO table.");
diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
index 06ae373..a16a4d3 100644
--- a/src/lib/include/fwts_acpi_tables.h
+++ b/src/lib/include/fwts_acpi_tables.h
@@ -49,6 +49,7 @@ int fwts_acpi_free_tables(void);
 int fwts_acpi_find_table(fwts_framework *fw, const char *name, const int which, fwts_acpi_table_info **info);
 int fwts_acpi_find_table_by_addr(fwts_framework *fw, const uint64_t addr, fwts_acpi_table_info **info);
 int fwts_acpi_get_table(fwts_framework *fw, const int index, fwts_acpi_table_info **info);
+bool fwts_acpi_obj_find(fwts_framework *fw, const char *obj_name);
 
 fwts_bool fwts_acpi_is_reduced_hardware(const fwts_acpi_table_fadt *fadt);
 
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
index 5e0fe1d..30b4060 100644
--- a/src/lib/src/fwts_acpi_tables.c
+++ b/src/lib/src/fwts_acpi_tables.c
@@ -35,6 +35,8 @@
 
 #include "fwts.h"
 
+#include "fwts_acpi_object_eval.h"
+
 #if defined(FWTS_HAS_ACPI)
 
 #define BIOS_START	(0x000e0000)		/* Start of BIOS memory */
@@ -1282,4 +1284,62 @@ int fwts_acpi_get_table(fwts_framework *fw, const int index, fwts_acpi_table_inf
 	return FWTS_OK;
 }
 
+/*
+ *  fwts_acpi_obj_find()
+ *  Returns whether obj_name can be found in the ACPI object name space.
+ */
+bool fwts_acpi_obj_find(fwts_framework *fw, const char *obj_name)
+{
+	char expanded[BUFSIZ];
+	char *c = expanded;
+	const char *obj_ptr;
+	int i;
+	fwts_list_link *item;
+	fwts_list *objects;
+	bool found = false;
+
+	if (fwts_acpi_init(fw) != FWTS_OK) {
+		fwts_log_error(fw, "Cannot initialise ACPI.");
+		return false;
+	}
+	if ((objects = fwts_acpi_object_get_names()) == NULL) {
+		fwts_log_info(fw, "Cannot find any ACPI objects");
+		fwts_acpi_deinit(fw);
+		return false;
+	}
+
+	memset(expanded, 0, BUFSIZ);
+
+	/*
+	 *  Converts \_SB.A.BB.CCC.DDDD.EE to
+	 *           \_SB_.A___.BB__.CCC_.DDDD.EE__
+	 */
+	for (i = -1, obj_ptr = obj_name; ; obj_ptr++) {
+		if (*obj_ptr == '.' || *obj_ptr == '\0') {
+			while (i < 4) {
+				*c++ = '_';
+				i++;
+			}
+			i = 0;
+		} else {
+			i++;
+		}
+		*c++ = *obj_ptr;
+		if (!*obj_ptr)
+			break;
+	}
+
+	/* Search for object */
+	fwts_list_foreach(item, objects) {
+		char *name = fwts_list_data(char*, item);
+		if (strcmp(expanded, name) == 0) {
+			found = true;
+			break;
+		}
+	}
+
+	fwts_acpi_deinit(fw);
+	return found;
+}
+
 #endif
-- 
2.7.4




More information about the fwts-devel mailing list