[coreboot-gerrit] Patch set updated for coreboot: fad89ac libpayload: video: Check for 'console' pointer before dereferencing it

[Marc Jones]

Marc Jones ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/7910

-gerrit

commit fad89acc9d0ffd7b98b8c47bbfbdd28cfc48e8bb
Author: Julius Werner 
Date:   Tue May 20 17:56:25 2014 -0700

libpayload: video: Check for 'console' pointer before dereferencing it

Seems that the 'if (cursor_enabled)' check in
    video_console_fixup_cursor() that was removed in commit 1f880bca0 really
    meant to check for 'if (console)'. Looks like the whole video console
    driver is built extra robust to not fail no matter how screwed up the
    console is, so let's add this missing check here as well. Also fixed up
    a few other missing 'if (!console)' checks while I'm at it.

However, what payloads should really be doing is check the return value
    of video_(console_)init() and not call the other video functions if that
    failed. This also adapts video_console_init() to correctly pass through
    the return value for that purpose (something that seems to have been
    overlooked in the dd9e4e58 refactoring).

BUG=chrome-os-partner:28494
    TEST=None. I don't know what Dave did to trigger this in the first
    place, but it's pretty straight-forward.

Original-Change-Id: I1b9f09d49dc70dacf20621b19e081c754d4814f7
    Original-Signed-off-by: Julius Werner 
    Original-Reviewed-on: https://chromium-review.googlesource.com/200688
    Original-Reviewed-by: David Hendricks 
    (cherry picked from commit 3f01d1dc0974774f0b3ba5fc4e069978f266f2fc)
    Signed-off-by: Marc Jones

Change-Id: I98c1d8360539b457e6df07cbcf799acaf6c4631b
---
 payloads/libpayload/drivers/video/video.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/payloads/libpayload/drivers/video/video.c b/payloads/libpayload/drivers/video/video.c
index 318548b..5f76a30 100644
--- a/payloads/libpayload/drivers/video/video.c
+++ b/payloads/libpayload/drivers/video/video.c
@@ -73,6 +73,9 @@ void video_get_rows_cols(unsigned int *rows, unsigned int *cols)

static void video_console_fixup_cursor(void)
 {
+	if (!console)
+		return;
+
    if (cursorx < 0)
    	cursorx = 0;

@@ -89,7 +92,7 @@ static void video_console_fixup_cursor(void)
    	cursory--;
    }

-	if (console && console->set_cursor)
+	if (console->set_cursor)
    	console->set_cursor(cursorx, cursory);
 }

@@ -119,6 +122,9 @@ void video_console_putc(u8 row, u8 col, unsigned int ch)

void video_console_putchar(unsigned int ch)
 {
+	if (!console)
+		return;
+
    /* replace black-on-black with light-gray-on-black.
     * do it here, instead of in libc/console.c
     */
@@ -145,16 +151,11 @@ void video_console_putchar(unsigned int ch)
    	break;

case '\t':
-		while(cursorx % 8 && cursorx < console->columns) {
-			if (console)
-				console->putc(cursory, cursorx, (ch & 0xFF00) | ' ');
-
-			cursorx++;
-		}
+		while(cursorx % 8 && cursorx < console->columns)
+			console->putc(cursory, cursorx++, (ch & 0xFF00) | ' ');
    	break;
    default:
-		if (console)
-			console->putc(cursory, cursorx++, ch);
+		console->putc(cursory, cursorx++, ch);
    	break;
    }

@@ -167,7 +168,7 @@ void video_console_get_cursor(unsigned int *x, unsigned int *y, unsigned int *en
    *y=0;
    *en=0;

-	if (console->get_cursor)
+	if (console && console->get_cursor)
    	console->get_cursor(x, y, en);

*x = cursorx;
@@ -214,9 +215,10 @@ int video_init(void)

int video_console_init(void)
 {
-	video_init();
-	if (console)
-		console_add_output_driver(&cons);
+	int ret = video_init();
+	if (ret)
+		return ret;
+	console_add_output_driver(&cons);
    return 0;
 }