This is my attempt to extend patch 1633670. This patch is created
against 5.7.2 plus patch 1633670.
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 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.
Log in to post a comment.