]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
Bugfix
authorIgor Putovny <igor.putovny@nic.cz>
Mon, 19 Aug 2024 13:24:52 +0000 (15:24 +0200)
committerIgor Putovny <igor.putovny@nic.cz>
Fri, 23 Aug 2024 14:26:17 +0000 (16:26 +0200)
During implementation of the new mechanism of storing potential buckets it was
found that aggregator allocates more than 4000 buckets despite routes in the
configuration file being grouped to approximately 200 buckets.

Upon closer inspection it became clear that tmp_bucket in aggregator_rt_notify()
was allocated in the linpool when it should have been allocated on stack.
Moreover, tmp_bucket was allocated without additional space for aggr_data.
These data were never copied from tmp_bucket to new_bucket.

This was the source of false negative results of HASH_FIND. Buckets are compared
not only by their hashes, but also lists of aggr_data. Because same_val_list()
was reading beyond allocated memory, buckets were never equal. HASH_FIND
therefore returned NULL, which prompted aggregator to create many more buckets
despite already existing buckets with the same hash.

proto/aggregator/aggregator.c

index d49fdbab29b0c1a732df874e95f0563e09d60df0..299c04eebf1696c38d0cc4a6e88c7a7c4ae09abf 100644 (file)
@@ -1554,7 +1554,7 @@ aggregator_rt_notify(struct proto *P, struct channel *src_ch, net *net, rte *new
       return;
 
     /* Evaluate route attributes. */
-    struct aggregator_bucket *tmp_bucket = lp_allocz(p->bucket_pool, sizeof(*tmp_bucket));
+    struct aggregator_bucket *tmp_bucket = allocz(sizeof(*tmp_bucket) + sizeof(tmp_bucket->aggr_data[0]) * p->aggr_on_count);
     assert(tmp_bucket->id == 0);
 
     for (uint val_idx = 0; val_idx < p->aggr_on_count; val_idx++)
@@ -1663,7 +1663,8 @@ aggregator_rt_notify(struct proto *P, struct channel *src_ch, net *net, rte *new
       ;
     else
     {
-      new_bucket = tmp_bucket;
+      new_bucket = lp_allocz(p->bucket_pool, sizeof(*new_bucket) + sizeof(new_bucket->aggr_data[0]) * p->aggr_on_count);
+      memcpy(new_bucket, tmp_bucket, sizeof(*new_bucket) + sizeof(new_bucket->aggr_data[0]) * p->aggr_on_count);
       HASH_INSERT2(p->buckets, AGGR_BUCK, p->p.pool, new_bucket);
     }