Bug report and Rqeust for confirm

Won-Sik Kim Fri, 23 Jul 2004 05:47:24 -0700

Hi all,

I use net-snmp-5.1.1 on Linux with agentX for developing multi device system.

I suffered from following three problems.

1. memory leak in master agent if manager sends many getbulk requests at once.

2. crash of master agent happens time to time if manager sends many getnext requests

3. some times, response from master agent slows down, but, if I wait, if get back to normal status.

I already reported 1st problem. But, my question was too vague to get response.

I debugged and traced net-snmp for a week, and find out which source code causes 2nd problem(crash)

The bug was in function netsnmp_check_outstanding_agent_requests(). (in file snmp_agent.c)

I only remove 1 line from the function. (line 24 in followed source code) And, problem solved...

There was a possibility of referencing to freed netsnmp_agent_session structure.

My explanations :

prev_asp is last asp (prev_asp = asp) - line 9 in followed source code

But, if you trace function check_delayed_request(asp) - line 40 in followed source code
asp can be freed. (of course, pointer itself is not NULL)

This means, code "prev_asp->next = " can crash master agent.

asp is freed by this way. (trace sequence)

check_delayed_request(asp) - line 40 in followed source code

netsnmp_wrap_up_request(asp)

netsnmp_remove_and_free_agent_snmp_session(asp)

free_agent_snmp_session(asp)

SNMP_FREE(asp);

As you know, SNMP_FREE(asp) does "asp = NULL", but, asp is parsed as pointer, so this NULLing

can not help anything to code "if (prev_asp != NULL)" - line 16 in followed source code

Finally, "prev_asp->next = asp->next" causes crash. (not always though)

I was not sure what "prev_asp->next = asp->next" really does. So, for test, I just removed it first.

And then, amazingly, this removing solved not only 2nd problem(crash), but also 1st(memory leak),

and 3rd(slow down) !! And, there is no problem in get, getnext, getbulk and set !!

After this change, no more crash, no more memory leak and no more slow down.

Here are my requests.

1. Can somebody confirm my change ?

I just remove one line, but, I am wonder if this removing causes other problem... (I can not find yet though)

2. If I am right, this change must be included in next release(5.1.2)

I will wait for answer.

Best regards,

Won-Sik Kim

  1 void
  2 netsnmp_check_outstanding_agent_requests(void)
  3 {

4 netsnmp_agent_session *asp, *prev_asp = NULL, *next_asp = NULL;

  6 /*
  7    * deal with delegated requests
  8    */
  9 for (asp = agent_delegated_list; asp; prev_asp = asp, asp = next_asp) {
10          next_asp = asp->next;   /* save in case we clean up asp */
11       if (!netsnmp_check_for_delegated(asp)) {

13           /*
14             * we're done with this one, remove from queue
15             */
16           if (prev_asp != NULL)
17            {
18 #if 0   /* by wonsikkim on 2004-07-22 to solve crash on master agent problem    */
19         /* Funny thing is by this change, other two major problems also solved */
20         /* So, this change fix following three bugs at once.                    */
21         /* 1. master agent crash during processing a lot of getnext requests    */
22         /* 2. master agent slow down sometimes during processing requests       */
23         /* 3. memory leak if process a lot of getbulk requests                  */
24                 prev_asp->next = asp->next;
25 #endif
26             }
27             else
28             {
29                 agent_delegated_list = asp->next;
30             }

32             /*
33              * check request status
34              */
35             netsnmp_check_all_requests_status(asp, 0);

37             /*
38              * continue processing or finish up
39              */
40             check_delayed_request(asp);
41         }
42     }

Bug report and Rqeust for confirm

Reply via email to