[net-snmp/f18] Fixed snmpd crashing when AgentX subagent disconnects in the middle of request processing Resolves:

Jan Šafránek jsafrane at fedoraproject.org
Thu Dec 5 12:21:10 UTC 2013


commit 9103f7f6018b8040a92e9de3cb617acf8a5e001d
Author: Jan Safranek <jsafrane at redhat.com>
Date:   Thu Dec 5 13:21:03 2013 +0100

    Fixed snmpd crashing when AgentX subagent disconnects in the middle of request processing
    Resolves: #1038011 CVE-2012-6151

 net-snmp-5.5-agentx-disconnect-crash.patch |  270 ++++++++++++++++++++++++++++
 net-snmp-5.7-agentx-crash.patch            |   57 ++++++
 net-snmp.spec                              |    8 +
 3 files changed, 335 insertions(+), 0 deletions(-)
---
diff --git a/net-snmp-5.5-agentx-disconnect-crash.patch b/net-snmp-5.5-agentx-disconnect-crash.patch
new file mode 100644
index 0000000..b3b47be
--- /dev/null
+++ b/net-snmp-5.5-agentx-disconnect-crash.patch
@@ -0,0 +1,270 @@
+955511 - net-snmpd crash on time out
+969061 - net-snmpd crash on time out
+1038011 - net-snmp: snmpd crashes/hangs when AgentX subagent times-out
+
+Based on usptream commit 793d596838ff7cb48a73b675d62897c56c9e62df,
+heavily backported to net-snmp-5.5
+
+diff -up net-snmp-5.7.2/agent/mibgroup/agentx/master_admin.c.disconnect-crash net-snmp-5.7.2/agent/mibgroup/agentx/master_admin.c
+--- net-snmp-5.7.2/agent/mibgroup/agentx/master_admin.c.disconnect-crash	2013-07-03 15:26:35.884813210 +0200
++++ net-snmp-5.7.2/agent/mibgroup/agentx/master_admin.c	2013-07-03 15:26:35.908813135 +0200
+@@ -158,6 +158,7 @@ close_agentx_session(netsnmp_session * s
+     for (sp = session->subsession; sp != NULL; sp = sp->next) {
+ 
+         if (sp->sessid == sessid) {
++            netsnmp_remove_delegated_requests_for_session(sp);
+             unregister_mibs_by_session(sp);
+             unregister_index_by_session(sp);
+             unregister_sysORTable_by_session(sp);
+diff -up net-snmp-5.7.2/agent/mibgroup/agentx/master.c.disconnect-crash net-snmp-5.7.2/agent/mibgroup/agentx/master.c
+--- net-snmp-5.7.2/agent/mibgroup/agentx/master.c.disconnect-crash	2013-07-03 15:26:35.000000000 +0200
++++ net-snmp-5.7.2/agent/mibgroup/agentx/master.c	2013-07-03 15:29:00.644362208 +0200
+@@ -222,7 +222,7 @@ agentx_got_response(int operation,
+         /* response is too late, free the cache */
+         if (magic)
+             netsnmp_free_delegated_cache((netsnmp_delegated_cache*) magic);
+-        return 0;
++        return 1;
+     }
+     requests = cache->requests;
+ 
+diff -up net-snmp-5.7.2/agent/snmp_agent.c.disconnect-crash net-snmp-5.7.2/agent/snmp_agent.c
+--- net-snmp-5.7.2/agent/snmp_agent.c.disconnect-crash	2013-07-03 15:26:35.893813182 +0200
++++ net-snmp-5.7.2/agent/snmp_agent.c	2013-07-03 15:28:28.979460861 +0200
+@@ -1446,6 +1446,7 @@ free_agent_snmp_session(netsnmp_agent_se
+         netsnmp_free_cachemap(asp->cache_store);
+         asp->cache_store = NULL;
+     }
++    agent_snmp_session_release_cancelled(asp);
+     SNMP_FREE(asp);
+ }
+ 
+@@ -1457,6 +1458,11 @@ netsnmp_check_for_delegated(netsnmp_agen
+ 
+     if (NULL == asp->treecache)
+         return 0;
++
++    if (agent_snmp_session_is_cancelled(asp)) {
++        printf("request %p cancelled\n", asp);
++        return 0;
++    }
+     
+     for (i = 0; i <= asp->treecache_num; i++) {
+         for (request = asp->treecache[i].requests_begin; request;
+@@ -1535,39 +1541,48 @@ int
+ netsnmp_remove_delegated_requests_for_session(netsnmp_session *sess)
+ {
+     netsnmp_agent_session *asp;
+-    int count = 0;
++    int total_count = 0;
+     
+     for (asp = agent_delegated_list; asp; asp = asp->next) {
+         /*
+          * check each request
+          */
++        int i;
++        int count = 0;
+         netsnmp_request_info *request;
+-        for(request = asp->requests; request; request = request->next) {
+-            /*
+-             * check session
+-             */
+-            netsnmp_assert(NULL!=request->subtree);
+-            if(request->subtree->session != sess)
+-                continue;
++        for (i = 0; i <= asp->treecache_num; i++) {
++            for (request = asp->treecache[i].requests_begin; request;
++                 request = request->next) {
++                /*
++                 * check session
++                 */
++                netsnmp_assert(NULL!=request->subtree);
++                if(request->subtree->session != sess)
++                    continue;
+ 
+-            /*
+-             * matched! mark request as done
+-             */
+-            netsnmp_request_set_error(request, SNMP_ERR_GENERR);
+-            ++count;
++                /*
++                 * matched! mark request as done
++                 */
++                netsnmp_request_set_error(request, SNMP_ERR_GENERR);
++                ++count;
++            }
++        }
++        if (count) {
++            agent_snmp_session_mark_cancelled(asp);
++            total_count += count;
+         }
+     }
+ 
+     /*
+      * if we found any, that request may be finished now
+      */
+-    if(count) {
++    if(total_count) {
+         DEBUGMSGTL(("snmp_agent", "removed %d delegated request(s) for session "
+-                    "%8p\n", count, sess));
+-        netsnmp_check_outstanding_agent_requests();
++                    "%8p\n", total_count, sess));
++        netsnmp_check_delegated_requests();
+     }
+     
+-    return count;
++    return total_count;
+ }
+ 
+ int
+@@ -2739,19 +2754,11 @@ handle_var_requests(netsnmp_agent_sessio
+     return final_status;
+ }
+ 
+-/*
+- * loop through our sessions known delegated sessions and check to see
+- * if they've completed yet. If there are no more delegated sessions,
+- * check for and process any queued requests
+- */
+ void
+-netsnmp_check_outstanding_agent_requests(void)
++netsnmp_check_delegated_requests(void)
+ {
+     netsnmp_agent_session *asp, *prev_asp = NULL, *next_asp = NULL;
+ 
+-    /*
+-     * deal with delegated requests
+-     */
+     for (asp = agent_delegated_list; asp; asp = next_asp) {
+         next_asp = asp->next;   /* save in case we clean up asp */
+         if (!netsnmp_check_for_delegated(asp)) {
+@@ -2790,6 +2797,22 @@ netsnmp_check_outstanding_agent_requests
+             prev_asp = asp;
+         }
+     }
++}
++
++/*
++ * loop through our sessions known delegated sessions and check to see
++ * if they've completed yet. If there are no more delegated sessions,
++ * check for and process any queued requests
++ */
++void
++netsnmp_check_outstanding_agent_requests(void)
++{
++    netsnmp_agent_session *asp;
++
++    /*
++     * deal with delegated requests
++     */
++    netsnmp_check_delegated_requests();
+ 
+     /*
+      * if we are processing a set and there are more delegated
+@@ -2819,7 +2842,8 @@ netsnmp_check_outstanding_agent_requests
+ 
+             netsnmp_processing_set = netsnmp_agent_queued_list;
+             DEBUGMSGTL(("snmp_agent", "SET request remains queued while "
+-                        "delegated requests finish, asp = %8p\n", asp));
++                        "delegated requests finish, asp = %8p\n",
++                        agent_delegated_list));
+             break;
+         }
+ #endif /* NETSNMP_NO_WRITE_SUPPORT */
+@@ -2880,6 +2904,11 @@ check_delayed_request(netsnmp_agent_sess
+     case SNMP_MSG_GETBULK:
+     case SNMP_MSG_GETNEXT:
+         netsnmp_check_all_requests_status(asp, 0);
++        if (agent_snmp_session_is_cancelled(asp)) {
++            printf("request %p is cancelled\n", asp);
++            DEBUGMSGTL(("snmp_agent","canceling next walk for asp %p\n", asp));
++            break;
++        }
+         handle_getnext_loop(asp);
+         if (netsnmp_check_for_delegated(asp) &&
+             netsnmp_check_transaction_id(asp->pdu->transid) !=
+@@ -3838,4 +3867,73 @@ netsnmp_set_all_requests_error(netsnmp_a
+     return error_value;
+ }
+ #endif /* NETSNMP_FEATURE_REMOVE_SET_ALL_REQUESTS_ERROR */
++
++/*
++ * Ugly hack to fix bug #950602 and preserve ABI
++ * (the official patch adds netsnmp_agent_session->flags).
++ * We must create parallel database of netsnmp_agent_sessions
++ * and put cancelled requests there instead of marking
++ * netsnmp_agent_session->flags.
++ */
++static netsnmp_agent_session **cancelled_agent_snmp_sessions;
++static int cancelled_agent_snmp_sessions_count;
++static int cancelled_agent_snmp_sessions_max;
++
++int
++agent_snmp_session_mark_cancelled(netsnmp_agent_session *session)
++{
++    DEBUGMSGTL(("agent:cancelled", "Cancelling session %p\n", session));
++    if (!session)
++        return 0;
++    if (cancelled_agent_snmp_sessions_count + 1 > cancelled_agent_snmp_sessions_max) {
++        netsnmp_agent_session **aux;
++        int max = cancelled_agent_snmp_sessions_max + 10;
++        aux = realloc(cancelled_agent_snmp_sessions, sizeof(netsnmp_agent_session*) * max);
++        if (!aux)
++            return SNMP_ERR_GENERR;
++        cancelled_agent_snmp_sessions = aux;
++        cancelled_agent_snmp_sessions_max = max;
++    }
++    cancelled_agent_snmp_sessions[cancelled_agent_snmp_sessions_count] = session;
++    cancelled_agent_snmp_sessions_count++;
++    return 0;
++}
++
++int
++agent_snmp_session_is_cancelled(netsnmp_agent_session *session)
++{
++    int i;
++    for (i=0; i<cancelled_agent_snmp_sessions_count; i++)
++        if (cancelled_agent_snmp_sessions[i] == session) {
++            DEBUGMSGTL(("agent:cancelled", "session %p is cancelled\n", session));
++            return TRUE;
++    }
++    return FALSE;
++}
++
++int
++agent_snmp_session_release_cancelled(netsnmp_agent_session *session)
++{
++    int i, j;
++
++    if (!session)
++        return 0;
++
++    DEBUGMSGTL(("agent:cancelled", "Removing session %p\n", session));
++
++    /* delete the session from cancelled_agent_snmp_sessions */
++    for (i=0, j=0; j<cancelled_agent_snmp_sessions_count; i++, j++)
++        if (cancelled_agent_snmp_sessions[j] == session)
++            i--; /* don't increase i in this loop iteration */
++        else
++            cancelled_agent_snmp_sessions[i] = cancelled_agent_snmp_sessions[j];
++
++    cancelled_agent_snmp_sessions_count = i;
++
++    for (; i< cancelled_agent_snmp_sessions_max; i++)
++        cancelled_agent_snmp_sessions[i] = NULL;
++    return 0;
++}
++
+ /** @} */
++
+diff -up net-snmp-5.7.2/include/net-snmp/agent/snmp_agent.h.disconnect-crash net-snmp-5.7.2/include/net-snmp/agent/snmp_agent.h
+--- net-snmp-5.7.2/include/net-snmp/agent/snmp_agent.h.disconnect-crash	2012-10-10 00:28:58.000000000 +0200
++++ net-snmp-5.7.2/include/net-snmp/agent/snmp_agent.h	2013-07-03 15:26:35.909813132 +0200
+@@ -240,6 +240,7 @@ extern          "C" {
+     int             init_master_agent(void);
+     void            shutdown_master_agent(void);
+     int             agent_check_and_process(int block);
++    void            netsnmp_check_delegated_requests(void);
+     void            netsnmp_check_outstanding_agent_requests(void);
+ 
+     int             netsnmp_request_set_error(netsnmp_request_info *request,
diff --git a/net-snmp-5.7-agentx-crash.patch b/net-snmp-5.7-agentx-crash.patch
new file mode 100644
index 0000000..d25ace5
--- /dev/null
+++ b/net-snmp-5.7-agentx-crash.patch
@@ -0,0 +1,57 @@
+729738 - net-snmp dumps core in netsnmp_oid_find_prefix
+1038011 - net-snmp: snmpd crashes/hangs when AgentX subagent times-out
+
+commit f9304c83f76202db0e684269ca1af32e43cd9db4
+Author: Jan Safranek <jsafranek at users.sourceforge.net>
+Date:   Tue Feb 7 14:53:44 2012 +0100
+
+    CHANGES: PATCH 1633670: fixed snmpd crashing when an AgentX subagent disconnect in the middle of processing of a request.
+    
+    I fixed also the memory leak reported in the tracker comments.
+
+diff --git a/agent/mibgroup/agentx/master.c b/agent/mibgroup/agentx/master.c
+index c42a42a..baeebaf 100644
+--- a/agent/mibgroup/agentx/master.c
++++ b/agent/mibgroup/agentx/master.c
+@@ -219,6 +219,9 @@ agentx_got_response(int operation,
+     if (!cache) {
+         DEBUGMSGTL(("agentx/master", "response too late on session %8p\n",
+                     session));
++        /* response is too late, free the cache */
++        if (magic)
++            netsnmp_free_delegated_cache((netsnmp_delegated_cache*) magic);
+         return 0;
+     }
+     requests = cache->requests;
+@@ -606,6 +609,8 @@ agentx_master_handler(netsnmp_mib_handler *handler,
+     result = snmp_async_send(ax_session, pdu, agentx_got_response, cb_data);
+     if (result == 0) {
+         snmp_free_pdu(pdu);
++        if (cb_data)
++            netsnmp_free_delegated_cache((netsnmp_delegated_cache*) cb_data);
+     }
+ 
+     return SNMP_ERR_NOERROR;
+diff --git a/agent/mibgroup/agentx/master_admin.c b/agent/mibgroup/agentx/master_admin.c
+index f16f392..b84b85e 100644
+--- a/agent/mibgroup/agentx/master_admin.c
++++ b/agent/mibgroup/agentx/master_admin.c
+@@ -133,11 +133,16 @@ close_agentx_session(netsnmp_session * session, int sessid)
+          * requests, so that the delegated request will be completed and
+          * further requests can be processed
+          */
+-        netsnmp_remove_delegated_requests_for_session(session);
++	while (netsnmp_remove_delegated_requests_for_session(session)) {
++		DEBUGMSGTL(("agentx/master", "Continue removing delegated reqests\n"));
++	}
++
+         if (session->subsession != NULL) {
+             netsnmp_session *subsession = session->subsession;
+             for(; subsession; subsession = subsession->next) {
+-                netsnmp_remove_delegated_requests_for_session(subsession);
++                while (netsnmp_remove_delegated_requests_for_session(subsession)) {
++			DEBUGMSGTL(("agentx/master", "Continue removing delegated subsession reqests\n"));
++		}
+             }
+         }
+                 
diff --git a/net-snmp.spec b/net-snmp.spec
index 3990ae1..057956e 100644
--- a/net-snmp.spec
+++ b/net-snmp.spec
@@ -39,6 +39,8 @@ Patch6: net-snmp-5.6-test-debug.patch
 Patch7: net-snmp-5.7.2-systemd.patch
 Patch8: net-snmp-5.7.2-python-ipaddress-size.patch
 Patch9: net-snmp-5.7.2-btrfs.patch
+Patch10: net-snmp-5.7-agentx-crash.patch
+Patch11: net-snmp-5.5-agentx-disconnect-crash.patch
 
 Requires(post): chkconfig
 Requires(preun): chkconfig
@@ -203,6 +205,8 @@ cp %{SOURCE12} .
 %patch7 -p1 -b .systemd
 %patch8 -p1 -b .ipaddress-size
 %patch9 -p1 -b .btrfs
+%patch10 -p1 -b .agentx-crash
+%patch11 -p1 -b .agentx-disconnect-crash
 
 %ifarch sparc64 s390 s390x
 # disable failing test - see https://bugzilla.redhat.com/show_bug.cgi?id=680697
@@ -497,6 +501,10 @@ rm -rf ${RPM_BUILD_ROOT}
 %{_initrddir}/snmptrapd
 
 %changelog
+* Thu Dec  5 2013 Jan Safranek <jsafrane at redhat.com> 1:5.7.2-7
+- Fixed snmpd crashing when AgentX subagent disconnects in the middle of
+  request processing (#1038011)
+
 * Fri Nov  8 2013 Jan Safranek <jsafrane at redhat.com> 1:5.7.2-6
 - Added btrfs support to hrFSTable
 


More information about the scm-commits mailing list