[patch 0/4] Various libpayload fixes

Here's a chunk of patches fixing various bugs in libpayload.

[patch 1/4] libpayload: Add the null terminator to the end of the duplicated string

Here's a chunk of patches fixing various bugs in libpayload.  
Content-Disposition: inline; filename=fix-strdup.patch

Signed-off-by: Jordan Crouse 
Index: libpayload/libc/string.c
===================================================================
--- libpayload.orig/libc/string.c	2008-04-24 17:58:11.000000000 -0600
+++ libpayload/libc/string.c	2008-04-24 17:58:04.000000000 -0600
@@ -173,11 +173,12 @@
 char *strdup(const char *s)
 {
    int n = strlen(s);
-	char *p = malloc(n);
+	char *p = malloc(n + 1);

if (p != NULL)
    	strncpy(p, s, n);

+	p[n] = 0;
    return p;
 }

[patch 1/4] libpayload: Add the null terminator to theend of the duplicated string

[Myles Watson]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]]
> On Behalf Of [e-mail]
> Sent: Friday, April 25, 2008 9:52 AM
> To: [e-mail]
> Cc: Jordan Crouse
> Subject: [coreboot] [patch 1/4] libpayload: Add the null terminator to
> theend of the duplicated string
>
> Here's a chunk of patches fixing various bugs in libpayload.
> Content-Disposition: inline; filename=fix-strdup.patch
>
> Signed-off-by: Jordan Crouse 
> Index: libpayload/libc/string.c
> ===================================================================
> --- libpayload.orig/libc/string.c	2008-04-24 17:58:11.000000000 -0600
> +++ libpayload/libc/string.c	2008-04-24 17:58:04.000000000 -0600
> @@ -173,11 +173,12 @@
>  char *strdup(const char *s)
>  {
>    int n = strlen(s);
> char *p = malloc(n);
>
> char *p = malloc(n + 1);
>
> if (p != NULL)
> strncpy(p, s, n);
>
> p[n] = 0;

It seems simpler to use strcpy here instead of strncpy and manual null
termination, since strlen gave you the size already.

Either way I'll agree that something needed to be done.

Myles

Acked-by: Myles Watson

[patch 2/4] libpayload: Fix malloc allocation

Here's a chunk of patches fixing various bugs in libpayload.  
Content-Disposition: inline; filename=fix-malloc.patch

Apparently the previous version worked on luck.  Fix the allocation
and add parens to better guide the compiler.  Also, halt() if 
the heap is poisoned (like by an overrun).  Finally, fix calloc()
so that it actually works/

Signed-off-by: Jordan Crouse 
Index: libpayload/libc/malloc.c
===================================================================
--- libpayload.orig/libc/malloc.c	2008-04-24 17:59:10.000000000 -0600
+++ libpayload/libc/malloc.c	2008-04-24 17:58:36.000000000 -0600
@@ -67,7 +67,8 @@

static void setup(void)
 {
-	int size = (unsigned int)(_heap - _eheap) - HDRSIZE;
+	int size = (unsigned int)(&_eheap - &_heap) - HDRSIZE;
+
    *((hdrtype_t *) hstart) = FREE_BLOCK(size);
 }

@@ -91,9 +92,12 @@
    	header = *((hdrtype_t *) ptr);
    	int size = SIZE(header);

+		if (!HAS_MAGIC(header) || size == 0)
+			halt();
+
    	if (header & FLAG_FREE) {
    		if (len <= size) {
-				void *nptr = ptr + HDRSIZE + len;
+				void *nptr = ptr + (HDRSIZE + len);
    			int nsize = size - (len + 8);

/* Mark the block as used. */
@@ -102,6 +106,7 @@
    			/* If there is still room in this block,
    			 * then mark it as such.
    			 */
+
    			if (nsize > 0)
    				*((hdrtype_t *) nptr) =
    				    FREE_BLOCK(nsize - 4);
@@ -184,8 +189,8 @@

void *calloc(size_t nmemb, size_t size)
 {
-	unsigned int total = (nmemb * size);
-	void *ptr = alloc(size);
+	size_t total = nmemb * size;
+	void *ptr = alloc(total);

if (ptr)
    	memset(ptr, 0, total);

[patch 2/4] libpayload: Fix malloc allocation

[Peter Stuge]

On Fri, Apr 25, 2008 at 09:52:11AM -0600, [e-mail] wrote:
> Here's a chunk of patches fixing various bugs in libpayload.  
> Content-Disposition: inline; filename=fix-malloc.patch

Something seems broken in your thingy that sends out patches.
> Apparently the previous version worked on luck.  Fix the allocation
> and add parens to better guide the compiler.  Also, halt() if 
> the heap is poisoned (like by an overrun).  Finally, fix calloc()
> so that it actually works/
>
> Signed-off-by: Jordan Crouse

Acked-by: Peter Stuge
> Index: libpayload/libc/malloc.c
>
> --- libpayload.orig/libc/malloc.c	2008-04-24 17:59:10.000000000 -0600
> +++ libpayload/libc/malloc.c	2008-04-24 17:58:36.000000000 -0600
> @@ -67,7 +67,8 @@
>
> static void setup(void)
>  {
> int size = (unsigned int)(_heap - _eheap) - HDRSIZE;
> int size = (unsigned int)(&_eheap - &_heap) - HDRSIZE;
> *((hdrtype_t *) hstart) = FREE_BLOCK(size);
>
> }
>
> @@ -91,9 +92,12 @@
>    	header = *((hdrtype_t *) ptr);
>    	int size = SIZE(header);
> if (!HAS_MAGIC(header) || size == 0)
> 	halt();
> if (header & FLAG_FREE) {
>   	if (len <= size) {
> 		void *nptr = ptr + HDRSIZE + len;
> 		void *nptr = ptr + (HDRSIZE + len);
> 	int nsize = size - (len + 8);
>
> 	/* Mark the block as used. */
>
> @@ -102,6 +106,7 @@
>    			/* If there is still room in this block,
>    			 * then mark it as such.
>    			 */
> 	if (nsize > 0)
> 		*((hdrtype_t *) nptr) =
> 		    FREE_BLOCK(nsize - 4);
>
> @@ -184,8 +189,8 @@
>
> void *calloc(size_t nmemb, size_t size)
>  {
> unsigned int total = (nmemb * size);
> void *ptr = alloc(size);
>
> size_t total = nmemb * size;
>
> void *ptr = alloc(total);
>
> if (ptr)
> memset(ptr, 0, total);

libpayload: Fix malloc allocation

[Jordan Crouse]

On 25/04/08 20:55 +0200, Peter Stuge wrote:
> On Fri, Apr 25, 2008 at 09:52:11AM -0600, [e-mail] wrote:
> > Here's a chunk of patches fixing various bugs in libpayload.  
> > Content-Disposition: inline; filename=fix-malloc.patch
>
> Something seems broken in your thingy that sends out patches.

Thats quilt for ya.  Not sure what it thought it was doing - it never
has done that before.
> > Apparently the previous version worked on luck.  Fix the allocation
> > and add parens to better guide the compiler.  Also, halt() if 
> > the heap is poisoned (like by an overrun).  Finally, fix calloc()
> > so that it actually works/
> >
> > Signed-off-by: Jordan Crouse
>
> Acked-by: Peter Stuge
> > Index: libpayload/libc/malloc.c
> >
> > --- libpayload.orig/libc/malloc.c	2008-04-24 17:59:10.000000000 -0600
> > +++ libpayload/libc/malloc.c	2008-04-24 17:58:36.000000000 -0600
> > @@ -67,7 +67,8 @@
> >
> > static void setup(void)
> >  {
> > int size = (unsigned int)(_heap - _eheap) - HDRSIZE;
> > int size = (unsigned int)(&_eheap - &_heap) - HDRSIZE;
> > *((hdrtype_t *) hstart) = FREE_BLOCK(size);
> >
> > }
> >
> > @@ -91,9 +92,12 @@
> >    	header = *((hdrtype_t *) ptr);
> >    	int size = SIZE(header);
> > if (!HAS_MAGIC(header) || size == 0)
> > 	halt();
> > if (header & FLAG_FREE) {
> >   	if (len <= size) {
> > 		void *nptr = ptr + HDRSIZE + len;
> > 		void *nptr = ptr + (HDRSIZE + len);
> > 	int nsize = size - (len + 8);
> >
> > 	/* Mark the block as used. */
> >
> > @@ -102,6 +106,7 @@
> >    			/* If there is still room in this block,
> >    			 * then mark it as such.
> >    			 */
> > 	if (nsize > 0)
> > 		*((hdrtype_t *) nptr) =
> > 		    FREE_BLOCK(nsize - 4);
> >
> > @@ -184,8 +189,8 @@
> >
> > void *calloc(size_t nmemb, size_t size)
> >  {
> > unsigned int total = (nmemb * size);
> > void *ptr = alloc(size);
> >
> > size_t total = nmemb * size;
> >
> > void *ptr = alloc(total);
> >
> > if (ptr)
> > memset(ptr, 0, total);

[patch 3/4] libpayload: Enable keyboard translation so that we can use scancode set 1

Here's a chunk of patches fixing various bugs in libpayload.  
Content-Disposition: inline; filename=use-keyboard-xlate.patch

The qemu keyboard controller defaults to using scancode set 2, we use set 1.
Turn on the translate mode in the keyboard controller to force the issue.

Signed-off-by: Jordan Crouse 
Index: libpayload/drivers/keyboard.c
===================================================================
--- libpayload.orig/drivers/keyboard.c	2008-04-23 17:33:07.000000000 -0600
+++ libpayload/drivers/keyboard.c	2008-04-23 17:33:27.000000000 -0600
@@ -29,6 +29,11 @@

#include <libpayload.h>

+#define I8042_CMD_READ_MODE  0x20
+#define I8042_CMD_WRITE_MODE 0x60
+
+#define I8042_MODE_XLATE     0x40
+
 unsigned char map[2][0x57] = {
    {
     0x00, 0x1B, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36,
@@ -118,3 +123,54 @@

return ret;
 }
+
+static int keyboard_wait_read(void)
+{
+	int timeout = 10000;
+
+	while(timeout-- && !(inb(0x64) & 0x01))
+		udelay(50);
+
+	return (timeout <= 0) ? -1 : 0;
+}
+
+static int keyboard_wait_write(void)
+{
+	int timeout = 10000;
+
+	while(timeout-- && (inb(0x64) & 0x02))
+		udelay(50);
+
+	return (timeout <= 0) ? -1 : 0;
+}
+
+static unsigned char keyboard_get_mode(void)
+{
+	outb(I8042_CMD_READ_MODE, 0x64);
+	keyboard_wait_read();
+	return inb(0x60);
+}
+
+static void keyboard_set_mode(unsigned char mode)
+{
+	outb(I8042_CMD_WRITE_MODE, 0x64);
+	keyboard_wait_write();
+	outb(mode, 0x60);
+}
+
+void keyboard_init(void)
+{
+	u8 mode;
+
+	/* Read the current mode */
+	mode = keyboard_get_mode();
+
+	/* Turn on scancode translate mode so that we can
+	   use the scancode set 1 tables */
+
+	mode |= I8042_MODE_XLATE;
+
+	/* Write the new mode */
+	keyboard_set_mode(mode);
+}
+
Index: libpayload/include/libpayload.h
===================================================================
--- libpayload.orig/include/libpayload.h	2008-04-23 17:33:07.000000000 -0600
+++ libpayload/include/libpayload.h	2008-04-24 08:59:35.000000000 -0600
@@ -67,6 +67,7 @@
 void nvram_write(u8 val, u8 addr);

/* drivers/keyboard.c */
+void keyboard_init(void);
 int keyboard_havechar(void);
 unsigned char keyboard_get_scancode(void);
 int keyboard_getchar(void);
Index: libpayload/libc/console.c
===================================================================
--- libpayload.orig/libc/console.c	2008-04-23 17:33:07.000000000 -0600
+++ libpayload/libc/console.c	2008-04-23 17:33:18.000000000 -0600
@@ -37,6 +37,9 @@
 #ifdef CONFIG_SERIAL_CONSOLE
    serial_init();
 #endif
+#ifdef CONFIG_PC_KEYBOARD
+	keyboard_init();
+#endif
 }

static void device_putchar(unsigned char c)

[patch 3/4] libpayload: Enable keyboard translation so that we can use scancode set 1

[Peter Stuge]

On Fri, Apr 25, 2008 at 09:52:12AM -0600, [e-mail] wrote:
> The qemu keyboard controller defaults to using scancode set 2, we use set 1.
> Turn on the translate mode in the keyboard controller to force the issue.
>
> Signed-off-by: Jordan Crouse

Acked-by: Peter Stuge
> Index: libpayload/drivers/keyboard.c
>
> --- libpayload.orig/drivers/keyboard.c	2008-04-23 17:33:07.000000000 -0600
> +++ libpayload/drivers/keyboard.c	2008-04-23 17:33:27.000000000 -0600
> @@ -29,6 +29,11 @@
>
> #include <libpayload.h>
>
> +#define I8042_CMD_READ_MODE  0x20
> +#define I8042_CMD_WRITE_MODE 0x60
>
> +#define I8042_MODE_XLATE     0x40
>
> unsigned char map[2][0x57] = {
>    {
>     0x00, 0x1B, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36,
> @@ -118,3 +123,54 @@
>
> return ret;
>  }
>
> +static int keyboard_wait_read(void)
> +{
> int timeout = 10000;
> while(timeout-- && !(inb(0x64) & 0x01))
> udelay(50);
> return (timeout <= 0) ? -1 : 0;
>
> +}
>
> +static int keyboard_wait_write(void)
> +{
> int timeout = 10000;
> while(timeout-- && (inb(0x64) & 0x02))
> udelay(50);
> return (timeout <= 0) ? -1 : 0;
>
> +}
>
> +static unsigned char keyboard_get_mode(void)
> +{
> outb(I8042_CMD_READ_MODE, 0x64);
> keyboard_wait_read();
> return inb(0x60);
>
> +}
>
> +static void keyboard_set_mode(unsigned char mode)
> +{
> outb(I8042_CMD_WRITE_MODE, 0x64);
> keyboard_wait_write();
> outb(mode, 0x60);
>
> +}
>
> +void keyboard_init(void)
> +{
> u8 mode;
> /* Read the current mode */
> mode = keyboard_get_mode();
> /* Turn on scancode translate mode so that we can
>   use the scancode set 1 tables */
> mode |= I8042_MODE_XLATE;
> /* Write the new mode */
> keyboard_set_mode(mode);
>
> +}
> Index: libpayload/include/libpayload.h
>
> --- libpayload.orig/include/libpayload.h	2008-04-23 17:33:07.000000000 -0600
> +++ libpayload/include/libpayload.h	2008-04-24 08:59:35.000000000 -0600
> @@ -67,6 +67,7 @@
>  void nvram_write(u8 val, u8 addr);
>
> /* drivers/keyboard.c */
> +void keyboard_init(void);
>  int keyboard_havechar(void);
>  unsigned char keyboard_get_scancode(void);
>  int keyboard_getchar(void);
> Index: libpayload/libc/console.c
> ===================================================================
> --- libpayload.orig/libc/console.c	2008-04-23 17:33:07.000000000 -0600
> +++ libpayload/libc/console.c	2008-04-23 17:33:18.000000000 -0600
> @@ -37,6 +37,9 @@
>  #ifdef CONFIG_SERIAL_CONSOLE
>    serial_init();
>  #endif
> +#ifdef CONFIG_PC_KEYBOARD
> keyboard_init();
>
> +#endif
>  }
>
> static void device_putchar(unsigned char c)

libpayload: Enable keyboard translation so that we can use scancode set 1

[Jordan Crouse]

On 25/04/08 20:56 +0200, Peter Stuge wrote:
> On Fri, Apr 25, 2008 at 09:52:12AM -0600, [e-mail] wrote:
> > The qemu keyboard controller defaults to using scancode set 2, we use set 1.
> > Turn on the translate mode in the keyboard controller to force the issue.
> >
> > Signed-off-by: Jordan Crouse
>
> Acked-by: Peter Stuge

r3270.  Thanks.

[patch 4/4] libpayload: Fix a small but aggressive bug in printf()

Here's a chunk of patches fixing various bugs in libpayload.  
Content-Disposition: inline; filename=fix-evil-printf-bug.patch

This was causing the returned counter value to be one more then it
should be when printing a single character.

Signed-off-by: Jordan Crouse 
Index: libpayload/libc/printf.c
===================================================================
--- libpayload.orig/libc/printf.c	2008-04-24 11:44:17.000000000 -0600
+++ libpayload/libc/printf.c	2008-04-24 11:45:40.000000000 -0600
@@ -156,7 +156,7 @@
    		++counter;
    }

-	return ++counter;
+	return counter;
 }

/**

[patch 4/4] libpayload: Fix a small but aggressive bug in printf()

[Peter Stuge]

On Fri, Apr 25, 2008 at 09:52:13AM -0600, [e-mail] wrote:
> This was causing the returned counter value to be one more then it
> should be when printing a single character.
>
> Signed-off-by: Jordan Crouse 
> Index: libpayload/libc/printf.c
> ===================================================================
> --- libpayload.orig/libc/printf.c	2008-04-24 11:44:17.000000000 -0600
> +++ libpayload/libc/printf.c	2008-04-24 11:45:40.000000000 -0600
> @@ -156,7 +156,7 @@
>    		++counter;
>    }
> return ++counter;
> return counter;

What about the cases when printf() does not print more than one
character? Does the simple fix cover all cases?

//Peter

libpayload: Fix a small but aggressive bug in printf()

[Jordan Crouse]

On 25/04/08 20:57 +0200, Peter Stuge wrote:
> On Fri, Apr 25, 2008 at 09:52:13AM -0600, [e-mail] wrote:
> > This was causing the returned counter value to be one more then it
> > should be when printing a single character.
> >
> > Signed-off-by: Jordan Crouse 
> > Index: libpayload/libc/printf.c
> > ===================================================================
> > --- libpayload.orig/libc/printf.c	2008-04-24 11:44:17.000000000 -0600
> > +++ libpayload/libc/printf.c	2008-04-24 11:45:40.000000000 -0600
> > @@ -156,7 +156,7 @@
> >    		++counter;
> >    }
> > return ++counter;
> > return counter;
>
> What about the cases when printf() does not print more than one
> character? Does the simple fix cover all cases?

Yeah - it should.  The counter is already incremented previously in
the function, just above the context for this patch.

Jordan

libpayload: Fix a small but aggressive bug in printf()

[Ward Vandewege]

On Fri, Apr 25, 2008 at 03:59:51PM -0600, Jordan Crouse wrote:
> On 25/04/08 20:57 +0200, Peter Stuge wrote:
> > On Fri, Apr 25, 2008 at 09:52:13AM -0600, [e-mail] wrote:
> > > This was causing the returned counter value to be one more then it
> > > should be when printing a single character.
> > >
> > > Signed-off-by: Jordan Crouse 
> > > Index: libpayload/libc/printf.c
> > > ===================================================================
> > > --- libpayload.orig/libc/printf.c	2008-04-24 11:44:17.000000000 -0600
> > > +++ libpayload/libc/printf.c	2008-04-24 11:45:40.000000000 -0600
> > > @@ -156,7 +156,7 @@
> > >    		++counter;
> > >    }
> > > return ++counter;
> > > return counter;
> >
> > What about the cases when printf() does not print more than one
> > character? Does the simple fix cover all cases?
>
> Yeah - it should.  The counter is already incremented previously in
> the function, just above the context for this patch.

Acked-by: Ward Vandewege

Thanks,
Ward.

libpayload: Fix a small but aggressive bug in printf()

[Jordan Crouse]

On 25/04/08 19:05 -0400, Ward Vandewege wrote:
> On Fri, Apr 25, 2008 at 03:59:51PM -0600, Jordan Crouse wrote:
> > On 25/04/08 20:57 +0200, Peter Stuge wrote:
> > > On Fri, Apr 25, 2008 at 09:52:13AM -0600, [e-mail] wrote:
> > > > This was causing the returned counter value to be one more then it
> > > > should be when printing a single character.
> > > >
> > > > Signed-off-by: Jordan Crouse 
> > > > Index: libpayload/libc/printf.c
> > > > ===================================================================
> > > > --- libpayload.orig/libc/printf.c	2008-04-24 11:44:17.000000000 -0600
> > > > +++ libpayload/libc/printf.c	2008-04-24 11:45:40.000000000 -0600
> > > > @@ -156,7 +156,7 @@
> > > >    		++counter;
> > > >    }
> > > > return ++counter;
> > > > return counter;
> > >
> > > What about the cases when printf() does not print more than one
> > > character? Does the simple fix cover all cases?
> >
> > Yeah - it should.  The counter is already incremented previously in
> > the function, just above the context for this patch.
>
> Acked-by: Ward Vandewege

r3271. Thanks.