Improve stack usage in libnvram environment variable code

The Forth-to-C wrapper code in libnvram.code uses a temporary
buffer of 255 bytes via the STRING_INIT macro for each Forth string
that has to be converted to a C string. However, using such big
arrays in the wrapper code is a bad idea: Each of the buffers
is put into the stack frame of the engine() function (from paflof.c)!
That means the 7 strings from libnvram.code increase the stack
usage of engine() by 7 * 255 = 1785 bytes! This can cause stack
overflows since engine() can be called recursively, e.g. via the
forth_eval() macro.

To fix this issue in the functions from envvar.c, we can simply
pass the Forth strings directly to the functions by adding the
string length as additional function parameter, since the functions
in envvar.c don't really depend on NUL-terminated strings.

And while we're at it (i.e. we touch the function prototypes here
anyway), also rename the functions to have a proper "nvram_" prefix,
so we clearly mark them as part of libnvram instead of cluttering
the global name space with rather trivial function names.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

3 files changed, 48 insertions, 40 deletions

diff --git a/lib/libnvram/envvar.c b/lib/libnvram/envvar.c
index 87aaf27..ee943fc 100644
--- a/lib/libnvram/envvar.c
+++ b/lib/libnvram/envvar.c
@@ -17,7 +17,7 @@
 #include "nvram.h"
 
 /* returns the offset of the first byte after the searched envvar */
