Commits

ripencc  committed 84b8af4

IS-657 Fix prefix list memory leak

  • Participants
  • Parent commits 8bcfc43

Comments (0)

Files changed (3)

File bgpdump_attr.h

 #define MP_IPV6_WITHDRAW(m) ((m)->withdraw[AFI_IP6][SAFI_UNICAST])
 #endif
 
+struct prefix {
+    BGPDUMP_IP_ADDRESS	address;
+    u_char		len;
+};
+
+#define MAX_PREFIXES 1000
 struct mp_nlri {
   u_char		nexthop_len;
 
   BGPDUMP_IP_ADDRESS 	nexthop_local;
 
   u_int16_t		prefix_count;
-  struct prefix		*nlri;
+  struct prefix		nlri[MAX_PREFIXES];
 };
 
 #endif /* _BGPDUMP_ATTR_H */

File bgpdump_formats.h

 #define BGP_MSG_ROUTE_REFRESH_01           5
 #define BGP_MSG_ROUTE_REFRESH	         128
 
-struct prefix {
-    BGPDUMP_IP_ADDRESS	address;
-    u_char		len;
-};
-
 typedef struct struct_BGPDUMP_MRTD_MESSAGE {
     u_int16_t		source_as;
     struct in_addr	source_ip;
     /* For UPDATE packets */
     u_int16_t		withdraw_count;
     u_int16_t		announce_count;
-    struct prefix	*withdraw;
-    struct prefix	*announce;
+    struct prefix	withdraw[MAX_PREFIXES];
+    struct prefix	announce[MAX_PREFIXES];
 
     /* For corrupt update dumps */
     u_int16_t cut_bytes;

File bgpdump_lib.c

 
 static    void process_mp_announce(struct mstream *s, struct mp_info *info, struct zebra_incomplete *incomplete);
 static    void process_mp_withdraw(struct mstream *s, struct mp_info *info, struct zebra_incomplete *incomplete);
-static    int read_prefix_list(struct mstream *s, u_int16_t af, struct prefix **prefixarray, struct zebra_incomplete *incomplete);
+static    int read_prefix_list(struct mstream *s, u_int16_t af, struct prefix *prefixes, struct zebra_incomplete *incomplete);
 
 static    as_t read_asn(struct mstream *s, as_t *asn, u_int8_t len);
 static    struct aspath *create_aspath(u_int16_t len, u_int8_t asn_len);
     for(afi = 1; afi <= BGPDUMP_MAX_AFI; afi++) {
 	for(safi = 1; safi < BGPDUMP_MAX_SAFI; safi++) {
 	    if(info->announce[afi][safi])
-		if(info->announce[afi][safi]->nlri) {
-			free(info->announce[afi][safi]->nlri);
-			info->announce[afi][safi]->nlri = NULL;
-		}
 		free(info->announce[afi][safi]);
 		info->announce[afi][safi] = NULL;
 	    if(info->withdraw[afi][safi]) {
 		switch(entry->subtype) {
 		    case BGPDUMP_SUBTYPE_ZEBRA_BGP_MESSAGE:
 			switch(entry->body.zebra_message.type) {
-			    case BGP_MSG_UPDATE:
-				if(entry->body.zebra_message.withdraw != NULL)
-				    free(entry->body.zebra_message.withdraw);
-    				if(entry->body.zebra_message.announce != NULL)
-				    free(entry->body.zebra_message.announce);
-				break;
 			    case BGP_MSG_NOTIFY:
 				if(entry->body.zebra_message.notify_data)
 				    free(entry->body.zebra_message.notify_data);
     mstream_getw(s,&entry->body.zebra_message.interface_index);
     mstream_getw(s,&entry->body.zebra_message.address_family);
 
-    /* Initialize announce and withdraw arrays: if there is a
-     * parse error, they will not be free()d, and we will not segfault. */
-    entry->body.zebra_message.withdraw = NULL;
-    entry->body.zebra_message.announce = NULL;
-
     entry->body.zebra_message.opt_len = 0;
     entry->body.zebra_message.opt_data = NULL;
     entry->body.zebra_message.notify_len = 0;
 
     mstream_t withdraw_stream = mstream_copy(s, mstream_getw(s, NULL));
     entry->body.zebra_message.withdraw_count = read_prefix_list(&withdraw_stream, AFI_IP,
-                         &entry->body.zebra_message.withdraw,
+                         entry->body.zebra_message.withdraw,
 			 &entry->body.zebra_message.incomplete);
 
     entry->attr = process_attributes(s, asn_len, &entry->body.zebra_message.incomplete);
 
     entry->body.zebra_message.announce_count = read_prefix_list(s, AFI_IP, 
-                         &entry->body.zebra_message.announce,
+                         entry->body.zebra_message.announce,
 			 &entry->body.zebra_message.incomplete);
 
     return 1;
         mstream_get(s, NULL, mstream_getc(s, NULL));
     }
 
-    info->announce[afi][safi]->prefix_count = read_prefix_list(s, afi, &info->announce[afi][safi]->nlri, incomplete);
+    info->announce[afi][safi]->prefix_count = read_prefix_list(s, afi, info->announce[afi][safi]->nlri, incomplete);
 }
 
 void process_mp_withdraw(struct mstream *s, struct mp_info *info, struct zebra_incomplete *incomplete) {
 	memset(mp_nlri, 0, sizeof(struct mp_nlri));
 	info->withdraw[afi][safi] = mp_nlri;
 
-	mp_nlri->prefix_count = read_prefix_list(s, afi, &mp_nlri->nlri, incomplete);
+	mp_nlri->prefix_count = read_prefix_list(s, afi, mp_nlri->nlri, incomplete);
 }
 
-static int read_prefix_list(struct mstream *s, u_int16_t afi, struct prefix **nlri, struct zebra_incomplete *incomplete) {
-    u_int16_t count = 0;
-    struct prefix *prefixes = NULL;
+static int read_prefix_list(struct mstream *s, u_int16_t afi, struct prefix *prefixes, struct zebra_incomplete *incomplete) {
+    int count = 0;
     
     while(mstream_can_read(s)) {
         u_int8_t p_len = mstream_getc(s,NULL); // length in bits
         
         /* Truncated prefix list? */
         if(mstream_can_read(s) < p_bytes) {
-            if(incomplete) {
-                /* Put prefix in incomplete structure */
-                memset(&incomplete->prefix, 0, sizeof(struct prefix));
-                incomplete->afi = afi;
-                incomplete->orig_len = p_len;
-                incomplete->prefix.len = mstream_can_read(s) * 8;
-                mstream_get(s, &incomplete->prefix.address, mstream_can_read(s));
-            } else {
-                /* Just skip over it */
-                mstream_get(s, NULL, mstream_can_read(s));
-            }
-            /* In either case, don't put it in the prefix array */
+            if(! incomplete)
+                break;
+            
+            /* Put prefix in incomplete structure */
+            incomplete->afi = afi;
+            incomplete->orig_len = p_len;
+            incomplete->prefix = (struct prefix) {
+                .len = mstream_can_read(s) * 8
+            };
+            mstream_get(s, &incomplete->prefix.address, p_bytes);
             break;
         }
         
-        /* Reallocate prefix array to add room for one more prefix*/
-        prefixes = realloc(prefixes, (count+1) * sizeof(struct prefix));
+        struct prefix *prefix = prefixes + count;
         
-        /* Fill new prefix with zeros, set prefix length */
-        memset(&prefixes[count],0,sizeof(struct prefix));
-        prefixes[count].len = p_len;
-        
-        /* Copy prefix */
-        mstream_get(s, &prefixes[count].address, p_bytes);
-        count++;
+        if(count++ > MAX_PREFIXES)
+            continue;
+
+        *prefix = (struct prefix) { .len = p_len };
+        mstream_get(s, &prefix->address, p_bytes);
     }
-    *nlri = prefixes;
+    
+    if(count > MAX_PREFIXES) {
+        err("too many prefixes (%i > %i)", count, MAX_PREFIXES);
+        return MAX_PREFIXES;
+    }
+    
     return count;
 }