[SLOF] [PATCH 3/5] Improve stack usage in libnvram
Thomas Huth
thuth at redhat.com
Thu Nov 26 06:58:17 AEDT 2015
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 into 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, introduce proper wrapper functions for the libnvram
code that has to deal with Forth strings, and handle the temporary
buffers there instead. This way the stack frame of engine() is not
overloaded by the temporary buffers anymore.
Signed-off-by: Thomas Huth <thuth at redhat.com>
---
lib/libnvram/envvar.c | 43 ++++++++++++++++++++++++++++++++++
lib/libnvram/libnvram.code | 58 +++++++++++++++++++++-------------------------
lib/libnvram/nvram.c | 10 ++++++++
lib/libnvram/nvram.h | 5 ++++
4 files changed, 84 insertions(+), 32 deletions(-)
diff --git a/lib/libnvram/envvar.c b/lib/libnvram/envvar.c
index 87aaf27..1ac0a28 100644
--- a/lib/libnvram/envvar.c
+++ b/lib/libnvram/envvar.c
@@ -80,6 +80,16 @@ char *get_env(partition_t part, char *envvar)
return NULL;
}
+char *get_env_fs(partition_t part, char *envvar, int evlen)
+{
+ char buf[evlen + 1];
+
+ memcpy(buf, envvar, evlen);
+ buf[evlen] = 0;
+
+ return get_env(part, buf);
+}
+
static int find_last_envvar(partition_t part)
{
uint8_t last, current;
@@ -132,6 +142,18 @@ int add_env(partition_t part, char *envvar, char *value)
return 0;
}
+int add_env_fs(partition_t part, char *envvar, int evlen, char *val, int vallen)
+{
+ char evbuf[evlen + 1], valbuf[vallen + 1];
+
+ memcpy(evbuf, envvar, evlen);
+ evbuf[evlen] = 0;
+ memcpy(valbuf, val, vallen);
+ valbuf[vallen] = 0;
+
+ return add_env(part, evbuf, valbuf);
+}
+
int del_env(partition_t part, char *envvar)
{
int last, current, pos, i;
@@ -168,6 +190,16 @@ int del_env(partition_t part, char *envvar)
return 0;
}
+int del_env_fs(partition_t part, char *envvar, int evlen)
+{
+ char buf[evlen + 1];
+
+ memcpy(buf, envvar, evlen);
+ buf[evlen] = 0;
+
+ return del_env(part, buf);
+}
+
int set_env(partition_t part, char *envvar, char *value)
{
char *oldvalue, *buffer;
@@ -241,3 +273,14 @@ int set_env(partition_t part, char *envvar, char *value)
return 0; /* success */
}
+int set_env_fs(partition_t part, char *envvar, int evlen, char *val, int vallen)
+{
+ char evbuf[evlen + 1], valbuf[vallen + 1];
+
+ memcpy(evbuf, envvar, evlen);
+ evbuf[evlen] = 0;
+ memcpy(valbuf, val, vallen);
+ valbuf[vallen] = 0;
+
+ return set_env(part, evbuf, valbuf);
+}
diff --git a/lib/libnvram/libnvram.code b/lib/libnvram/libnvram.code
index 723941d..6d6d06a 100644
--- a/lib/libnvram/libnvram.code
+++ b/lib/libnvram/libnvram.code
@@ -11,17 +11,6 @@
*****************************************************************************/
#include <nvram.h>
-#define STRING_INIT(str) \
- char str[255]; \
- char * str##_address; \
- int str##_length;
-
-#define STRING_FROM_STACK(str) \
- str##_length = TOS.u; POP; \
- str##_address = TOS.a; POP; \
- memcpy(str, str##_address, str##_length); \
- memset(str + str##_length, 0, 255 - str##_length);
-
PRIM(nvram_X2d_c_X40)
unsigned int offset = TOS.u;
TOS.u=nvram_read_byte(offset);
@@ -80,21 +69,18 @@ MIRP
/* get-named-nvram-partition ( name.addr name.len -- addr len FAILED? ) */
PRIM(get_X2d_named_X2d_nvram_X2d_partition)
- STRING_INIT(name)
partition_t partition;
+ int namelen = TOS.n; POP;
- STRING_FROM_STACK(name)
- partition = get_partition(-1, name);
+ partition = get_partition_fs(-1, TOS.a, namelen);
if(partition.len && partition.len != -1) {
- PUSH;
TOS.u = partition.addr;
PUSH;
TOS.u = partition.len;
PUSH;
TOS.u = 0; // FALSE
} else {
- PUSH;
TOS.u = -1; // TRUE
}
MIRP
@@ -165,15 +151,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 = get_env_fs(part, name, namelen);
if(val) {
PUSH; TOS.a = val;
PUSH; TOS.u = strlen(val);
@@ -185,17 +173,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 = add_env_fs(part, name, namelen, val, vallen);
if(ret) {
PUSH; TOS.u = -1; // TRUE
} else {
@@ -205,15 +195,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 = del_env_fs(part, name, namelen);
if(ret) {
PUSH; TOS.u = -1; // TRUE
} else {
@@ -224,17 +216,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 = set_env_fs(part, name, namelen, value, valuelen);
if(ret) {
PUSH; TOS.u = -1; // TRUE
} else {
diff --git a/lib/libnvram/nvram.c b/lib/libnvram/nvram.c
index 5c11376..12251e3 100644
--- a/lib/libnvram/nvram.c
+++ b/lib/libnvram/nvram.c
@@ -358,6 +358,16 @@ partition_t get_partition(unsigned int type, char *name)
return ret;
}
+partition_t get_partition_fs(unsigned int type, char *name, int namelen)
+{
+ char buf[namelen + 1];
+
+ memcpy(buf, name, namelen);
+ buf[namelen] = 0;
+
+ return get_partition(type, buf);
+}
+
void erase_nvram(int offset, int len)
{
int i;
diff --git a/lib/libnvram/nvram.h b/lib/libnvram/nvram.h
index fa6bdd4..a1f3e67 100644
--- a/lib/libnvram/nvram.h
+++ b/lib/libnvram/nvram.h
@@ -51,6 +51,7 @@ char *get_nvram_buffer(int len);
void free_nvram_buffer(char *buffer);
int nvramlog_printf(const char* fmt, ...);
partition_t get_partition(unsigned int type, char *name);
+partition_t get_partition_fs(unsigned int type, char *name, int namelen);
void erase_nvram(int offset, int len);
int wipe_partition(partition_t partition, int header_only);
partition_t new_nvram_partition(int type, char *name, int len);
@@ -66,8 +67,12 @@ unsigned int get_nvram_size(void);
/* envvar.c */
char *get_env(partition_t part, char *envvar);
+char *get_env_fs(partition_t part, char *envvar, int evnamelen);
int add_env(partition_t part, char *envvar, char *value);
+int add_env_fs(partition_t part, char *envvar, int evlen, char *val, int vlen);
int del_env(partition_t part, char *envvar);
+int del_env_fs(partition_t part, char *envvar, int evlen);
int set_env(partition_t part, char *envvar, char *value);
+int set_env_fs(partition_t part, char *envvar, int evlen, char *val, int vlen);
#endif
--
1.8.3.1
More information about the SLOF
mailing list