[commit] r1250 - trunk/openbios-devel/libopenbios

[repository service]

Author: mcayland
Date: Mon Jan 13 10:46:43 2014
New Revision: 1250
URL: http://tracker.coreboot.org/trac/openbios/changeset/1250

Log:
client.c: stop clients from being able to invoke stack underflow

Clients that incorrectly requested too many return arguments for a client
interface call could cause a Forth stack underflow (this was discovered due
to a bug in NetBSD SPARC64).

Ensure that we can never POP() beyond the original position of the stack from
before the client interface call.

Signed-off-by: Mark Cave-Ayland

Modified:
   trunk/openbios-devel/libopenbios/client.c

Modified: trunk/openbios-devel/libopenbios/client.c
==============================================================================
--- trunk/openbios-devel/libopenbios/client.c	Mon Jan 13 10:46:41 2014	(r1249)
+++ trunk/openbios-devel/libopenbios/client.c	Mon Jan 13 10:46:43 2014	(r1250)
@@ -244,41 +244,43 @@

/* call-method, interpret */
 static int
-handle_calls( prom_args_t *pb )
+handle_calls(prom_args_t *pb)
 {
-	int i, dstacksave = dstackcnt;
-	prom_uarg_t val;
+	int i, j, dstacksave;
+	ucell val;

#ifdef DEBUG_CIF
    printk("%s %s ([" FMT_prom_arg "] -- [" FMT_prom_arg "])\n",
    	get_service(pb), arg2pointer(pb->args[0]), pb->nargs, pb->nret);
 #endif

+	dstacksave = dstackcnt;
    for (i = pb->nargs - 1; i >= 0; i--)
    	PUSH(pb->args[i]);

push_str(get_service(pb));
    fword("client-call-iface");

-	/* Drop the return code from client-call-iface (status is handled by the 
-	   catch result which is the first parameter below) */
-	POP();
-
-	for (i = 0; i < pb->nret; i++) {
-                val = POP();
-		pb->args[pb->nargs + i] = val;
+	/* If the catch result is non-zero, restore stack and exit */
+	val = POP();
+	if (val) {
+		dstackcnt = dstacksave;
+		return 0;
+	}

-		/* don't pop args if an exception occured */
-		if (!i && val)
-			break;
+	j = dstackcnt;
+	for (i = 0; i < pb->nret; i++, j--) {
+                if (dstackcnt > dstacksave) {
+			pb->args[pb->nargs + i] = POP();
+		}
    }

#ifdef DEBUG_CIF
    /* useful for debug but not necessarily an error */
-	if (i != pb->nret || dstackcnt != dstacksave) {
+	if (j != dstacksave) {
    	printk("%s '%s': possible argument error (" FMT_prom_arg "--" FMT_prom_arg ") got %d\n",
    		get_service(pb), arg2pointer(pb->args[0]),
-			pb->nargs - 2, pb->nret, i);
+			pb->nargs - 2, pb->nret, j - dstacksave);
    }

printk("handle_calls return:");
@@ -293,10 +295,11 @@
 }

int
-of_client_interface( int *params )
+of_client_interface(int *params)
 {
    prom_args_t *pb = (prom_args_t*)params;
-	int val, i, dstacksave;
+	ucell val;
+	int i, j, dstacksave;

if (pb->nargs < 0 || pb->nret < 0 ||
             pb->nargs + pb->nret > PROM_MAX_ARGS)
@@ -317,34 +320,41 @@
    push_str(get_service(pb));
    fword("client-iface");

-	if (val=POP()) {
-		dstackcnt = dstacksave;
-		if (val == -1)
+	val = POP();
+	if (val) {
+		if (val == -1) {
    		printk("Unimplemented service %s ([" FMT_prom_arg "] -- [" FMT_prom_arg "])\n",
-			       get_service(pb), pb->nargs, pb->nret);
+				get_service(pb), pb->nargs, pb->nret);
+		} else {
 #ifdef DEBUG_CIF
-		else
-			printk("ERROR!\n");
+			printk("Error calling client interface: " FMT_ucellx "\n", val);
 #endif
+		}
+
+		dstackcnt = dstacksave;
    	return -1;
    }

-	for (i = 0; i < pb->nret ; i++)
-		pb->args[pb->nargs + i] = POP();
+	j = dstackcnt;
+	for (i = 0; i < pb->nret; i++, j--) {
+		if (dstackcnt > dstacksave) {
+			pb->args[pb->nargs + i] = POP();
+		}
+	}

-	if (dstackcnt != dstacksave) {
 #ifdef DEBUG_CIF
+	if (j != dstacksave) {
    	printk("service %s: possible argument error (%d %d)\n",
-		       get_service(pb), i, dstackcnt - dstacksave);
-#endif
+		       get_service(pb), i, j - dstacksave);
+
    	/* Some clients request less parameters than the CIF method
    	returns, e.g. getprop with OpenSolaris. Hence we drop any
-		stack parameters after issuing a warning above */
-		dstackcnt = dstacksave;
+		stack parameters on exit after issuing a warning above */
    }

-#ifdef DEBUG_CIF
    dump_return(pb);
 #endif
+
+	dstackcnt = dstacksave;
    return 0;
 }