[Skiboot] [PATCH] hdata: Make hdata_to_dt more suitable for fuzzing

Stewart Smith stewart at linux.vnet.ibm.com
Tue May 17 14:59:57 AEST 2016


We make parse_hdat() return success/failure rather than assert.
This allows the hdata_to_dt binary to gracefully error out rather
than assert, which is useful when throwing it at a fuzzer.

Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
 core/init.c              | 12 +++++++-----
 hdata/hdata.h            |  2 +-
 hdata/paca.c             |  5 +++--
 hdata/spira.c            |  7 +++++--
 hdata/test/hdata_to_dt.c | 30 ++++++++++++++++++++++++++----
 include/skiboot.h        |  2 +-
 6 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/core/init.c b/core/init.c
index a72972ddd0ec..4542a1d8fb97 100644
--- a/core/init.c
+++ b/core/init.c
@@ -634,11 +634,13 @@ void __noreturn main_cpu_entry(const void *fdt, u32 master_cpu)
 	 * Hack alert: When entering via the OPAL entry point, fdt
 	 * is set to -1, we record that and pass it to parse_hdat
 	 */
-	if (fdt == (void *)-1ul)
-		parse_hdat(true, master_cpu);
-	else if (fdt == NULL)
-		parse_hdat(false, master_cpu);
-	else {
+	if (fdt == (void *)-1ul) {
+		if (parse_hdat(true, master_cpu) < 0)
+			abort();
+	} else if (fdt == NULL) {
+		if (parse_hdat(false, master_cpu) < 0)
+			abort();
+	} else {
 		dt_expand(fdt);
 	}
 
diff --git a/hdata/hdata.h b/hdata/hdata.h
index 567927c2443c..1d0da1e9992e 100644
--- a/hdata/hdata.h
+++ b/hdata/hdata.h
@@ -20,7 +20,7 @@
 struct dt_node;
 
 extern void memory_parse(void);
-extern void paca_parse(void);
+extern int paca_parse(void);
 extern bool pcia_parse(void);
 extern void fsp_parse(void);
 extern void io_parse(void);
diff --git a/hdata/paca.c b/hdata/paca.c
index 145b825ddc6d..6d001fdcdad3 100644
--- a/hdata/paca.c
+++ b/hdata/paca.c
@@ -327,10 +327,11 @@ static bool __paca_parse(void)
 	return true;
 }
 
-void paca_parse(void)
+int paca_parse(void)
 {
 	if (!__paca_parse()) {
 		prerror("CPU: Initial CPU parsing failed\n");
-		abort();
+		return -1;
 	}
+	return 0;
 }
diff --git a/hdata/spira.c b/hdata/spira.c
index bff6d71bf6eb..cb97615a3eae 100644
--- a/hdata/spira.c
+++ b/hdata/spira.c
@@ -1085,7 +1085,7 @@ static void fixup_spira(void)
 	spira.ntuples.hs_data = spiras->ntuples.hs_data;
 }
 
-void parse_hdat(bool is_opal, uint32_t master_cpu)
+int parse_hdat(bool is_opal, uint32_t master_cpu)
 {
 	cpu_type = PVR_TYPE(mfspr(SPR_PVR));
 
@@ -1110,7 +1110,8 @@ void parse_hdat(bool is_opal, uint32_t master_cpu)
 
 	/* Parse SPPACA and/or PCIA */
 	if (!pcia_parse())
-		paca_parse();
+		if (paca_parse() < 0)
+			return -1;
 
 	/* IPL params */
 	add_iplparams();
@@ -1144,4 +1145,6 @@ void parse_hdat(bool is_opal, uint32_t master_cpu)
 	slca_dt_add_sai_node();
 
 	prlog(PR_INFO, "Parsing HDAT...done\n");
+
+	return 0;
 }
diff --git a/hdata/test/hdata_to_dt.c b/hdata/test/hdata_to_dt.c
index 94f1de6982fd..2ed683e98836 100644
--- a/hdata/test/hdata_to_dt.c
+++ b/hdata/test/hdata_to_dt.c
@@ -83,7 +83,9 @@ struct dt_node *add_ics_node(void)
 #include <bitutils.h>
 
 /* Your pointers won't be correct, that's OK. */
-#define spira_check_ptr(ptr, file, line) ((ptr) != NULL)
+#define spira_check_ptr spira_check_ptr
+
+static bool spira_check_ptr(const void *ptr, const char *file, unsigned int line);
 
 #include "../cpu-common.c"
 #include "../fsp.c"
@@ -108,13 +110,30 @@ char __rodata_start[1], __rodata_end[1];
 
 enum proc_gen proc_gen = proc_gen_p7;
 
+static bool spira_check_ptr(const void *ptr, const char *file, unsigned int line)
+{
+	if (!ptr)
+		return false;
+	/* we fake the SPIRA pointer as it's relative to where it was loaded
+	 * on real hardware */
+	(void)file;
+	(void)line;
+	return true;
+}
+
 static void *ntuple_addr(const struct spira_ntuple *n)
 {
 	uint64_t addr = be64_to_cpu(n->addr);
 	if (n->addr == 0)
 		return NULL;
-	assert(addr >= base_addr);
-	assert(addr < base_addr + spira_heap_size);
+	if (addr < base_addr) {
+		fprintf(stderr, "assert failed: addr >= base_addr (%"PRIu64" >= %"PRIu64")\n", addr, base_addr);
+		exit(EXIT_FAILURE);
+	}
+	if (addr >= base_addr + spira_heap_size) {
+		fprintf(stderr, "assert failed: addr not in spira_heap\n");
+		exit(EXIT_FAILURE);
+	}
 	return spira_heap + ((unsigned long)addr - base_addr);
 }
 
@@ -211,7 +230,10 @@ int main(int argc, char *argv[])
 		fclose(stderr);
 	}
 
-	parse_hdat(false, 0);
+	if(parse_hdat(false, 0) < 0) {
+		fprintf(stderr, "FATAL ERROR parsing HDAT\n");
+		exit(EXIT_FAILURE);
+	}
 
 	if (!quiet)
 		dump_dt(dt_root, 0, !tree_only);
diff --git a/include/skiboot.h b/include/skiboot.h
index bece690631e7..a878d213ea96 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -175,7 +175,7 @@ extern void start_kernel32(uint64_t entry, void* fdt,
 extern void start_kernel_secondary(uint64_t entry) __noreturn;
 
 /* Get description of machine from HDAT and create device-tree */
-extern void parse_hdat(bool is_opal, uint32_t master_cpu);
+extern int parse_hdat(bool is_opal, uint32_t master_cpu);
 
 /* Root of device tree. */
 extern struct dt_node *dt_root;
-- 
2.1.4



More information about the Skiboot mailing list