[Net-snmp-patches] [ net-snmp-Patches-3580458 ] snmpd crashes/hangs when subagent disconnects during GetNext

[SourceForge.net <no...@so...>]

Patches item #3580458, was opened at 2012-10-26 01:42
Message generated for change (Tracker Item Submitted) made by jcervenka
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312694&aid=3580458&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: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: jcervenka (jcervenka)
Assigned to: Nobody/Anonymous (nobody)
Summary: snmpd crashes/hangs when subagent disconnects during GetNext

Initial Comment:
This is my attempt to extend patch 1633670. This patch is created
against 5.7.2 plus patch 1633670.

Problems addressed:

1) Inifinite loop in delegated requests removal

In my opionion, the for-loop iteration on asp->requests in
netsnmp_remove_delegated_requests_for_session() is incorrect, as it
may end up flagging only a subset of asp's requests (depending on how
many different subtrees (and hence treecaches) this asp's requests
OIDs fall in). 

The loop situation occurs when the asp->requests[0] request has
already been answered (and is not part of any treecache), whereas some
other requests on this asp still have the delegated flag on
(reproducible by GetNext requests which contain both a variable
pointing to subtree belonging to the closing session, and another one
pointing to a range preceding that very subtree (but not fulfilled by
any preceding handler).

I think this for-loop should walk all asp->vbcount items on the
asp->requests array instead.

2) Crashes on dangling subtree pointers after AgentX Close PDU

The same crash patterns as those described in patch 1633670 appear
when a session gets closed via an explicit Close PDU from a subagent.
So, I think we need to go through the remove delegated requests loop
also when a valid sessid has been passed to close_agentx_session().

Adding this change caused double frees on delegated caches in
agentx_got_response() ('response is too late') -- apparently caused by
responses to retries, and also by callback from snmp_sess_close(). I
think this can be fixed by returning 1 from agentx_got_response() on
this occasion, thus letting _sess_process_packet() delete the request
from the session pending requests list.


Tested with 5.7.2 w/ patch 1633670, on x86_64 Linux, RHEL 5.5.

I tested the patch also with the subagent and traffic generating
script attached to bug 3565004 -- and it worked for me.


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

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