Suggestion for prekern: warning messages

Pierre Pronchery <[email protected]> Mon, 12 Apr 2021 03:56:29 +0200

Newsgroups	gmane.os.netbsd.ports.x86-64
Message-ID	<[email protected]>

			Hi port-amd64@,

I’d like to have opinions on a few changes I am preparing for the prekern. (Patch attached)

Basically I would like to modify print_state() to accept more contexts than just ok/fail, and sprinkle a few more warning messages:
- when no hardware entropy instruction is supported
- when the entropy file was corrupted (instead of panic, rationale: none may have been found or specified)
- when no entropy file could be loaded

I introduced a yellow “[*]" symbol for this purpose (alternative suggestions welcome).

In my tests with QEMU (5.2.0 from pkgsrc 2021Q1 on macOS Catalina amd64) no CPU entropy instruction was detected (RDRAND and RDSEED are supported). Without an entropy file, I guess it only leaves the time of boot in order to guess the seed for KASLR, which is not ideal.

On another note, it might make sense to fully clear the screen before drawing the logo; otherwise the output is mangled with that of boot(8) when switching the console to e.g. a serial port for the kernel.

Another feature I have in mind is to detect the choice of console and support serial consoles in prekern. I guess it would make it easier to debug.



Cheers!
--
khorben

patch-prekern_warnings.diff (application/octet-stream, 6.1 KB)

diff --git a/sys/arch/amd64/stand/prekern/console.c b/sys/arch/amd64/stand/prekern/console.c
index b3fc82c89e1b..752f73826e52 100644
--- a/sys/arch/amd64/stand/prekern/console.c
+++ b/sys/arch/amd64/stand/prekern/console.c
@@ -94,13 +94,24 @@ void print(char *buf)
 	print_ext(WHITE_ON_BLACK, buf);
 }
 
