bgpd: fix a bug in bgp_attr_dup

Commit 558d1fec11749d3257e improved bgp_attr_dup so it would be possible
for the caller to provide attr_extra, allowing to use the stack instead
of the heap for operations requiring only a short lived attr.

However, this commit introduced a bug where bgp_attr_dup wouldn't copy
attr_extra at all (but provide a reference to the original) if the
caller provided attr_extra.

Cc: Jorge Boncompte [DTI2] <jorge@dti2.net>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 2cbd7bc3570f0794573145bcf98891d3aa956e94..1dce39bcc3321f60df4deec344a1c130915bcf2a 100644 (file)
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -321,11 +321,24 @@ bgp_attr_dup (struct attr *new, struct attr *orig)
   struct attr_extra *extra = new->extra;
 
   *new = *orig;
-  if (orig->extra)
+  /* if caller provided attr_extra space, use it in any case.
+   *
+   * This is neccesary even if orig->extra equals NULL, because otherwise
+   * memory may be later allocated on the heap by bgp_attr_extra_get.
+   *
+   * That memory would eventually be leaked, because the caller must not
+   * call bgp_attr_extra_free if he provided attr_extra on the stack.
+   */
+  if (extra)
+    {
+      new->extra = extra;
+      memset(new->extra, 0, sizeof(struct attr_extra));
+      if (orig->extra)
+        *new->extra = *orig->extra;
+    }
+  else if (orig->extra)
     {
-      /* if caller provided attr_extra space use it */
-      if (! extra)
-        new->extra = bgp_attr_extra_new();
+      new->extra = bgp_attr_extra_new();
       *new->extra = *orig->extra;
     }
 }