[prev in list] [next in list] [prev in thread] [next in thread] 

List:       net-snmp-bugs
Subject:    [ net-snmp-Bugs-1413728 ] get/getnext with multiple varbind against table crash.
From:       "SourceForge.net" <noreply () sourceforge ! net>
Date:       2006-01-25 13:21:12
Message-ID: E1F1kaG-0001zP-Je () sc8-sf-web2 ! sourceforge ! net
[Download RAW message or body]

Bugs item #1413728, was opened at 2006-01-24 15:32
Message generated for change (Comment added) made by hmeuleman
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=112694&aid=1413728&group_id=12694

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: agent
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Hante Meuleman (hmeuleman)
Assigned to: Nobody/Anonymous (nobody)
Summary: get/getnext with multiple varbind against table crash.

Initial Comment:
This bug is not straightforward to explain, so please
read carefully. 

Software net-snmp 5.1.3 (but 5.3.0.1 source code holds
same bug)

Operating system independent, but found on linux where
net-snmp was used as agent.

There is no exact command sequence; 

There are two bugs. First bug is in table.c in debug
part of the function table_helper_handler (at end of
this function). When using -D as parameter for agent
(so full debug) then this part will crash if a getnext
with multiple varbinds against a table is performed.
This is caused by:

line514:
for (vb = tbl_req_info->indexes, count = 0;
     vb && count < tbl_info->number_indexes;
     count++, vb = vb->next_variable) {

The problem is with tbl_info->number_indexes. This
should be tbl_req_info->number_indexes;

That was just debug bug, not really a problem. Second
bug is much more serious. The problem occurred in
following situation. A table was defined and
mib2c.iterate.conf  was used to generate template
source code. In this template functions were installed
for get_first_data_point, get_next_data_point and
free_data_context. In the get_first and get_next a
buffer was assigned to *my_data_context (containing row
data). Against this a SNMP getnext was donw where
multiple columns of this table were requested.

Doing the getnext with multiple varbinds causes the
installed function for free_data_context to be called
for every varbind. This is amongst others caused by
helper function netsnmp_table_iterator_helper_handler
in file agent/helpers/table_iterator.c. The problem is
the following: for each row a getfirst or getnext is
done. This will return (possibly depending on
implementation) a buffer which is allocated and later
to be freed through the installed free_data_context
function. However for each varbind the buffer gets
freed. So if you use two varbinds you will get two frees.

To explain with code (all based upon version 5.1.3, but
5.3.0.1 doesnt differ much):

line 448@table_iterator.c:

index_search =
    (iinfo->get_first_data_point) (&callback_loop_context,
				   &callback_data_context,
				   index_search, iinfo);

/* loop over each data point */
while(index_search) {

This is the start of the loop where first_data_point is
called. Then a loop is done and inside (at end of) loop
the get_next_data_point is performed. As can be seen a
callback_data_context can be set in the
installed/called function and this is exactly what we
were doing. This variable (callback_data_context) is
per row. A little lower we find the following code:

line 459@table_iterator.c:

/* compare against each request*/
for(request = requests ; request; request =
request->next) {
    if (request->processed)
	continue;
	
For each request the row_data is processed and here is
the problem. For GET or GETNEXT there are two
possibilities, either put in cache (remember) or free.
However the data can only be freed once. This works
fine as long as only one varbind is used, but as soon
as multiple varbinds are used this will always fail (if
of course) a buffer is allocated in get-first/next and
freed in free_data_context.

In order to overcome this problem a change was made to
table_iterator.c. A check was added for each free to
see if the data_pointer was not in use somewhere else.
Now this is probably not the most efficient way. A more
efficient way would be to keep a reference count for
the datapointer and free based upon the count == 0. I
think that would be bit more efficient, but I choose an
other way and it works for me. 

This will only fix for the freeing of
callback_data_context. A similar problem exists for
callback_loop_context, but I'm not using that, sorry....



----------------------------------------------------------------------

>Comment By: Hante Meuleman (hmeuleman)
Date: 2006-01-25 14:21

Message:
Logged In: YES 
user_id=1434160

The previously submitted fix was not good enough. Under some
circumstances it can create a crash. So a new proper fix was
made. This time using a reference counter as mentioned earlier.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=112694&aid=1413728&group_id=12694


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
Net-snmp-bugs mailing list
Net-snmp-bugs@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-bugs
[prev in list] [next in list] [prev in thread] [next in thread]