Re: double-free crash in agentx/master.c (bug #2943)

Bill, Bart,

Shouldn't there be a check (netsnmp_handler_check_cache)
just before snmp_async_send to make sure the cb_data is still
valid (or that the session hasn't been disconnected)?

I'm assuming (with good evidence) that our subagent
session was disconnected (our subagent has a problem)
and something was freed...and we attempt to free cb_data
again and core dump.  So at the bottom of agentx_got_response():


    /*
     * When the master sends a CleanupSet PDU, it will never get a response
     * back from the subagent. So we shouldn't allocate the
     * netsnmp_delegated_cache structure in this case.
     */
    if (pdu->command != AGENTX_MSG_CLEANUPSET)
        cb_data = netsnmp_create_delegated_cache(handler, reginfo,
                                                 reqinfo, requests,
                                                 (void *) ax_session);
    else
        cb_data = NULL;


*/* some checks */*








* cache = netsnmp_handler_check_cache(cb_data);    if (!cache) {
DEBUGMSGTL(("agentx/master", "session may be disconnected %8p\n",
          session));        /* response is too late, perhaps session was
disconnected? */        if (cb_data)
netsnmp_free_delegated_cache((netsnmp_delegated_cache*) magic);
return 1;    }*
*/* end checks */*

   /*
     * send the requests out.
     */
    DEBUGMSGTL(("agentx/master", "sending pdu
(req=0x%x,trans=0x%x,sess=0x%x)\n",
                (unsigned)pdu->reqid, (unsigned)pdu->transid,
(unsigned)pdu->sessid));

   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;

--Sam

On Tue, Jun 25, 2019 at 3:38 PM Sam Tannous <[email protected]>
wrote:

>
> I'm still not able to recreate this bug (#2943) where we
> double free cb_data at the bottom of agentx_master_handler()
> (with the netsnmp_free_delegated_cache()).
>
> Just in looking at the code logic, it seems like we allocate
> the netsnmp_delegated_cache structure only if the master
> sent a CleanupSet PDU.   In the case I'm looking at, I can
> see the master has already disconnected the subagent:
>
> bgpd[5180]: snmp[info]: AgentX master disconnected us, reconnecting in 15
> ip[8042]: *** Error in `/usr/sbin/snmpd': double free or corruption
> (fasttop): 0x0000000001d15420 ***
> zebra[21794]: snmp[info]: AgentX master disconnected us, reconnecting in 15
>
> So when this happens, the master attempts to close_agentx_session().
> My best guess (without being able to recreate this) is that this structure
> is freed here first.   Is it possible to somehow protect the session
> (and all subsessions) from being freed if there the master is in the
> process
> of allocating  netsnmp_delegated_cache?
>
> Can we set something like AGENTX_MSG_CLEANUPSET if we have
> disconnected (or timed out) any/all subagents?  This is just to prevent the
> double free that happens at the end of agentx_master_handler().
>
> Thanks,
> Sam
>
>
>
>

_______________________________________________
Net-snmp-coders mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders