bgpd: fix bgp_pbr_or_filter memory leaks

Note that bgp_pbr_policyroute_add_from_zebra() and
bgp_pbr_policyroute_remove_from_zebra() are only called from
bgp_pbr_handle_entry().

>  ==966967==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x7fd44744401a in list_new lib/linklist.c:49
>     #3 0x560f8c094490 in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2818
>     FRRouting#4 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     FRRouting#5 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     FRRouting#6 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     FRRouting#7 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     FRRouting#8 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     FRRouting#9 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     FRRouting#10 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     FRRouting#11 0x7fd4475779b2 in event_call lib/event.c:2011
>     FRRouting#12 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     FRRouting#13 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     FRRouting#14 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x7fd44744401a in list_new lib/linklist.c:49
>     #3 0x560f8c09439d in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2812
>     FRRouting#4 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     FRRouting#5 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     FRRouting#6 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     FRRouting#7 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     FRRouting#8 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     FRRouting#9 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     FRRouting#10 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     FRRouting#11 0x7fd4475779b2 in event_call lib/event.c:2011
>     FRRouting#12 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     FRRouting#13 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     FRRouting#14 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> Direct leak of 4 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x560f8c080cec in bgp_pbr_extract_enumerate_unary bgpd/bgp_pbr.c:362
>     #3 0x560f8c080f7e in bgp_pbr_extract_enumerate bgpd/bgp_pbr.c:400
>     FRRouting#4 0x560f8c094530 in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2819
>     FRRouting#5 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     FRRouting#6 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     FRRouting#7 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     FRRouting#8 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     FRRouting#9 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     FRRouting#10 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     FRRouting#11 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     FRRouting#12 0x7fd4475779b2 in event_call lib/event.c:2011
>     FRRouting#13 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     FRRouting#14 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     FRRouting#15 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> Direct leak of 4 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x560f8c080cec in bgp_pbr_extract_enumerate_unary bgpd/bgp_pbr.c:362
>     #3 0x560f8c080f7e in bgp_pbr_extract_enumerate bgpd/bgp_pbr.c:400
>     FRRouting#4 0x560f8c09443d in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2813
>     FRRouting#5 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     FRRouting#6 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     FRRouting#7 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     FRRouting#8 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     FRRouting#9 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     FRRouting#10 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     FRRouting#11 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     FRRouting#12 0x7fd4475779b2 in event_call lib/event.c:2011
>     FRRouting#13 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     FRRouting#14 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     FRRouting#15 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Signed-off-by: Louis Scalbert <[email protected]>

Author: louis-6wind

bgpd/bgp_pbr.c

@@ -279,6 +279,13 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp,
 
 static void bgp_pbr_dump_entry(struct bgp_pbr_filter *bpf, bool add);
 
+static void bgp_pbr_val_mask_free(void *arg)
+{
+	struct bgp_pbr_val_mask *pbr_val_mask = arg;
+
+	XFREE(MTYPE_PBR_VALMASK, pbr_val_mask);
+}
+
 static bool bgp_pbr_extract_enumerate_unary_opposite(
 				 uint8_t unary_operator,
 				 struct bgp_pbr_val_mask *and_valmask,
@@ -2130,17 +2137,6 @@ static void bgp_pbr_policyroute_remove_from_zebra(
 			bgp, path, bpf, bpof, FLOWSPEC_ICMP_TYPE);
 	else
 		bgp_pbr_policyroute_remove_from_zebra_unit(bgp, path, bpf);
-	/* flush bpof */
-	if (bpof->tcpflags)
-		list_delete_all_node(bpof->tcpflags);
-	if (bpof->dscp)
-		list_delete_all_node(bpof->dscp);
-	if (bpof->flowlabel)
-		list_delete_all_node(bpof->flowlabel);
-	if (bpof->pkt_len)
-		list_delete_all_node(bpof->pkt_len);
-	if (bpof->fragment)
-		list_delete_all_node(bpof->fragment);
 }
 
 static void bgp_pbr_dump_entry(struct bgp_pbr_filter *bpf, bool add)
