Mercurial > hg > ucx / changeset

--- a/src/kv_list.c	Sat Sep 06 11:57:17 2025 +0200
+++ b/src/kv_list.c	Sun Sep 07 17:08:26 2025 +0200
@@ -53,10 +53,79 @@
     struct cx_kv_list_map_s *map;
     const cx_list_class *list_methods;
     const cx_map_class *map_methods;
+    cx_destructor_func list_destr;
+    cx_destructor_func2 list_destr2;
+    void *list_destr_data;
+    cx_destructor_func map_destr;
+    cx_destructor_func2 map_destr2;
+    void *map_destr_data;
 };
 
+static void cx_kv_list_destructor_wrapper_list(void *list_ptr, void *elem) {
+    const cx_kv_list *kv_list = list_ptr;
+    // list destructor is already called with proper deref of the elem
+    if (kv_list->list_destr) {
+        kv_list->list_destr(elem);
+    }
+    if (kv_list->list_destr2) {
+        kv_list->list_destr2(kv_list->list_destr_data, elem);
+    }
+    if (kv_list->map_destr) {
+        kv_list->map_destr(elem);
+    }
+    if (kv_list->map_destr2) {
+        kv_list->map_destr2(kv_list->map_destr_data, elem);
+    }
+}
+
+static void cx_kv_list_destructor_wrapper_map(void *list_ptr, void *node_data) {
+    const cx_kv_list *kv_list = list_ptr;
+    // the elem called with a map destructor is a pointer to the node
+    // we need to deref the elem accordingly
+    void *elem = kv_list->list.list_base.collection.store_pointer ? *(void**)node_data : node_data;
+    if (kv_list->list_destr) {
+        kv_list->list_destr(elem);
+    }
+    if (kv_list->list_destr2) {
+        kv_list->list_destr2(kv_list->list_destr_data, elem);
+    }
+    if (kv_list->map_destr) {
+        kv_list->map_destr(elem);
+    }
+    if (kv_list->map_destr2) {
+        kv_list->map_destr2(kv_list->map_destr_data, elem);
+    }
+}
+
+static void cx_kv_list_update_destructors(cx_kv_list *list) {
+    // we copy the destructors to our custom fields and register
+    // an own destructor function which invokes all these
+    if (list->list.list_base.collection.simple_destructor != NULL) {
+        list->list_destr = list->list.list_base.collection.simple_destructor;
+        list->list.list_base.collection.simple_destructor = NULL;
+    }
+    if (list->list.list_base.collection.advanced_destructor != cx_kv_list_destructor_wrapper_list) {
+        list->list_destr2 = list->list.list_base.collection.advanced_destructor;
+        list->list_destr_data = list->list.list_base.collection.destructor_data;
+        list->list.list_base.collection.advanced_destructor = cx_kv_list_destructor_wrapper_list;
+        list->list.list_base.collection.destructor_data = list;
+    }
+    if (list->map->map_base.base.collection.simple_destructor != NULL) {
+        list->map_destr = list->map->map_base.base.collection.simple_destructor;
+        list->map->map_base.base.collection.simple_destructor = NULL;
+    }
+    if (list->map->map_base.base.collection.advanced_destructor != cx_kv_list_destructor_wrapper_map) {
+        list->map_destr2 = list->map->map_base.base.collection.advanced_destructor;
+        list->map_destr_data = list->map->map_base.base.collection.destructor_data;
+        list->map->map_base.base.collection.advanced_destructor = cx_kv_list_destructor_wrapper_map;
+        list->map->map_base.base.collection.destructor_data = list;
+    }
+}
+
 static void cx_kvl_deallocate(struct cx_list_s *list) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
+    // patch the destructors
+    cx_kv_list_update_destructors(kv_list);
     // free the map first
     kv_list->map_methods->deallocate(&kv_list->map->map_base.base);
     kv_list->list_methods->deallocate(list);
@@ -110,20 +179,23 @@
         void *targetbuf
 ) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
