From 063971dd14138bcefaa9e21a703a3c156c3c045c Mon Sep 17 00:00:00 2001 From: martijn Date: Mon, 18 Dec 2023 09:42:57 +0000 Subject: [PATCH] Fix two memory leaks: - MIB_snmpInReadOnlys was tried to be registered twice, leading to a leak of the second instance. Prevent this mistake in the future by making a double registration fatal. - The response buffer is owned by the backend, so the backend must also free it. OK tb@ --- usr.sbin/snmpd/application_internal.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/usr.sbin/snmpd/application_internal.c b/usr.sbin/snmpd/application_internal.c index 11fec82d67a..a9c637b23d0 100644 --- a/usr.sbin/snmpd/application_internal.c +++ b/usr.sbin/snmpd/application_internal.c @@ -1,4 +1,4 @@ -/* $OpenBSD: application_internal.c,v 1.9 2023/12/12 20:15:49 martijn Exp $ */ +/* $OpenBSD: application_internal.c,v 1.10 2023/12/18 09:42:57 martijn Exp $ */ /* * Copyright (c) 2023 Martijn van Duren @@ -130,8 +130,6 @@ appl_internal_init(void) NULL); appl_internal_object(&OID(MIB_snmpInReadOnlys), appl_internal_snmp, NULL); - appl_internal_object(&OID(MIB_snmpInReadOnlys), appl_internal_snmp, - NULL); appl_internal_object(&OID(MIB_snmpInGenErrs), appl_internal_snmp, NULL); appl_internal_object(&OID(MIB_snmpInTotalReqVars), appl_internal_snmp, NULL); @@ -253,6 +251,7 @@ appl_internal_object(struct ber_oid *oid, struct ber_element *(*getnext)(int8_t, struct ber_oid *)) { struct appl_internal_object *obj; + char buf[1024]; if ((obj = calloc(1, sizeof(*obj))) == NULL) fatal(NULL); @@ -261,7 +260,10 @@ appl_internal_object(struct ber_oid *oid, obj->getnext = getnext; obj->stringval = NULL; - RB_INSERT(appl_internal_objects, &appl_internal_objects, obj); + if (RB_INSERT(appl_internal_objects, + &appl_internal_objects, obj) != NULL) + fatalx("%s: %s already registered", __func__, + smi_oid2string(oid, buf, sizeof(buf), 0)); } const char * @@ -351,6 +353,8 @@ appl_internal_get(struct appl_backend *backend, __unused int32_t transactionid, resp[i - 1].av_next = NULL; appl_response(backend, requestid, APPL_ERROR_NOERROR, 0, resp); + + free(resp); return; fail: @@ -434,6 +438,8 @@ appl_internal_getnext(struct appl_backend *backend, resp[i - 1].av_next = NULL; appl_response(backend, requestid, APPL_ERROR_NOERROR, 0, resp); + + free(resp); return; fail: -- 2.20.1