[OpenBIOS] [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 <mark.cave-ayland at ilande.co.uk>

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;
 }