Code

snmp_agent: Fix null pointer dereference
authorPavel Rochnyack <pavel2000@ngs.ru>
Thu, 13 Jul 2017 07:34:27 +0000 (14:34 +0700)
committerPavel Rochnyack <pavel2000@ngs.ru>
Thu, 13 Jul 2017 15:21:24 +0000 (22:21 +0700)
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

index fe1ecd56ac246b0b3cd4ad3198d420469426cd9f..456d43e0ab933724cb2c3332a299e4d7e70e8653 100644 (file)
@@ -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;