On Thu, Jan 05, 2017 at 02:21:42PM +0000, Moreno Floris wrote:
> Hello,
>
> I'm running net-snmp 5.7.3 on an embedded platform with 16KB of stack for a
> single task and I ran into a nasty stack overflow while running the coding
> tutorial example.
>
> After some investigation I found out that _get_realloc_symbol() in mib.c is
> called recursively 8 times and contains the following declaration:
>
> u_char buffer[1024];
>
> This is bad for stack consumption, I renamed it "my_buffer" and declared it
> externally (because I don't have the 16KB limitation for global stack).
>
> Inside the function I replaced the declaration with
>
> memset(&my_buffer,0,sizeof(my_buffer));
>
> to make sure it's clean before each use.
>
> I chose a global static over a local dynamic because with all the gotos I
> wouldn't know where to put the free()
>
> After this simple modification, the maximum stack consumption is 78%, still
> high but well within the limit (and my program is not crashing anymore).
>
> Please let me know your opinion on this.
I am not happy about more global variables so I have this alternate patch where
I move the variable 'buffer' to another function that I call from
_get_realloc_symbol when it is needed.
This has the advantage of no global variables (yay - thread safety) and only
creating the variable when it is used.
Does this version also wok for you?
/MF
Subject: [PATCH] Reduce stack usage
Extract the outsized variable 'buffer' from the recursive _get_realloc_symbol
function into a separate function in order to save stack space
---
snmplib/mib.c | 46 ++++++++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/snmplib/mib.c b/snmplib/mib.c
index a33f595..81a4693 100644
--- a/snmplib/mib.c
+++ b/snmplib/mib.c
@@ -4228,6 +4228,31 @@ _oid_finish_printing(const oid * objid, size_t objidlen,
}
#ifndef NETSNMP_DISABLE_MIB_LOADING
+static void
+_get_realloc_symbol_octet_string(size_t numids, const oid * objid,
+ u_char ** buf, size_t * buf_len,
+ size_t * out_len, int allow_realloc,
+ int *buf_overflow, struct tree* tp)
+{
+ netsnmp_variable_list var = { 0 };
+ u_char buffer[1024];
+ size_t i;
+
+ for (i = 0; i < numids; i++)
+ buffer[i] = (u_char) objid[i];
+ var.type = ASN_OCTET_STR;
+ var.val.string = buffer;
+ var.val_len = numids;
+ if (!*buf_overflow) {
+ if (!sprint_realloc_octet_string(buf, buf_len, out_len,
+ allow_realloc, &var,
+ NULL, tp->hint,
+ NULL)) {
+ *buf_overflow = 1;
+ }
+ }
+}
+
static struct tree *
_get_realloc_symbol(const oid * objid, size_t objidlen,
struct tree *subtree,
@@ -4351,11 +4376,6 @@ _get_realloc_symbol(const oid * objid, size_t objidlen,
switch (tp->type) {
case TYPE_OCTETSTR:
if (extended_index && tp->hint) {
- netsnmp_variable_list var;
- u_char buffer[1024];
- int i;
-
- memset(&var, 0, sizeof var);
if (in_dices->isimplied) {
numids = objidlen;
if (numids > objidlen)
@@ -4374,19 +4394,9 @@ _get_realloc_symbol(const oid * objid, size_t objidlen,
}
if (numids > objidlen)
goto finish_it;
- for (i = 0; i < (int) numids; i++)
- buffer[i] = (u_char) objid[i];
- var.type = ASN_OCTET_STR;
- var.val.string = buffer;
- var.val_len = numids;
- if (!*buf_overflow) {
- if (!sprint_realloc_octet_string(buf, buf_len, out_len,
- allow_realloc, &var,
- NULL, tp->hint,
- NULL)) {
- *buf_overflow = 1;
- }
- }
+ _get_realloc_symbol_octet_string(numids, objid, buf, buf_len,
+ out_len, allow_realloc,
+ buf_overflow, tp);
} else if (in_dices->isimplied) {
numids = objidlen;
if (numids > objidlen)
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