+    // patch the destructors
+    cx_kv_list_update_destructors(kv_list);
     // TODO: always use the target buffer to get the element first,
     //       then obtain the key, remove it from the map,
     //       and finally call any destructors manually
-    // TODO: map destructors are not called
     return kv_list->list_methods->remove(list, index, num, targetbuf);
 }
 
 static void cx_kvl_clear(struct cx_list_s *list) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
-    // FIXME: this is probably totally bugged, because
-    //        a) the map may have destructors which are invoked triggering UAF
-    //        b) the destructors registered with the map are not called for all list elements
+    // patch the destructors
+    // but remove the wrapper from the map to avoid calling it twice
+    cx_kv_list_update_destructors(kv_list);
+    cxDefineAdvancedDestructor(&kv_list->map->map_base.base, NULL, NULL);
+    // clear the list
     kv_list->list_methods->clear(list);
-    // also clear all lookup entries
+    // then clear the map
     cxMapClear(&kv_list->map->map_base.base);
 }
 
@@ -150,7 +222,8 @@
         bool remove
 ) {
     cx_kv_list *kv_list = (cx_kv_list*)list;
-    // TODO: implement removal of the key in the map and calling the map destructors
+    cx_kv_list_update_destructors(kv_list);
+    // TODO: implement removal of the key in the map
     return kv_list->list_methods->find_remove(list, elem, remove);
 }
 
@@ -182,7 +255,8 @@
 
 static void cx_kvl_map_clear(struct cx_map_s *map) {
     cx_kv_list *kv_list = ((struct cx_kv_list_map_s*)map)->list;
-    // TODO: iterate through the map elements and remove the key from the referenced list items
+    cx_kv_list_update_destructors(kv_list);
+    // TODO: iterate through the map elements, unlink them from the list, and free them
     kv_list->map_methods->clear(map);
 }
 
@@ -206,6 +280,7 @@
 
 int cx_kvl_map_remove(CxMap *map, CxHashKey key, void *targetbuf) {
     cx_kv_list *kv_list = ((struct cx_kv_list_map_s*)map)->list;
+
     void *node_data;
     if (kv_list->map_methods->remove(map, key, &node_data)) {
         return 1;
@@ -217,20 +292,9 @@
 
     // check if the outside caller want's us to return or to destroy the element
     if (targetbuf == NULL) {
-        // destroy the element
-        // invoke all destructors from both the list and the map aspect
-        // (usually the user will only use one of the aspects anyway and if not, it's documented)
-        cx_invoke_destructor(&kv_list->list.list_base, node_data);
-        // note that we cannot use the macro, because it will use the wrong store_pointer information
-        CxMap *map_aspect = &kv_list->map->map_base.base;
-        if (map_aspect->collection.simple_destructor) {
-            map_aspect->collection.simple_destructor(
-                kv_list->list.list_base.collection.store_pointer ? *(void**)node_data : node_data);
-        }
-        if (map_aspect->collection.advanced_destructor) {
-            map_aspect->collection.advanced_destructor(map_aspect->collection.destructor_data,
-                kv_list->list.list_base.collection.store_pointer ? *(void**)node_data : node_data);
-        }
+        // patch the destructors and invoke them through the wrapper
+        cx_kv_list_update_destructors(kv_list);
+        cx_invoke_advanced_destructor(&kv_list->list.list_base, node_data);
     } else {
         // copy the element to the target buffer
         memcpy(targetbuf, node_data, kv_list->list.list_base.collection.elem_size);
@@ -316,6 +380,9 @@
     // reallocate the list to add memory for storing the metadata
     cx_kv_list *kv_list = cxRealloc(allocator, list, sizeof(struct cx_kv_list_s));
 
+    // zero the custom destructor information
+    memset((char*)kv_list + offsetof(cx_kv_list, list_destr), 0, sizeof(void*)*6);
+
     // if any of the reallocations failed, we bail out
     if (kv_map != NULL && kv_list != NULL) {
         map = (CxMap*) kv_map;