-void print_state(bool ok, char *buf)
+void print_state(state_t state, char *buf)
 {
 	print("[");
-	if (ok)
-		print_ext(GREEN_ON_BLACK, "+");
-	else
-		print_ext(RED_ON_BLACK, "!");
+	switch (state)
+	{
+		case STATE_NORMAL:
+			print_ext(GREEN_ON_BLACK, "+");
+			break;
+		case STATE_ERROR:
+			print_ext(RED_ON_BLACK, "!");
+			break;
+		case STATE_WARNING:
+			print_ext(YELLOW_ON_BLACK, "*");
+			break;
+		default:
+			print_ext(WHITE_ON_BLACK, "?");
+			break;
+	}
 	print("] ");
 	print(buf);
 	print("\n");
diff --git a/sys/arch/amd64/stand/prekern/elf.c b/sys/arch/amd64/stand/prekern/elf.c
index a4ee9f09f3ee..cbde65ab37df 100644
--- a/sys/arch/amd64/stand/prekern/elf.c
+++ b/sys/arch/amd64/stand/prekern/elf.c
@@ -377,7 +377,7 @@ elf_kernel_reloc(void)
 	Elf_Sym *sym;
 	size_t i, j;
 
-	print_state(true, "ELF info created");
+	print_state(STATE_NORMAL, "ELF info created");
 
 	/*
 	 * Update all symbol values with the appropriate offset.
@@ -398,7 +398,7 @@ elf_kernel_reloc(void)
 		}
 	}
 
-	print_state(true, "Symbol values updated");
+	print_state(STATE_NORMAL, "Symbol values updated");
 
 	/*
 	 * Perform relocations without addend if there are any.
@@ -427,7 +427,7 @@ elf_kernel_reloc(void)
 		}
 	}
 
-	print_state(true, "REL relocations applied");
+	print_state(STATE_NORMAL, "REL relocations applied");
 
 	/*
 	 * Perform relocations with addend if there are any.
@@ -456,7 +456,7 @@ elf_kernel_reloc(void)
 		}
 	}
 
-	print_state(true, "RELA relocations applied");
+	print_state(STATE_NORMAL, "RELA relocations applied");
 
 	/*
 	 * Get the entry point.
@@ -466,7 +466,7 @@ elf_kernel_reloc(void)
 		fatal("elf_kernel_reloc: entry point not found");
 	}
 
-	print_state(true, "Entry point found");
+	print_state(STATE_NORMAL, "Entry point found");
 
 	return ent;
 }
diff --git a/sys/arch/amd64/stand/prekern/mm.c b/sys/arch/amd64/stand/prekern/mm.c
index f7575c1e95db..c7e07403110b 100644
--- a/sys/arch/amd64/stand/prekern/mm.c
+++ b/sys/arch/amd64/stand/prekern/mm.c
@@ -148,7 +148,7 @@ mm_bootspace_mprotect(void)
 		mm_mprotect(bootspace.segs[i].va, bootspace.segs[i].sz, prot);
 	}
 
-	print_state(true, "Segments protection updated");
+	print_state(STATE_NORMAL, "Segments protection updated");
 }
 
 static size_t
@@ -439,9 +439,9 @@ mm_map_kernel(void)
 {
 	memset(&bootspace, 0, sizeof(bootspace));
 	mm_map_head();
-	print_state(true, "Head region mapped");
+	print_state(STATE_NORMAL, "Head region mapped");
 	elf_map_sections();
-	print_state(true, "Segments mapped");
+	print_state(STATE_NORMAL, "Segments mapped");
 	mm_map_boot();
-	print_state(true, "Boot region mapped");
+	print_state(STATE_NORMAL, "Boot region mapped");
 }
diff --git a/sys/arch/amd64/stand/prekern/prekern.c b/sys/arch/amd64/stand/prekern/prekern.c
index 410a12d35954..b63092dfb34a 100644
--- a/sys/arch/amd64/stand/prekern/prekern.c
+++ b/sys/arch/amd64/stand/prekern/prekern.c
@@ -286,7 +286,7 @@ init_prekern(paddr_t pa_start)
 	 */
 	init_idt();
 
-	print_state(true, "Prekern loaded");
+	print_state(STATE_NORMAL, "Prekern loaded");
 
 	/*
 	 * Init the PRNG.
@@ -308,7 +308,7 @@ init_prekern(paddr_t pa_start)
 	/*
 	 * Finally, jump into the kernel.
 	 */
-	print_state(true, "Jumping into the kernel");
+	print_state(STATE_NORMAL, "Jumping into the kernel");
 	jump_kernel(ent);
 
 	fatal("init_prekern: unreachable!");
diff --git a/sys/arch/amd64/stand/prekern/prekern.h b/sys/arch/amd64/stand/prekern/prekern.h
index fc9dd6315c9f..2fcfacce27f8 100644
--- a/sys/arch/amd64/stand/prekern/prekern.h
+++ b/sys/arch/amd64/stand/prekern/prekern.h
@@ -42,6 +42,7 @@ typedef uint64_t pte_prot_t;
 #define WHITE_ON_BLACK 0x07
 #define RED_ON_BLACK 0x04
 #define GREEN_ON_BLACK 0x02
+#define YELLOW_ON_BLACK 0x0e
 
 #define HEAD_WINDOW_BASE	(KERNBASE - NBPD_L3)
 #define HEAD_WINDOW_SIZE	NBPD_L3
@@ -49,6 +50,13 @@ typedef uint64_t pte_prot_t;
 #define KASLR_WINDOW_BASE	KERNBASE		/* max - 2GB */
 #define KASLR_WINDOW_SIZE	(2LLU * (1 << 30))	/* 2GB */
 
+typedef enum
+{
+	STATE_NORMAL = 0,
+	STATE_ERROR,
+	STATE_WARNING
+} state_t;
+
 /* -------------------------------------------------------------------------- */
 
 #define BTSEG_NONE	0
@@ -83,7 +91,7 @@ struct bootspace {
 void init_cons(void);
 void print_ext(int, char *);
 void print(char *);
-void print_state(bool, char *);
+void print_state(state_t, char *);
 void print_banner(void);
 
 /* elf.c */
diff --git a/sys/arch/amd64/stand/prekern/prng.c b/sys/arch/amd64/stand/prekern/prng.c
index 95087a3d4aa5..075aa19bc309 100644
--- a/sys/arch/amd64/stand/prekern/prng.c
+++ b/sys/arch/amd64/stand/prekern/prng.c
@@ -84,6 +84,7 @@ prng_get_entropy_file(SHA512_CTX *ctx)
 	uint8_t digest[SHA1_DIGEST_LENGTH];
 	rndsave_t *rndsave;
 	SHA1_CTX sig;
+	size_t count = 0;
 
 	biml =
 	    (struct btinfo_modulelist *)prng_lookup_bootinfo(BTINFO_MODULELIST);
@@ -98,7 +99,9 @@ prng_get_entropy_file(SHA512_CTX *ctx)
 			continue;
 		}
 		if (bi->len != sizeof(rndsave_t)) {
-			fatal("rndsave_t size mismatch");
+			print_state(STATE_WARNING,
+					"size mismatch in entropy file");
+			continue;
 		}
 		rndsave = (rndsave_t *)(vaddr_t)bi->base;
 
@@ -109,11 +112,16 @@ prng_get_entropy_file(SHA512_CTX *ctx)
 		SHA1Update(&sig, rndsave->data, sizeof(rndsave->data));
 		SHA1Final(digest, &sig);
 		if (memcmp(digest, rndsave->digest, sizeof(digest))) {
-			fatal("bad SHA1 checksum");
+			print_state(STATE_WARNING,
+					"bad SHA1 checksum in entropy file");
+			continue;
 		}
 
 		SHA512_Update(ctx, rndsave->data, sizeof(rndsave->data));
+		count++;
 	}
+	if (count == 0)
+		print_state(STATE_WARNING, "no entropy file could be loaded");
 }
 
 /*
@@ -159,6 +167,8 @@ prng_init(void)
 	has_rdseed = (descs[1] & CPUID_SEF_RDSEED) != 0;
 	cpuid(0x01, 0x00, descs);
 	has_rdrand = (descs[2] & CPUID2_RDRAND) != 0;
+	if (!has_rdseed && !has_rdrand)
+		print_state(STATE_WARNING, "No CPU entropy feature detected");
 
 	SHA512_Init(&ctx);
 	prng_get_entropy_file(&ctx);

signature.asc (application/pgp-signature, 833 B)