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