Use event_add(), instead of calling appl_agentx_send() directly. If an

error occurs the connection will be freed and if the caller uses the
connection afterwards it will lead to a use after free.

OK tb@

diff --git a/usr.sbin/snmpd/application_agentx.c b/usr.sbin/snmpd/application_agentx.c
index 2d13f04..c661cda 100644 (file)
--- a/usr.sbin/snmpd/application_agentx.c
+++ b/usr.sbin/snmpd/application_agentx.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: application_agentx.c,v 1.12 2023/10/24 14:11:14 martijn Exp $ */
+/*     $OpenBSD: application_agentx.c,v 1.13 2023/10/27 10:23:58 martijn Exp $ */
 /*
  * Copyright (c) 2022 Martijn van Duren <martijn@openbsd.org>
  *
@@ -254,9 +254,6 @@ appl_agentx_free(struct appl_agentx_connection *conn,
 {
        struct appl_agentx_session *session;
 
-       event_del(&(conn->conn_rev));
-       event_del(&(conn->conn_wev));
-
        while ((session = TAILQ_FIRST(&(conn->conn_sessions))) != NULL) {
                if (conn->conn_ax == NULL)
                        appl_agentx_session_free(session);
@@ -265,7 +262,12 @@ appl_agentx_free(struct appl_agentx_connection *conn,
                            reason);
        }
 
+       event_del(&(conn->conn_rev));
+       event_del(&(conn->conn_wev));
+
        RB_REMOVE(appl_agentx_conns, &appl_agentx_conns, conn);
+       if (conn->conn_ax != NULL)
+               (void)ax_send(conn->conn_ax);
        ax_free(conn->conn_ax);
        if (conn->conn_backend)
                fatalx("AgentX(%"PRIu32"): disappeared unexpected",
@@ -419,7 +421,7 @@ appl_agentx_recv(int fd, short event, void *cookie)
                    pdu->ap_header.aph_transactionid,
                    pdu->ap_header.aph_packetid, smi_getticks(),
                    APPL_ERROR_NOERROR, 0, NULL, 0);
-               appl_agentx_send(-1, EV_WRITE, conn);
+               event_add(&(conn->conn_wev), NULL);
                break;
        case AX_PDU_TYPE_INDEXALLOCATE:
        case AX_PDU_TYPE_INDEXDEALLOCATE:
@@ -431,7 +433,7 @@ appl_agentx_recv(int fd, short event, void *cookie)
                    APPL_ERROR_PROCESSINGERROR, 1,
                    pdu->ap_payload.ap_vbl.ap_varbind,
                    pdu->ap_payload.ap_vbl.ap_nvarbind);
-               appl_agentx_send(-1, EV_WRITE, conn);
+               event_add(&(conn->conn_wev), NULL);
                break;
        case AX_PDU_TYPE_ADDAGENTCAPS:
        case AX_PDU_TYPE_REMOVEAGENTCAPS:
@@ -451,7 +453,7 @@ appl_agentx_recv(int fd, short event, void *cookie)
            pdu->ap_header.aph_transactionid,
            pdu->ap_header.aph_packetid, smi_getticks(),
            error, 0, NULL, 0);
-       appl_agentx_send(-1, EV_WRITE, conn);
+       event_add(&(conn->conn_wev), NULL);
        ax_pdu_free(pdu);
 
        if (session == NULL || error != APPL_ERROR_PARSEERROR)
@@ -560,13 +562,13 @@ appl_agentx_open(struct appl_agentx_connection *conn, struct ax_pdu *pdu)
        ax_response(conn->conn_ax, session->sess_id,
            pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid,
            smi_getticks(), APPL_ERROR_NOERROR, 0, NULL, 0);
-       appl_agentx_send(-1, EV_WRITE, conn);
+       event_add(&(conn->conn_wev), NULL);
 
        return;
  fail:
        ax_response(conn->conn_ax, 0, pdu->ap_header.aph_transactionid,
            pdu->ap_header.aph_packetid, 0, error, 0, NULL, 0);
-       appl_agentx_send(-1, EV_WRITE, conn);
+       event_add(&(conn->conn_wev), NULL);
        if (session != NULL)
                free(session->sess_descr.aos_string);
        free(session);
@@ -592,7 +594,7 @@ appl_agentx_close(struct appl_agentx_session *session, struct ax_pdu *pdu)
        ax_response(conn->conn_ax, pdu->ap_header.aph_sessionid,
            pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid,
            smi_getticks(), error, 0, NULL, 0);
-       appl_agentx_send(-1, EV_WRITE, conn);
+       event_add(&(conn->conn_wev), NULL);
        if (error == APPL_ERROR_NOERROR)
                return;
 
@@ -612,7 +614,7 @@ appl_agentx_forceclose(struct appl_backend *backend,
        session->sess_conn->conn_ax->ax_byteorder = session->sess_byteorder;
        ax_close(session->sess_conn->conn_ax, session->sess_id,
            (enum ax_close_reason) reason);
-       appl_agentx_send(-1, EV_WRITE, session->sess_conn);
+       event_add(&(session->sess_conn->conn_wev), NULL);
 
        strlcpy(name, session->sess_backend.ab_name, sizeof(name));
        appl_agentx_session_free(session);
@@ -671,7 +673,7 @@ appl_agentx_register(struct appl_agentx_session *session, struct ax_pdu *pdu)
        ax_response(session->sess_conn->conn_ax, session->sess_id,
            pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid,
            smi_getticks(), error, 0, NULL, 0);
-       appl_agentx_send(-1, EV_WRITE, session->sess_conn);
+       event_add(&(session->sess_conn->conn_wev), NULL);
 }
 
 void
@@ -698,7 +700,7 @@ appl_agentx_unregister(struct appl_agentx_session *session, struct ax_pdu *pdu)
        ax_response(session->sess_conn->conn_ax, session->sess_id,
            pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid,
            smi_getticks(), error, 0, NULL, 0);
-       appl_agentx_send(-1, EV_WRITE, session->sess_conn);
+       event_add(&(session->sess_conn->conn_wev), NULL);
 }
 
 #define AX_PDU_FLAG_INDEX (AX_PDU_FLAG_NEW_INDEX | AX_PDU_FLAG_ANY_INDEX)
@@ -748,7 +750,7 @@ appl_agentx_get(struct appl_backend *backend, int32_t transactionid,
            requestid, context, srl, nsr) == -1)
                appl_response(backend, requestid, APPL_ERROR_GENERR, 1, vblist);
        else
-               appl_agentx_send(-1, EV_WRITE, session->sess_conn);
+               event_add(&(session->sess_conn->conn_wev), NULL);
        free(srl);
        if (context != NULL)
                free(context->aos_string);
@@ -801,7 +803,7 @@ appl_agentx_getnext(struct appl_backend *backend, int32_t transactionid,
            requestid, context, srl, nsr) == -1)
                appl_response(backend, requestid, APPL_ERROR_GENERR, 1, vblist);
        else
-               appl_agentx_send(-1, EV_WRITE, session->sess_conn);
+               event_add(&(session->sess_conn->conn_wev), NULL);
        free(srl);
        if (context != NULL)
                free(context->aos_string);