@@ -2625,19 +2621,6 @@ static void bgp_pbr_policyroute_add_to_zebra(struct bgp *bgp,
 			bgp, path, bpf, bpof, nh, rate, FLOWSPEC_ICMP_TYPE);
 	else
 		bgp_pbr_policyroute_add_to_zebra_unit(bgp, path, bpf, nh, rate);
-	/* flush bpof */
-	if (bpof->tcpflags)
-		list_delete_all_node(bpof->tcpflags);
-	if (bpof->dscp)
-		list_delete_all_node(bpof->dscp);
-	if (bpof->pkt_len)
-		list_delete_all_node(bpof->pkt_len);
-	if (bpof->fragment)
-		list_delete_all_node(bpof->fragment);
-	if (bpof->icmp_type)
-		list_delete_all_node(bpof->icmp_type);
-	if (bpof->icmp_code)
-		list_delete_all_node(bpof->icmp_code);
 }
 
 static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
@@ -2703,6 +2686,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 			srcp = ⦥
 		else {
 			bpof.icmp_type = list_new();
+			bpof.icmp_type->del = bgp_pbr_val_mask_free;
 			bgp_pbr_extract_enumerate(api->icmp_type,
 						  api->match_icmp_type_num,
 						  OPERATOR_UNARY_OR,
@@ -2718,6 +2702,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 			dstp = &range_icmp_code;
 		else {
 			bpof.icmp_code = list_new();
+			bpof.icmp_code->del = bgp_pbr_val_mask_free;
 			bgp_pbr_extract_enumerate(api->icmp_code,
 						  api->match_icmp_code_num,
 						  OPERATOR_UNARY_OR,
@@ -2738,6 +2723,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 						  FLOWSPEC_TCP_FLAGS);
 		} else if (kind_enum == OPERATOR_UNARY_OR) {
 			bpof.tcpflags = list_new();
+			bpof.tcpflags->del = bgp_pbr_val_mask_free;
 			bgp_pbr_extract_enumerate(api->tcpflags,
 						  api->match_tcpflags_num,
 						  OPERATOR_UNARY_OR,
@@ -2755,6 +2741,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 			bpf.pkt_len = &pkt_len;
 		else {
 			bpof.pkt_len = list_new();
+			bpof.pkt_len->del = bgp_pbr_val_mask_free;
 			bgp_pbr_extract_enumerate(api->packet_length,
 						  api->match_packet_length_num,
 						  OPERATOR_UNARY_OR,
@@ -2764,12 +2751,14 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 	}
 	if (api->match_dscp_num >= 1) {
 		bpof.dscp = list_new();
+		bpof.dscp->del = bgp_pbr_val_mask_free;
 		bgp_pbr_extract_enumerate(api->dscp, api->match_dscp_num,
 					  OPERATOR_UNARY_OR,
 					  bpof.dscp, FLOWSPEC_DSCP);
 	}
 	if (api->match_fragment_num) {
 		bpof.fragment = list_new();
+		bpof.fragment->del = bgp_pbr_val_mask_free;
 		bgp_pbr_extract_enumerate(api->fragment,
 					  api->match_fragment_num,
 					  OPERATOR_UNARY_OR,
@@ -2785,7 +2774,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 	bpf.family = afi2family(api->afi);
 	if (!add) {
 		bgp_pbr_policyroute_remove_from_zebra(bgp, path, &bpf, &bpof);
-		return;
+		goto flush_bpof;
 	}
 	/* no action for add = true */
 	for (i = 0; i < api->action_num; i++) {
@@ -2863,6 +2852,22 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
 		if (continue_loop == 0)
 			break;
 	}
+
+flush_bpof:
+	if (bpof.tcpflags)
+		list_delete(&bpof.tcpflags);
+	if (bpof.dscp)
+		list_delete(&bpof.dscp);
+	if (bpof.flowlabel)
+		list_delete(&bpof.flowlabel);
+	if (bpof.pkt_len)
+		list_delete(&bpof.pkt_len);
+	if (bpof.fragment)
+		list_delete(&bpof.fragment);
+	if (bpof.icmp_type)
+		list_delete(&bpof.icmp_type);
+	if (bpof.icmp_code)
+		list_delete(&bpof.icmp_code);
 }
 
 void bgp_pbr_update_entry(struct bgp *bgp, const struct prefix *p,