[coreboot] [PATCH] v3: Improve printk reliability

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Mar 19 13:17:25 CET 2009


This patch tries to make printk more readable and reliable.

Base 16 number printing used digits[33] instead of the more readable 'x'
for the "0x" prefix. That means you have to look up an array just to
find out what the code does. Change it.

We already write "<NULL>" if printk gets a NULL pointer as string
argument. Introduce "<almost NULL>" for string arguments with addresses
below 0x400. This error happened in the past when dereferencing a struct
member with a string and the struct had the address NULL.

Check if all to-be-printed characters in the string are indeed
printable. The idea is to catch garbage strings from stray pointers and
print "<GARBAGE>" instead. This is the only part of the patch I'm not so
sure about. If a string contains characters outside classic ASCII
printable stuff, this will trigger. An example would be characters with
diacritic marks like äöüéăçőč. Then again, I don't see localized
versions of coreboot on the horizon. Maybe for payloads, but not for
coreboot itself.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: corebootv3-printk_safe_strings/lib/vtxprintf.c
===================================================================
--- corebootv3-printk_safe_strings/lib/vtxprintf.c	(Revision 1157)
+++ corebootv3-printk_safe_strings/lib/vtxprintf.c	(Arbeitskopie)
@@ -16,6 +16,7 @@
 #define isdigit(c)	((c) >= '0' && (c) <= '9')
 #define is_digit isdigit
 #define isxdigit(c)	(((c) >= '0' && (c) <= '9') || ((c) >= 'a' && (c) <= 'f') || ((c) >= 'A' && (c) <= 'F'))
+#define isprint(c)	((c) == '\n' || ((c) >= ' ' && (c) <= '~'))
 
 static int skip_atoi(const char **s)
 {
@@ -87,7 +88,7 @@
 			tx_byte('0', arg), count++;
 		else if (base==16) {
 			tx_byte('0', arg), count++;
-			tx_byte(digits[33], arg), count++;
+			tx_byte('x', arg), count++;
 		}
 	}
 	if (!(type & LEFT))
@@ -194,9 +195,15 @@
 			s = va_arg(args, char *);
 			if (!s)
 				s = "<NULL>";
+			/* Catch almost-NULL pointers as well */
+			if ((size_t)s < 0x400)
+				s = "<almost NULL>";
 
 			len = strnlen(s, precision);
 
+			for (i = 0; i < len; ++i)
+				if (!isprint(*s[i]))
+					s = "<GARBAGE>";
 			if (!(flags & LEFT))
 				while (len < field_width--)
 					tx_byte(' ', arg), count++;


-- 
http://www.hailfinger.org/

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: linuxbios3_printk_safe_strings.diff
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090319/87e13ac9/attachment.ksh>


More information about the coreboot mailing list