From 7778f852d4c40066c4c5c096d17ff890a656ad31 Mon Sep 17 00:00:00 2001 From: Pavel Rochnyack Date: Thu, 13 Jul 2017 14:34:27 +0700 Subject: [PATCH] snmp_agent: Fix null pointer dereference As `table` field in `data_definition_t` is just backreference, then `snmp_agent_free_data()` should not alter parent structure. So, table columns unregistering and freeing has moved to new `snmp_agent_free_table_columns()` function. That also fixes null pointer dereference. Closes: #2338. --- src/snmp_agent.c | 72 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/src/snmp_agent.c b/src/snmp_agent.c index fe1ecd56..456d43e0 100644 --- a/src/snmp_agent.c +++ b/src/snmp_agent.c @@ -420,28 +420,6 @@ static void snmp_agent_free_data(data_definition_t **dd) { for (size_t i = 0; i < (*dd)->oids_len; i++) unregister_mib((*dd)->oids[i].oid, (*dd)->oids[i].oid_len); } - if (!(*dd)->table->index_oid.oid_len) { - char *instance; - - c_avl_iterator_t *iter = c_avl_get_iterator((*dd)->table->instance_index); - while (c_avl_iterator_next(iter, (void *)&instance, (void *)&instance) == - 0) { - for (size_t i = 0; i < (*dd)->oids_len; i++) - snmp_agent_unregister_oid_string(&(*dd)->oids[i], instance); - } - c_avl_iterator_destroy(iter); - } else { - /* unregister all table OIDs */ - int *index; - char *value; - - c_avl_iterator_t *iter = c_avl_get_iterator((*dd)->table->index_instance); - while (c_avl_iterator_next(iter, (void *)&index, (void *)&value) == 0) { - for (size_t i = 0; i < (*dd)->oids_len; i++) - snmp_agent_unregister_oid_index(&(*dd)->oids[i], *index); - } - c_avl_iterator_destroy(iter); - } sfree((*dd)->name); sfree((*dd)->plugin); @@ -455,6 +433,43 @@ static void snmp_agent_free_data(data_definition_t **dd) { return; } +static void snmp_agent_free_table_columns(table_definition_t *td) { + if (td->columns == NULL) + return; + + for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) { + data_definition_t *dd = de->value; + + if (td->index_oid.oid_len) { + int *index; + char *instance; + + c_avl_iterator_t *iter = c_avl_get_iterator(td->index_instance); + while (c_avl_iterator_next(iter, (void *)&index, (void *)&instance) == + 0) { + for (size_t i = 0; i < dd->oids_len; i++) + snmp_agent_unregister_oid_index(&dd->oids[i], *index); + } + c_avl_iterator_destroy(iter); + } else { + char *instance; + + c_avl_iterator_t *iter = c_avl_get_iterator(dd->table->instance_index); + while (c_avl_iterator_next(iter, (void *)&instance, (void *)&instance) == + 0) { + for (size_t i = 0; i < dd->oids_len; i++) + snmp_agent_unregister_oid_string(&dd->oids[i], instance); + } + c_avl_iterator_destroy(iter); + } + + snmp_agent_free_data(&dd); + } + + llist_destroy(td->columns); + td->columns = NULL; +} /* void snmp_agent_free_table_columns */ + static void snmp_agent_free_table(table_definition_t **td) { if (td == NULL || *td == NULL) @@ -463,23 +478,20 @@ static void snmp_agent_free_table(table_definition_t **td) { if ((*td)->size_oid.oid_len) unregister_mib((*td)->size_oid.oid, (*td)->size_oid.oid_len); + /* Unregister Index OIDs */ if ((*td)->index_oid.oid_len) { int *index; - char *value; + char *instance; c_avl_iterator_t *iter = c_avl_get_iterator((*td)->index_instance); - while (c_avl_iterator_next(iter, (void *)&index, (void *)&value) == 0) + while (c_avl_iterator_next(iter, (void *)&index, (void *)&instance) == 0) snmp_agent_unregister_oid_index(&(*td)->index_oid, *index); c_avl_iterator_destroy(iter); } - for (llentry_t *de = llist_head((*td)->columns); de != NULL; de = de->next) { - data_definition_t *dd = de->value; - snmp_agent_free_data(&dd); - } - - llist_destroy((*td)->columns); + /* Unregister all table columns and their registered OIDs */ + snmp_agent_free_table_columns(*td); void *key = NULL; void *value = NULL; -- 2.11.0