-static int get_past_env_pos(partition_t part, char *envvar)
+static int get_past_env_pos(partition_t part, char *envvar, int evlen)
 {
 	int offset, len;
 	static char temp[256];
@@ -32,7 +32,7 @@ static int get_past_env_pos(partition_t part, char *envvar)
 		while((data=nvram_read_byte(offset++)) && len < 256) {
 			temp[len++]=data;
 		}
-		if (!strncmp(envvar, temp, strlen(envvar))) {
+		if (!strncmp(envvar, temp, evlen)) {
 			return offset;
 		}
 	} while (len);
@@ -43,16 +43,16 @@ static int get_past_env_pos(partition_t part, char *envvar)
 /**
  * @param partition name of the envvar partition
  * @param envvar name of the environment variable
+ * @param evlen string length of the envvar parameter
  * @return pointer to temporary string containing the value of envvar
  */
-
-char *get_env(partition_t part, char *envvar)
+char *nvram_get_env(partition_t part, char *envvar, int evlen)
 {
 	static char temp[256+1];
 	int len, offset;
 	uint8_t data;
 
-	DEBUG("get_env %s... ", envvar);
+	DEBUG("nvram_get_env %p... ", envvar);
 	if(!part.addr) {
 		/* ERROR: No environment variable partition */
 		DEBUG("invalid partition.\n");
@@ -68,7 +68,7 @@ char *get_env(partition_t part, char *envvar)
 		}
 		temp[len]=0;
 
-		if (!strncmp(envvar, temp, strlen(envvar))) {
+		if (!strncmp(envvar, temp, evlen)) {
 			int pos=0;
 			while (temp[pos]!='=' && pos < len) pos++;
 			// DEBUG("value='%s'\n", temp+pos+1); 
@@ -100,7 +100,7 @@ static int find_last_envvar(partition_t part)
 	return -1;
 }
 
-int add_env(partition_t part, char *envvar, char *value)
+int nvram_add_env(partition_t part, char *envvar, int evlen, char *value, int vallen)
 {
 	int freespace, last, len, offset;
 	unsigned int i;
@@ -112,7 +112,7 @@ int add_env(partition_t part, char *envvar, char *value)
 	freespace = part.addr+part.len-last;
 
 	/* how long is the entry we want to write? */
-	len = strlen(envvar) + strlen(value) + 2;
+	len = evlen + vallen + 2;
 
 	if(freespace<len) {
 		// TODO try to increase partition size
@@ -121,18 +121,18 @@ int add_env(partition_t part, char *envvar, char *value)
 
 	offset=last;
 
-	for(i=0; i<strlen(envvar); i++)
+	for (i = 0; i < evlen; i++)
 		nvram_write_byte(offset++, envvar[i]);
 
 	nvram_write_byte(offset++, '=');
 
-	for(i=0; i<strlen(value); i++)
+	for (i = 0; i < vallen; i++)
 		nvram_write_byte(offset++, value[i]);
 
 	return 0;
 }
 
-int del_env(partition_t part, char *envvar)
+int nvram_del_env(partition_t part, char *envvar, int evlen)
 {
 	int last, current, pos, i;
 	char *buffer;
@@ -141,7 +141,7 @@ int del_env(partition_t part, char *envvar)
 		return -1;
 
 	last=find_last_envvar(part);
-	current = pos = get_past_env_pos(part, envvar);
+	current = pos = get_past_env_pos(part, envvar, evlen);
 	
 	// TODO is this really required?
 	/* go back to non-0 value */
@@ -168,25 +168,25 @@ int del_env(partition_t part, char *envvar)
 	return 0;
 }
 
-int set_env(partition_t part, char *envvar, char *value)
+int nvram_set_env(partition_t part, char *envvar, int evlen, char *value, int vallen)
 {
 	char *oldvalue, *buffer;
 	int last, current, buffersize, i;
 
-	DEBUG("set_env %lx[%lx]: %s=%s\n", part.addr, part.len, envvar, value);
+	DEBUG("nvram_set_env %lx[%lx]: %p=>%p\n", part.addr, part.len, envvar, value);
 
 	if(!part.addr)
 		return -1;
 
 	/* Check whether the environment variable exists already */
-	oldvalue = get_env(part, envvar);
+	oldvalue = nvram_get_env(part, envvar, evlen);
 
-	if(oldvalue==NULL)
-		return add_env(part, envvar, value);
+	if (oldvalue == NULL)
+		return nvram_add_env(part, envvar, evlen, value, vallen);
 
 
 	/* The value did not change. So we succeeded! */
-	if(!strncmp(oldvalue, value, strlen(value)+1))
+	if (strlen(oldvalue) == vallen && !strncmp(oldvalue, value, vallen))
 		return 0;
 
 	/* we need to overwrite environment variables, back them up first */
@@ -195,7 +195,7 @@ int set_env(partition_t part, char *envvar, char *value)
 
 	/* allocate a buffer */
 	last=find_last_envvar(part);
-	current=get_past_env_pos(part, envvar);
+	current = get_past_env_pos(part, envvar, evlen);
 	buffersize = last - current;
 	buffer=get_nvram_buffer(buffersize);
 	if(!buffer)
@@ -214,7 +214,7 @@ int set_env(partition_t part, char *envvar, char *value)
 	current++;
 
 	/* Write the new value */
-	for(i=0; i<(int)strlen(value); i++) {
+	for(i = 0; i < vallen; i++) {
 		nvram_write_byte(current++, value[i]);
 	}
 	
diff --git a/lib/libnvram/libnvram.code b/lib/libnvram/libnvram.code
index 427adc2..dd2346e 100644
--- a/lib/libnvram/libnvram.code
+++ b/lib/libnvram/libnvram.code
@@ -158,15 +158,17 @@ MIRP
 
 // ( part.start part.len name.addr name.len -- var.addr var.len TRUE | false )
 PRIM(internal_X2d_get_X2d_env)
-	STRING_INIT(name)
+	char *name;
+	int namelen;
 	partition_t part;
 	char *val;
 
-	STRING_FROM_STACK(name)
+	namelen = TOS.u; POP;
+	name = TOS.a; POP;
 	part.len = TOS.u; POP;
 	part.addr = TOS.u; POP;
 
-	val=get_env(part, name);
+	val = nvram_get_env(part, name, namelen);
 	if(val) {
 		PUSH; TOS.a = val;
 		PUSH; TOS.u = strlen(val);
@@ -178,17 +180,19 @@ MIRP
 
 // ( part.start part.len name.addr name.len val.addr val.len -- FALSE|TRUE)
 PRIM(internal_X2d_add_X2d_env)
-	STRING_INIT(name)
-	STRING_INIT(value)
+	char *name, *val;
+	int namelen, vallen;
 	partition_t part;
 	int ret;
 
-	STRING_FROM_STACK(value)
-	STRING_FROM_STACK(name)
+	vallen = TOS.u; POP;
+	val = TOS.a; POP;
+	namelen = TOS.u; POP;
+	name = TOS.a; POP;
 	part.len = TOS.u; POP;
 	part.addr = TOS.u; POP;
 
-	ret=add_env(part, name, value);
+	ret = nvram_add_env(part, name, namelen, val, vallen);
 	if(ret) {
 		PUSH; TOS.u = -1; // TRUE
 	} else {
@@ -198,15 +202,17 @@ MIRP
 
 // ( part.addr part.len name.addr name.len -- FALSE|TRUE)
 PRIM(internal_X2d_del_X2d_env)
-	STRING_INIT(name)
+	char *name;
+	int namelen;
 	partition_t part;
 	int ret;
 
-	STRING_FROM_STACK(name);
+	namelen = TOS.u; POP;
+	name = TOS.a; POP;
 	part.len = TOS.u; POP;
 	part.addr = TOS.u; POP;
 
-	ret=del_env(part, name);
+	ret = nvram_del_env(part, name, namelen);
 	if(ret) {
 		PUSH; TOS.u = -1; // TRUE
 	} else {
@@ -217,17 +223,19 @@ MIRP
 
 // internal-set-env ( part.addr part.len name.addr name.len val.addr val.len -- FALSE|TRUE)
 PRIM(internal_X2d_set_X2d_env)
-	STRING_INIT(name)
-	STRING_INIT(value)
+	char *name, *value;
+	int namelen, valuelen;
 	partition_t part;
 	int ret;
 
-	STRING_FROM_STACK(value)
-	STRING_FROM_STACK(name)
+	valuelen = TOS.u; POP;
+	value = TOS.a; POP;
+	namelen = TOS.u; POP;
+	name = TOS.a; POP;
 	part.len = TOS.u; POP;
 	part.addr = TOS.u; POP;
 
-	ret=set_env(part, name, value);
+	ret = nvram_set_env(part, name, namelen, value, valuelen);
 	if(ret) {
 		PUSH; TOS.u = -1; // TRUE
 	} else {
diff --git a/lib/libnvram/nvram.h b/lib/libnvram/nvram.h
index b4964f6..3fdba98 100644
--- a/lib/libnvram/nvram.h
+++ b/lib/libnvram/nvram.h
@@ -66,9 +66,9 @@ void nvram_init(uint32_t store_token, uint32_t fetch_token,
 unsigned int get_nvram_size(void);
 
 /* envvar.c */
-char *get_env(partition_t part, char *envvar);
-int add_env(partition_t part, char *envvar, char *value);
-int del_env(partition_t part, char *envvar);
-int set_env(partition_t part, char *envvar, char *value);
+char *nvram_get_env(partition_t part, char *envvar, int evlen);
+int nvram_add_env(partition_t part, char *envvar, int evlen, char *value, int vallen);
+int nvram_del_env(partition_t part, char *envvar, int evlen);
+int nvram_set_env(partition_t part, char *envvar, int evlen, char *val, int vlen);
 
 #endif